<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/130179/">https://git.reviewboard.kde.org/r/130179/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Simone Gaiarin.</div>
<p style="color: grey;"><i>Updated July 12, 2017, 8:19 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Removed trailing spaces</p></pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=358139">358139</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=359233">359233</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The current "open with" dialog implementation does not follow the KDE principle "Simple by default, powerful when needed" for the following reasons:
- The current primary goal of the lineedit is to type in a command. Indeed when start typing a drop-down list with command auto-completion appears.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Usability problems:
- The mentioned drop-down list covers the application tree
- It's not possible to search in the application tree, so to find a software it is necessary to guess the category every time</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My patch changes the behavior as follow:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Simple by default:
- The primary goal of the lineedit is to be a search box that allows to filter the applications in the tree
- The command auto-completion is disabled by default</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Powerful when needed
- The command autocompletion can be turned on by advanced users by selecting the preferred autocompletion mode in the right click context menu of the lineedit (this behavior is quite hidden, and I guess most of the normal users don't even know it's possible (I didn't know it until I dig into the code))</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Implementation details:
- The application tree filtering is achieved using a subclass of QSortFilterProxyModel
- When 3 characters are typed in the search box all the tree is expanded (there should be few matching entries at this point)
- When the 3rd character is deleted all the tree is collapsed
- In order to be able to filter, it's necessary to fetch all the tree nodes when the model is created, and not on demand as it was before. This is achieved by adding a fetchAll method. Maybe there is a more elegant way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've tested the implementation, but I'm pretty sure I haven't covered all the possible cases, so a careful testing is necessary.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Relevant discussions:
https://bugs.kde.org/show_bug.cgi?id=359233
https://forum.kde.org/viewtopic.php?f=285&t=131014</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">With and without some text in the line edit:
- Double click on category opens it
- Double click on entry launches the software
- Remember program association for mime works
- Run in terminal works
- Keep open terminal works</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/widgets/kopenwithdialog.cpp <span style="color: grey">(b28c5b05186bc77413524c99ef61b7967b0ec8b9)</span></li>
<li>src/widgets/kopenwithdialog_p.h <span style="color: grey">(52e1bf4febcd5085a12769fc5e9fd614cb490dd6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/130179/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>