<table><tr><td style="">rkflx requested changes to this revision.<br />rkflx added a comment.<br />This revision now requires changes to proceed.
</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/D12130">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>BUG: 79904</p></blockquote>

<p><a href="https://phabricator.kde.org/p/bruns/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@bruns</a> <a href="https://phabricator.kde.org/p/alexeymin/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@alexeymin</a> I'm not sure you have reviewed this change properly? This bug is about "MacOS styled menus and remote X sessions". If you "Accept" a revision, to keep quality up it is important to check everything (in particular at least read what the bug is about), otherwise please give a "+1" only.</p>

<p><a href="https://phabricator.kde.org/p/ngraham/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@ngraham</a> I don't think we should change the string (at least if text input in the filter is enabled, but  I'd value consistency with the other case too…). The widget is not only about selecting a filetype, but about being able to filter the filenames based on wildcards, much like what you can do in Dolphin. There is a "What's this" help to explain the feature further, but changing it to "File format" hides it even more from discovery, which would be a bad thing and counter your other efforts in this dialog to make things more discoverable.</p>

<p>Also, reading what's available in the filter already makes it pretty obvious that the widget can be used to filter for filetypes too. I doubt users have problems using this only because it says "Filter" on the very left side.</p>

<p>There are some situations where changing the string might be warranted:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">The file dialog does not allow to filter and displays a fixed (non-editable) combobox of filetypes.</li>
<li class="remarkup-list-item">Another (more obvious) UI to filter the view is added.</li>
</ul>

<p>I'm willing to accept the patch if the naming is made more intelligent to account for all situations. Another approach would be to change the API so that applications can override the default "Filter" string. Until that's done, a string which fits all cases has to stay, which is "Filter".</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/D12130#inline-61146">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfilewidget.cpp:586-591</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; ">    <span class="n">whatsThisText</span> <span style="color: #aa2211">=</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"<qt>This is the filter to apply to the file list. "</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                         <span style="color: #766510">"File names that do not match the filter will not be shown.<p>"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                         <span style="color: #766510">"You may select from one of the preset filters in the "</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                         <span style="color: #766510">"drop down menu, or you may enter a custom filter "</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                         <span style="color: #766510">"directly into the text area.</p><p>"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                         <span style="color: #766510">"Wildcards such as * and ? are allowed.</p></qt>"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm talking about this. E.g. an intermediate-level user could filter for <tt style="background: #ebebeb; font-size: 13px;">*ProjectX*CustomerMeetingApril*</tt>, which is not at all about filetypes.</p>

<p style="padding: 0; margin: 8px;">Of course the very simple folder listing in your screenshot does not have the need for filtering like that, but you know very well that in the real world folder listings can get very long, otherwise you would not work on the Places panel ;)</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/D12130">https://phabricator.kde.org/D12130</a></div></div><br /><div><strong>To: </strong>ngraham, Frameworks, VDG, bruns, alexeymin, rkflx<br /><strong>Cc: </strong>rkflx, alexeymin, abetts, bruns, michaelh, ngraham<br /></div>