<table><tr><td style="">dfaure added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D9206" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Hmm. I can see the usefulness, it's certainly much better than the way people have historically inserted custom widgets into QFileDialog (<a href="https://stackoverflow.com/questions/16987916/add-widgets-to-qfiledialog" class="remarkup-link" target="_blank" rel="noreferrer">https://stackoverflow.com/questions/16987916/add-widgets-to-qfiledialog</a>, URGH).</p>

<p>On the other hand, I'm wondering if this is going to be abused as a KFileDialog replacement, losing the "native" look-n-feel for the file dialog on Windows / macOS.<br />
Well, that's certainly a pre-requisite for any custom widget to work, it should just be made extra clear to app developers that what they lose in return is the ability to get native dialogs on Windows and macOS (and Gnome, I suppose).</p>

<p>So, I'm OK with this, provided that this documentation is added:</p>

<p>The downside of using this class is that your application will not be able to get a native file dialog on non-Plasma environments (Windows, macOS, ...). For this reason you should really think twice before deciding to use it, as a custom widget might improve user experience within the Plasma workspace, but might make it worse for users who are used to the native file dialog. In 99% of the cases, you want to use QFileDialog instead.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9206#inline-42072" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfilecustomdialogtest.cpp:42</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QVBoxLayout</span> <span style="color: #aa2211">*</span><span class="n">mainLayout</span> <span style="color: #aa2211">=</span> <span class="n">dlg</span><span class="p">.</span><span class="n">findChild</span><span style="color: #aa2211"><</span><span class="n">QVBoxLayout</span> <span style="color: #aa2211">*></span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"mainlayout"</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QVERIFY</span><span class="p">(</span><span class="n">mainLayout</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This isn't really testing anything useful. Rename the layout and the test breaks. Maybe better verify that dlg itself has a layout, rather than "there is a child layout somwhere called mainlayout".</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9206#inline-42073" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfilecustomdialogtest.cpp:47</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QVERIFY</span><span class="p">(</span><span class="n">mFileWidget</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QVERIFY</span><span class="p">(</span><span class="n">dlg</span><span class="p">.</span><span class="n">fileWidget</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QCOMPARE(dlg.fileWidget(), mFileWidget) ? More useful ;)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9206#inline-42071" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfilecustomdialogtest_gui.cpp:26</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">KFileCustomDialogTest_gui</span><span style="color: #aa2211">::</span><span class="n">KFileCustomDialogTest_gui</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9206#inline-42070" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfilecustomdialogtest_gui.h:26</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">class</span> <span class="n">KFileCustomDialogTest_gui</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">unused, remove (remove the whole header...)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9206" rel="noreferrer">https://phabricator.kde.org/D9206</a></div></div><br /><div><strong>To: </strong>mlaurent, mwolff, dfaure<br /><strong>Cc: </strong>Frameworks<br /></div>