D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  In D8056#206326 , @simgunz wrote:
  
  > I was thinking to discuss the possibility to make a new KDE UI component 
for collapsible headers so that it can be used in multiple places. Maybe I can 
open a task for discussing this and collect opinions and possible use cases to 
see if it makes sense.
  
  
  That sounds great. This is a pretty common and user-friendly UI paradigm, so 
it would make perfect sense to make it more widely available.
  
  In D8056#206326 , @simgunz wrote:
  
  > My next big goal is try to improve the usability of Okular, especially 
improving the annotations. (I think there are at least 10 bug reports opened by 
me on this). I'll get you involved soon.
  
  
  You're my hero! This is a big tentpole of the Usability & Productivity 
initiative. Take a look at some of the Okular bugs mentioned on 
https://phabricator.kde.org/T6831
  
  A major problem is that Okular (or poppler) often saves annotations in ways 
that don't make them renderable in other viewers, or when printing. See the 
following bugs:
  
  - https://bugs.kde.org/show_bug.cgi?id=383651
  - https://bugs.kde.org/show_bug.cgi?id=275371
  - https://bugs.kde.org/show_bug.cgi?id=378186
  - https://bugs.kde.org/show_bug.cgi?id=390293

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  Great to hear. Hopefully you'll find the right balance between chasing the 
big goals and doing small changes here and there to learn the trade for 
achieving the former ;)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  In D8056#206303 , @rkflx wrote:
  
  > @simgunz Thanks again for your patience (first commit in July!). Hope to 
see more patches from you in the future (both large* like this one, but also 
smaller things**), as you both have a good eye for usability as well as the 
means to implement changes. You should think about applying for commit access 
in tandem with you next patch or the one after that ;)
  
  
  I finished writing my Ph.D. thesis right now. Finally I am a free man again! 
(this is also the reason why it took so long to complete this revision, because 
my commits came spaced by months :-) )
  
  I'll definitely have more free time from this moment on, and I'll be glad to 
contribute.
  
  > *) I know Nate would love to get collapsible headers in Dolphin's sidebar…
  
  I was thinking to discuss the possibility to make a new KDE UI component for 
collapsible headers so that it can be used in multiple places. Maybe I can open 
a task for discussing this and collect opinions and possible use cases to see 
if it makes sense.
  
  > **) Just ask if you need ideas.
  
  I usually have too many, and not enough time. Reason why my big projects are 
swamped, and why I open a lot of bugs without fixing them myself.
  
  I must say I loved the phabricator experience. I always programmed by myself 
my own projects, but discussing how to implement things, having someone to 
review the modifications is why more fun and the quality of the outcome is way 
better.
  
  My next big goal is try to improve the usability of Okular, especially 
improving the annotations. (I think there are at least 10 bug reports opened by 
me on this). I'll get you involved soon.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:e3af26bbb1e7: Improve usability of Open With 
dialog by adding option to filter the… (authored by simgunz, committed by 
rkflx).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=27136=27171

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  In D8056#206303 , @rkflx wrote:
  
  > *) I know Nate would love to get collapsible headers in Dolphin's sidebar…
  
  
  *completely coincidental mention* https://bugs.kde.org/show_bug.cgi?id=389803 
:)
  
  Allow me to echo everything @rkflx said. This is excellent work, and I can't 
wait to see what else you come up with in the future!

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx accepted this revision.
rkflx added a comment.


  @simgunz Thanks again for your patience (first commit in July!). Hope to see 
more patches from you in the future (both large* like this one, but also 
smaller things**), as you both have a good eye for usability as well as the 
means to implement changes. You should think about applying for commit access 
in tandem with you next patch or the one after that ;)
  
  *) I know Nate would love to get collapsible headers in Dolphin's sidebar…
  **) Just ask if you need ideas.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  > - [↓] to move focus (this breaks with the completion enabled, though)
  
  Only for completion: popup. Still works for completion: auto
  
  I have changed the tooltip as suggested. I think we are ready to land it.
  
  Can't wait to see the gif!

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 27136.
simgunz added a comment.


  - Improve tooltip to let user know he can type a command

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=27084=27136

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  I'm super excited to see this happening. Fantastic work, @simgunz, and thanks 
as always for your incredibly thorough review prowess, @rkflx. I'm planning to 
feature this in the coming Sunday's Usability & Productivity post as it it not 
only a //serious// improvement to both, but it should also look really, really 
good in animated gif form. :)

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx accepted this revision.
rkflx added a comment.
This revision is now accepted and ready to land.


  In D8056#205595 , @simgunz wrote:
  
  > Fixed the problems reported by @rkflx  hopefully without introducing new 
ones. I did some testing and now everything seems ok, but please check again.
  
  
  Thanks Simone, works great now.
  
  Successfully tested opening a file with the following methods:
  
  - mouse selection only
  - type a few characters, select with the mouse
  - type a few characers, hit [Enter]
  - double click on entry
  - history dropdown
  - enable completion dropdown, open with custom bin in path
  - open button
  - paste custom bin not in path
  - autocomplete custom path
  - [↓] to move focus (this breaks with the completion enabled, though)
  
  That's basically all the dialog should be able to do that I know of, 
therefore we can land it (once the tooltip is changed).
  
  For improving the usability, I still want to open a task with some ideas to 
discuss them further (really soon™, the first step is done by listing the 
current functionality ↑↑↑). [My promise for Nate's 
https://phabricator.kde.org/D10245 is still open too…]
  
  ---
  
  In D8056#205608 , @simgunz wrote:
  
  > The tooltip of the line edit now says: `Type some text to filter the 
application tree.\nPress down arrow to navigate the results tree.`
  >  I suggest to change it to: `Type to filter the application tree or type 
the name of a command.\nPress down arrow to navigate the results tree.` just to 
avoid having a hidden feature, which is the possibility to type commands. Also 
because the completion mode default is now set to None, so there will not even 
be the completion that suggests the existence of this feature.
  
  
  You are right, the least we can do for now is to improve the tooltip. I don't 
think users know what a "tree" is, so how about this:
  
Type to filter the applications below, or specify the name of a command.
Press down arrow to navigate the results.
  
  Use/adapt it if you like it, I'll wait with landing.
  
  ---
  
  > When you click OK the function `checkAccepted` is called. To me it seems 
that it actually uses the service associated to the selected "Kate" entry 
(`kate -b %U`) and does not execute the command `kate`. 
  >  (That function is quite complicated and I haven't really analyzed it 
completely)
  
  Yeah, that's not ideal (but not too bad since the completion is off by 
default). In my testing the service was also preferred over the executable, 
which is fine because you can explicitly specify the latter by typing the full 
path.

INLINE COMMENTS

> kopenwithdialog.cpp:834-836
> -if (typedExec.isEmpty()) {
> -return false;
> -}

I checked this, but AFAIK it's fine to remove. @dfaure introduced this 10 years 
ago in R446:d6903613f07b 
, 
but I guess we are covered both by the disabled button for empty line edits as 
well as the checks below.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  I rebased to master and updated the review. I hope I didn't messed up 
anything.
  
  In D8056#205621 , @ngraham wrote:
  
  > I was under the impression that the text box gave priority to the app 
filtering unless you checked the semi-hidden option to enable command-line 
completion instead.
  
  
  That is true, but in the moment you type a text, which is also a valid 
command, let's say `kate` you have two things:
  
  - The Kate entry is selected (and so its associated service `kate -b %U` or 
similar)
  - The text box contains a valid binary command `kate` that can be executed
  
  When you click OK the function `checkAccepted` is called. To me it seems that 
it actually uses the service associated to the selected "Kate" entry (`kate -b 
%U`) and does not execute the command `kate`. 
  (That function is quite complicated and I haven't really analyzed it 
completely)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 27084.
simgunz added a comment.


  - Automatically select first match of the filter
  - ArrowDown on lineEdit pass focus to QTreeView
  - Add tooltip to explain how to use the search filter
  - Fix code styling
  - Set appModel and proxyModel together, store proxyModel in data member
  - Avoid casts using pointer to proxyModel
  - Use fixed string case insensitive filter
  - Avoid cast by using member var to proxyModel
  - Initialize m_proxyModel to null pointer
  - Add access method to m_proxyModel, use it to avoid cast
  - Remove unuseful returns, call parent eventFilter if conditions unmatched
  - Fix OK button not disabled when category is selected
  - Fix cannot accept selection if filter line edit is empty

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=27079=27084

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  It's been a while since I tested (could you re-based it on current master?), 
but I was under the impression that the text box gave priority to the app 
filtering unless you checked the semi-hidden option to enable command-line 
completion instead.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  In D8056#205614 , @ngraham wrote:
  
  > interspersing the CLI command and application filtering would be a mistake. 
Say I type `kate`. Am I intending to use the `kate` command, or select the 
"Kate" app? It's ambiguous if we use one text field for both.
  
  
  That is a good point, actually I need to check which of the two has the 
priority now. Given that we were planning to land the revision as it is now (so 
with this ambiguity), don't we need to describe correctly the current behavior? 
Then this can be changed when we figure out a better design for dealing with 
how to input the commands.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  interspersing the CLI command and application filtering would be a mistake. 
Say I type `kate`. Am I intending to use the `kate` command, or select the 
"Kate" app? It's ambiguous if we use one text field for both.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  The tooltip of the line edit now says: `Type some text to filter the 
application tree.\nPress down arrow to navigate the results tree.`
  I suggest to change it to: `Type to filter the application tree or type the 
name of a command.\nPress down arrow to navigate the results tree.` just to 
avoid having a hidden feature, which is the possibility to type commands. Also 
because the completion mode default is now set to None, so there will not even 
be the completion that suggests the existence of this feature.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  Fixed the problems reported by @rkflx  hopefully without introducing new 
ones. I did some testing and now everything seems ok, but please check again.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 27079.
simgunz added a comment.


  - Fix OK button not disabled when category is selected
  - Fix cannot accept selection if filter line edit is empty

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=24488=27079

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx requested changes to this revision.
rkflx added a comment.
This revision now requires changes to proceed.


  I rebased and was just about to land this, but found a serious problem: 
Accepting via OK, double clicking or [Enter] will only work if you have typed 
at least a single character. Otherwise nothing will happen and the dialog just 
stays open.
  
  Can anybody confirm?
  
  Perhaps related: The OK button does get enabled correctly after selecting an 
item. However, it does not get disabled again when selecting a category. Sounds 
like something is still off wrt. tracking what is actually selected.)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  Needs a rebase on top of master, FWIW. Can't wait to have this!

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  In https://phabricator.kde.org/D8056#200013, @simgunz wrote:
  
  > I cannot push so someone needs to do it for me.
  
  
  No problem, I can do it for you.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  Good!
  
  I cannot push so someone needs to do it for me.
  
  Soon, I'll have more free time, and I'll try to polish the code (remove fetch 
more, use Qt 5.10 recursive filtering).

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks, looks ok to me now.
  
  Well, all that code about incremental fetching (canFetchMore/fetchMore) only 
to then fetch everything upfront, could be simplified to just create the full 
tree without going through the fetchMore API... but I'll stop nitpicking ;)
  
  Please wait until the end of this weekend (5.43 tagging) before pushing 
though (new i18n calls).

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  I found some time to implement the suggestions given by @dfaure and I removed 
all the static_cast. I haven't implemented any UI change for now. I think that 
if there are not problems we can land it as it is now, and then improve the UI 
in a second moment (in few months when I'll be free I can work on this more).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 24488.
simgunz marked 8 inline comments as done.
simgunz added a comment.


  - Avoid cast by using member var to proxyModel
  - Initialize m_proxyModel to null pointer
  - Add access method to m_proxyModel, use it to avoid cast
  - Remove unuseful returns, call parent eventFilter if conditions unmatched

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=21990=24488

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  No problem at all :) Good luck with your thesis!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  The deadline of my PhD thesis is very close, so my free time is basically 
zero in this period. Unfortunately, I didn't manage to complete these reviews 
before entering this critical period. In January I should have free time and 
I'll complete this. I also can't wait to complete this.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  @simgunz, how are we doing on addressing the latest review comments? I just 
love this new UI; it's a huge improvement over the status quo IMHO, and it 
would be nice to get it in sometime soon.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  IMHO the problem is not so much in the single text field, but how/where the 
results are presented.
  
  (Sorry I haven't got around to open the task yet. I did not forget, but 
Okular was more important last weekend…)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  If the ability to add a custom path is important, maybe we should put that 
under "Terminal options". I don't think we're ever going to figure out a 
functional UI for that single text field to work equally well for searching and 
entering a path with history.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  In https://phabricator.kde.org/D8056#165453, @simgunz wrote:
  
  > Is it there a way to make my contact info in kde identity public? I 
couldn't find an option for it.
  
  
  There is no such option yet, as far as I know. Turns out as you used `arc` to 
upload your patch (good job!) instead of the web upload, it was already 
included and I would not have needed to ask…
  
  > One last note on this modification. Given that the previous default was to 
use `PopupCompletion`, it means that most of the users out there have this 
setting configured. If there will be any type of announcement or changelog 
regarding this change, it may be useful to explicitly suggest to change to 
completion `None` in order to fully enjoy the new feature without the popup 
covering the tree.
  
  I see were you are coming from, but not sure this would work, would also be a 
bit weird to read. I remember some distro release notes advising on manual 
changes of high importance, but listing every little change would probably be 
too much. The closest would be someone with a blog aggregated on Planet KDE 
featuring this improvement (@ngraham you should totally start a blog, as I 
believe you have a quite good overview on what's happening each week in 
Phabricator land).
  
  Normally such changes can be automated with `kconf_update` 
. Recently, I 
did something similar  
(with a perl detour 
). Instead of 
copying you would have to set a new value explicitly and touch more than one 
file (e.g. `dolphinrc` and everywhere else the dialog is used). You can try 
this direction, but we could also just wait and see what further changes are 
possible for the dialog.
  
  In https://phabricator.kde.org/D8056#165455, @ngraham wrote:
  
  > IMO popup completion is rendered obsolete by the new filter behavior, which 
is much nicer. We might consider just removing it.
  
  
  
  
  In https://phabricator.kde.org/D8056#165599, @dfaure wrote:
  
  > Yes, feel free to kill the hidden setting for completion, especially if the 
default value isn't the one that gives the best user experience.
  
  
  As I wrote above, this would mean two things (I believe both should still be 
supported, in one form or another at least):
  
  - An important use case is to enter a custom path. Having a recent history of 
those paths to avoid typing/selecting them over and over again is useful. The 
history can still be accessed via the arrow on the right, but the completion 
would be gone.
  - There would be no way anymore to autocomplete or discover anything 
contained in your `$PATH` (e.g. your own/custom scripts), safe for typing it 
out in full.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> simgunz wrote in kopenwithdialog.cpp:463
> Segmentation fault happens if I use the member var. Accessing the member var 
> is the cause of the fault, because even `qDebug() << d->m_proxyModel` 
> generates the same error. It is not clear to me why.

Please try moving that code inside `if (d->appModel) { ... }`.

> simgunz wrote in kopenwithdialog.cpp:1108
> Yes, the cascade of return false doesn't make sense. You mean calling 
> eventFilter of QDialog instead of returning false at the end? eventFilter of 
> QDialog is virtual.

Yes.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added inline comments.

INLINE COMMENTS

> dfaure wrote in kopenwithdialog.cpp:463
> Use the member var instead of casting.

Segmentation fault happens if I use the member var. Accessing the member var is 
the cause of the fault, because even `qDebug() << d->m_proxyModel` generates 
the same error. It is not clear to me why.

> dfaure wrote in kopenwithdialog.cpp:830
> Could the view have a method that returns the proxymodel (possibly as a 
> QSortFilterProxyModel* if we don't actually need a subclass-typed pointer), 
> to avoid this cast?
> Such casts make refactoring more difficult, moving problems from compile-time 
> to runtime.

Ok I'll add a method.

> dfaure wrote in kopenwithdialog.cpp:1108
> All this "return false" cascade isn't really needed, a single return false at 
> the end would do the job, or even better, calling the eventFilter method of 
> the base class, just in case that's implemented for other reasons.

Yes, the cascade of return false doesn't make sense. You mean calling 
eventFilter of QDialog instead of returning false at the end? eventFilter of 
QDialog is virtual.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kopenwithdialog.cpp:463
>  
> -if (d->appModel && !d->appModel->isDirectory(current)) {
> -QString exec = d->appModel->execFor(current);
> +QModelIndex sourceCurrent = static_cast *>(model())->mapToSource(current);
> +

Use the member var instead of casting.

> kopenwithdialog.cpp:830
> +//Update the filter regexp with the new text in the lineedit
> +static_cast *>(d->view->model())->setFilterFixedString(d->edit->text());
> +

Could the view have a method that returns the proxymodel (possibly as a 
QSortFilterProxyModel* if we don't actually need a subclass-typed pointer), to 
avoid this cast?
Such casts make refactoring more difficult, moving problems from compile-time 
to runtime.

> kopenwithdialog.cpp:1108
> +}
> +return false;
> +}

All this "return false" cascade isn't really needed, a single return false at 
the end would do the job, or even better, calling the eventFilter method of the 
base class, just in case that's implemented for other reasons.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure added a comment.


  Yes, feel free to kill the hidden setting for completion, especially if the 
default value isn't the one that gives the best user experience.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  IMO popup completion is rendered obsolete by the new filter behavior, which 
is much nicer. We might consider just removing it.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz marked an inline comment as done.
simgunz added a comment.


  > @simgunz If you have no further changes planned for this Diff and 
https://phabricator.kde.org/D8670, I could land both on your behalf. Contact 
information of non-developers are private by default on identity.kde.org, but I 
found some in https://github.com/simgunz/redshift-plasmoid. Are those okay to 
use for the authorship (name + email)?
  
  I don't need to add anything so you can land both. The contact information 
you have found are fine. Is it there a way to make my contact info in kde 
identity public? I couldn't find an option for it.
  
  > Stumbled upon this too some time ago, amazingly you used the very same 
workaround I applied back then. Looking at 
https://en.wikipedia.org/wiki/Esc_key the similar looking glyph seems 
intentional, so I don't see a chance to convince Phab upstream to change this. 
Just embrace and use it, maybe we all get accustomed after a while :) At least 
we get tooltips for all those symbols: [⎋] + [⌘] + [⌥]
  
  Wow. Never saw that glyph before. One never finish learning.
  
  > As for the [↓] and completion popup issues, the root cause seems to be that 
we have one input line but two overlapping "lists", which is really weird 
conceptually and gets complex quite fast as you noticed. A (quick to talk 
about, slow to implement) idea would be to get rid of the completion popup even 
for advanced users and merge those results (recent inputs, but also some other 
search results like `kmimetypefinder` which are not even in a category!?) into 
the list view as a new section.
  > 
  > This definitely needs more thinking, no need to rush…
  
  Yes, doesn't seems trivial. We can talk about it.
  
  One last note on this modification. Given that the previous default was to 
use `PopupCompletion`, it means that most of the users out there have this 
setting configured. And if they are like me, most of them don't even know they 
can change this behavior. If there will be any type of announcement or 
changelog regarding this change, it may be useful to explicitly suggest to 
change to completion `None` in order to fully enjoy the new feature without the 
popup covering the tree.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx accepted this revision.
rkflx added a comment.


  @dfaure Could we get your green (for real, on Phab :) light from a Frameworks 
point of view for this Diff?
  
  @simgunz If you have no further changes planned for this Diff and 
https://phabricator.kde.org/D8670, I could land both on your behalf. Contact 
information of non-developers are private by default on identity.kde.org, but I 
found some in https://github.com/simgunz/redshift-plasmoid. Are those okay to 
use for the authorship (name + email)?
  
  ---
  
  > Just for my mental sanity, what is the remarkup syntax to make the ESC key? 
key Esc or Escape shows a hammer and sickle :-)
  
  Stumbled upon this too some time ago, amazingly you used the very same 
workaround I applied back then. Looking at 
https://en.wikipedia.org/wiki/Esc_key the similar looking glyph seems 
intentional, so I don't see a chance to convince Phab upstream to change this. 
Just embrace and use it, maybe we all get accustomed after a while :) At least 
we get tooltips for all those symbols: [⎋] + [⌘] + [⌥]
  
  > I agree in opening a new task to talk about the proposed points.
  
  Great, I'll try to get this done on the weekend (need to digest your forum 
post and bugzillas first).
  
  As for the [↓] and completion popup issues, the root cause seems to be that 
we have one input line but two overlapping "lists", which is really weird 
conceptually and gets complex quite fast as you noticed. A (quick to talk 
about, slow to implement) idea would be to get rid of the completion popup even 
for advanced users and merge those results (recent inputs, but also some other 
search results like `kmimetypefinder` which are not even in a category!?) into 
the list view as a new section.
  
  This definitely needs more thinking, no need to rush…

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz marked an inline comment as done.
simgunz added a comment.


  @rkflx I agree on all your points.
  
  In general I also prefer to have the categories. Implementing what you 
propose will require a bit of time. Basically if we want any behavior different 
from the current one, we need to replace the `QTreeView` with a `QListView` 
every time some text is typed in the `QLineEdit`.
  
  I agree in opening a new task to talk about the proposed points.
  
  For this patch everything should be ok, now. I also fixed the `QRegExp` point.
  
  Just for my mental sanity, what is the remarkup syntax to make the ESC key? 
key Esc or Escape shows a hammer and sickle :-)

INLINE COMMENTS

> rkflx wrote in kopenwithdialog.cpp:826
> @simgunz This is marked as done, but still uses `QRegExp` and not 
> `QRegularExpression`?

I was using the regular expression only to provide cases insensitive filter, 
but I saw now I can achieve it with fixed string filter as well.

The interface of the method `setFilterRegExp` only accepts `QRegExp`.

Thanks for pointing this out.

> rkflx wrote in kopenwithdialog.cpp:1099
> Is this a "fixme-before-commit" or a "fixme-sometimes-in-the-future" FIXME?

It is a fixme-sometimes-in-the-future. Actually I am not even sure how much of 
a problem that is, so any other opinion is welcomed.

To be more clear on this point. Currently [↓] moves the focus to the tree view, 
while [↑] moves it back in the line edit history of commands. When the search 
provides no results, [↓] moves forward in the history. This is true for any 
completion type.

Before my changes, [↓] was just moving forward in the history of commands.

When we use CompletionPopup or CompletionPopupAuto we want [↓] to let use 
navigate the entries in the popup, so it shouldn't be used to focus on the 
treeview. At this point there are two possibilities:

1. We hit [Enter] and select an entry. The popup closes, and in the tree there 
will be likely a single result if the command chosen has also a desktop entry, 
or no results if the command is a shell command
2. We hit `Esc` and in the line edit there is a piece of text that may match 
something in the tree view

At this point, once the popup is closed, one could use [↓] to focus the tree 
view, but currently [↓] is disabled. So in this sense the behavior is not 
consistent with the cases where other completion modes are used. (Note that 
when the popup  is closed [↓] does not open it again, only typing more text 
opens it)

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 21990.
simgunz marked an inline comment as done.
simgunz added a comment.


  - Use fixed string case insensitive filter

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=21925=21990

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  @abetts  @simgunz If you agree with my ideas in general and also with doing 
those in followup patches, please add a short comment. I'll then open a task on 
some of Phab's workboards which is better suited for tracking the discussion 
than some random comment ;)

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  In https://phabricator.kde.org/D8056#165014, @rkflx wrote:
  
  > That said, I would suggest to just commit this patch as is and work on any 
further improvements in a followup patch
  
  
  +1, no need to get bogged down making it perfect in the first patch. 
Incremental improvement, baby!

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Henrik Fehlauer]

rkflx added a comment.


  Gave this a spin, it's a really nice improvement. As for the categories, I do 
not feel they "get in the way", because:
  
  - In most cases, even after only typing three letters there is only one 
result left, which is already preselected so [Enter] will accept seemlessly.
  - It is actually helpful to also see which category an app is in, as this 
provides confirmation you are on the right track (e.g. if you do "graphics" 
things, you do not want "devel" stuff) as well as disambiguation of similar 
entries.
  - In a way those visual anchors help finding results faster in case the 
resulting list is long, as you can easily move to the next category instead of 
having to scan the complete list of results.
  
  Here is an idea for a compromise which both preserves important context and 
reduces visual noise: Include subtle grouping like in the new type-ahead search 
in System Settings, i.e. instead of a tree, flatten the list and include the 
categories as disabled items / intermediate headers. (Not sure how difficult 
this would be to implement, though.)
  
  At least any duplicates should be removed if you should decide to remove 
categories altogether (e.g. KMail is both in Internet and Office, which would 
be confusing without context). Note the duplication is actually helpful when 
just using categories, and searching in Kickoff already de-duplicates results.
  
  That said, there are some improvements I could imagine would polish this even 
further:
  
  - Expand the categories once the first character is typed. This gives a hint 
on what to type next (e.g. `km` would hint at KMail and KMag instead of just 
showing the categories).
  - Search in the descriptions too (e.g. `browser` would show Firefox and 
Chromium instead of returning nothing).
  - First line of text: Make it clear what is description and what is the item 
being opened, i.e. like on Phab I would not write "install the arc helper" but 
add markup like "install the `arc` helper". Not sure about the best way to do 
this, but using bold text would be a first step (also see screenshot below).
  - It is not discoverable you are allowed to type in a full path to a binary, 
which is important for a lot of third party / proprietary / binary.tar.gz-style 
software not installing proper desktop files and one of the primary use cases 
of this dialog IMHO. Ideas: Add something to the tooltip? Add a gray text (e.g. 
"Type to search or enter full path" – exact formulation could be better) to the 
line edit which is currently just empty?
  
  That said, I would suggest to just commit this patch as is and work on any 
further improvements in a followup patch (just wanted to put this out there to 
give a complete picture).
  
  ---
  
  In addition to the screenshot in https://phabricator.kde.org/D8056#159491, 
the dialog is used when opening `kcmshell5 filetypes` and then navigating 
"text" > "css" > Application Preference Order > Add…. Note the bold text 
(although I wish the colon in front of it could go):
  
  F5478956: app-chooser.png 
  
  I'm showing this because there is actually a mention of "If the program is 
not listed…". In the end both dialogs should be consistent in intro text, line 
edit background text and tooltip help text. I'd be happy to help implementing 
any adaptations in wording once we've reached a conclusion here.

INLINE COMMENTS

> dfaure wrote in kopenwithdialog.cpp:826
> Please use QRegularExpression; QRegExp is deprecated.
> 
> Although, if this is about FixedString, why not just use setFilterFixedString?
> Regexps are slow, better avoid them when not necessary.

@simgunz This is marked as done, but still uses `QRegExp` and not 
`QRegularExpression`?

> kopenwithdialog.cpp:1099
> +// FIXME: Disable arrow down in CompletionPopup and 
> CompletionPopupAuto only when the dropdown list is shown.
> +// When popup completion mode is used the down arrow is used to 
> navigate the dropdown list of results
> +if (combo->completionMode() != KCompletion::CompletionPopup && 
> combo->completionMode() != KCompletion::CompletionPopupAuto) {

Is this a "fixme-before-commit" or a "fixme-sometimes-in-the-future" FIXME?

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  I have fixed what dfaure suggested. I couldn't remove the cast at line 463 
because a Segmentation fault happens. It is not clear to me why exactly.
  
  Regarding removing the categories I need to work on that, because it is not 
trivial apparently. I thought it was enough to remove line 399-404, but in that 
case the treeview is almost empty once opened (only the "Help" item is shown).
  
  I run out of time for today, I'll work on that soon.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 21925.
simgunz marked 4 inline comments as done.
simgunz added a comment.


  - Fix code styling
  - Set appModel and proxyModel together, store proxyModel in data member
  - Avoid casts using pointer to proxyModel

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=21261=21925

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Andres Betts]

abetts added a comment.


  In https://phabricator.kde.org/D8056#164287, @ngraham wrote:
  
  > I would say make the categories disappear as soon as that view displays 
search results.
  
  
  Yeah, because in the end, when you want to narrow down the results, the 
categories will get in your way to select the item you want.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  I would say make the categories disappear as soon as that view displays 
search results.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  Can you be more specific?
  
  Currently, the categories are expanded automatically after 3 characters have 
been typed.
  
  Do you want to make the categories disappear at this point? Or not having 
them from the beginning?

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  In https://phabricator.kde.org/D8056#164283, @abetts wrote:
  
  > Can I suggest to maybe simplify even a little more and remove the category 
separators? AKA, system, internet?
  
  
  +1 totally agree.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Andres Betts]

abetts added a comment.


  Can I suggest to maybe simplify even a little more and remove the category 
separators? AKA, system, internet?

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz marked 9 inline comments as done.
simgunz added a comment.


  I fixed most of them. I think I'll have time tomorrow.

INLINE COMMENTS

> dfaure wrote in kopenwithdialog.cpp:398
> Ah. Filipe and I implemented recursive filtering in QSortFilterProxyModel but 
> that's only in Qt 5.10. Too bad ;) We'll have to keep this for now then 
> (easier here since the data doesn't change, anyway)

I saw the new feature in Qt 5.10 indeed.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  @simgunz have you had a chance to work on @dfaure's comments? It would be 
great to get this in.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure added a comment.


  Looks good, just some coding style issues.

INLINE COMMENTS

> kopenwithdialog.cpp:195
>  d->fillNode(QString(), d->root);
> +for (int i=0; i +fetchAll(index(i,0));

spaces around '=' and '<'

I would put rowCount() into a local var, the compiler can't optimize away that 
call.

> kopenwithdialog.cpp:196
> +for (int i=0; i +fetchAll(index(i,0));
> +}

space after comma

> kopenwithdialog.cpp:274
> +}
> +return;
> +}

remove useless statement

> kopenwithdialog.cpp:382
> +QSortFilterProxyModel(parent)
> +{
> +}

weird indentation, please indent these to column 0

> kopenwithdialog.cpp:398
> +
> +//Show the non-leaf node also if the regexp matches one one of its 
> children
> +int rows = sourceModel()->rowCount(index);

Ah. Filipe and I implemented recursive filtering in QSortFilterProxyModel but 
that's only in Qt 5.10. Too bad ;) We'll have to keep this for now then (easier 
here since the data doesn't change, anyway)

> kopenwithdialog.cpp:440
>  
> -d->appModel = qobject_cast(model);
> +d->appModel = qobject_cast *>(static_cast(model)->sourceModel());
>  if (d->appModel) {

Urgh. Why don't we change this method to be setModels(QSortFilterProxyModel *, 
KApplicationModel *) to avoid all these casts?

> kopenwithdialog.cpp:451
>  QModelIndex index = selectionModel()->currentIndex();
> +index = static_cast *>(model())->mapToSource(index);
>  return d->appModel->isDirectory(index);

... and keep a QSFPM * member variable to avoid these casts here.

> kopenwithdialog.cpp:479
> +if (indexes.count() == 1) {
> +if(!d->appModel->isDirectory(indexes.at(0))) {
> +QString exec = d->appModel->execFor(indexes.at(0));

space after if

> kopenwithdialog.cpp:819
> +// otherwise changing text but hitting the same result clears curService
> +bool selectionEmpty = !( d->view->currentIndex().row() >= 0);
> +if (d->curService && selectionEmpty) {

I think this would be more readable as

  const bool selectionEmpty = !d->view->currentIndex().isValid();

> kopenwithdialog.cpp:826
> +//Update the filter regexp with the new text in the lineedit
> +static_cast *>(d->view->model())->setFilterRegExp(QRegExp(d->edit->text(),
> +
> Qt::CaseInsensitive, QRegExp::FixedString));

Please use QRegularExpression; QRegExp is deprecated.

Although, if this is about FixedString, why not just use setFilterFixedString?
Regexps are slow, better avoid them when not necessary.

> kopenwithdialog.cpp:830
> +//Expand all the nodes when the search string is 3 characters long
> +//If the search string doesn't match anything there will be not nodes to 
> expand
> +if (d->edit->text().size() > 2) {

s/not/no/ ?

> kopenwithdialog.h:137
>  private:
> +bool eventFilter(QObject *object, QEvent *event);
> +

add "override" at the end

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  > Can we not just remove the drop down menu and always show the checkbox for 
terminal?
  
  Actually I've just changed that behaviour few months ago to follow the 
"simple by default, powerful when needed" philosophy. See this review and the 
related bug for the details:
  https://git.reviewboard.kde.org/r/130180/
  https://bugs.kde.org/show_bug.cgi?id=359233

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Roman Gilg]

subdiff added a comment.


  In https://phabricator.kde.org/D8056#159636, @fabianr wrote:
  
  > Not really on topic, but could you change the label for the "Advanced 
options" to something more meaningful, eg "Terminal options" ? The HIG 
recommends not to use the label "Advanced options", but " Use a descriptive 
label so that it reflects the functionality. "
  
  
  Can we not just remove the drop down menu and always show the checkbox for 
terminal?
  
  @simgunz Please do the change to this though in a separate patch. And 
regarding your patch here please give @dfaure a few days to review it as well. 
Also @abetts should give his ok from VDG.
  
  I personally like the change. +1

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  @ngraham Great! I don't have commit access.
  
  @fabianr I agree on changing that label to "Terminal options". I'll send a 
new revision request for that.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  This is fantastic work. I'm running with it full time now and can't imagine 
going back to the old one. Huge +1 from me. Do you have commit access?

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Fabian Riethmayer]

fabianr added a comment.


  Not really on topic, but could you change the label for the "Advanced 
options" to something more meaningful, eg "Terminal options" ? The HIG 
recommends not to use the label "Advanced options", but " Use a descriptive 
label so that it reflects the functionality. "

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: fabianr, abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  Ok. Now it should work properly.
  
  I've also added a tooltip to the search box to explain the down arrow feature 
so that it is not hidden.
  The current text is: "Type some text to filter the application tree.\nPress 
down arrow to navigate the results tree."

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz updated this revision to Diff 21261.
simgunz added a comment.


  - Automatically select first match of the filter
  - ArrowDown on lineEdit pass focus to QTreeView
  - Add tooltip to explain how to use the search filter

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8056?vs=20087=21261

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog.h
  src/widgets/kopenwithdialog_p.h

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  Here is a picture:
  
  F5448036: openwith_filter.png 

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Andres Betts]

abetts added a comment.


  Can you guys add a quick picture of what this looks like right now?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: abetts, ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  Sounds great, thanks!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  In https://phabricator.kde.org/D8056#158939, @ngraham wrote:
  
  > @simgunz, have you had a chance to try implementing David's suggestion?
  
  
  Yes. Everything seems to work but I haven't pushed yesterday because I wanted 
to do an extra test.
  
  The current behavior is the following:
  
  - Down arrow pass the focus to the QTreeView and moves the selection down by 
one. (alternatively we can just pass the focus without changing the selection).
  - Up arrow is used to go back in the history of the combobox (as the current 
behavior)
  - Making down arrow change the focus, partially breaks it functionality of 
navigating the history of the combo box.
  - If the filter doesn't match anything, down arrow doesn't change the focus 
and works for navigating the history
  
  - The feature is not enabled if PopupAutocomplete is enabled. Theoretically 
it should only be disabled while the popup is shown, but I couldn't find a way 
to detect this state.
  
  Moreover the first match is selected by default and hitting enter calls it. 
To enable this feature I had to change the behaviour so that the text box is 
not filled anymore with the command corresponding to the selected desktop 
entry. For me this is even cleaner from the UI point of view.
  
  I'll push tomorrow evening.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  @simgunz, have you had a chance to try implementing David's suggestion?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[David Faure]

dfaure added a comment.


  You can send (forward) a key event to a widget (like the QTreeView) using 
qApp->sendEvent(), rather than trying to call the protected method 
keyPressEvent() directly.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  No worries, take your time. :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  In https://phabricator.kde.org/D8056#155537, @ngraham wrote:
  
  > Any progress? This is too good a change to not merge. :)
  
  
  Today I finally had the time to work on this and I've implemented the "select 
first result by default" and it seems to work nicely.
  
  I started working on how to  make up/down arrows move in the treeview while 
maintaining the focus on the text field. I thought of using an eventFilter to 
intercept those two keys and pass them to the QTreeView using the 
keyPressEvent() method. The problem is that I get an error on the fact that the 
method is protected. For now I've put the eventFilter method in the class 
KOpenWithDialog since it is the only one deriving QObject, but I'm not sure it 
is the right place.
  
  So I need to work on this a little bit more, and I'm afraid I won't have time 
next week because I am on a trip. So it needs to wait a bit more.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  Any progress? This is too good a change to not merge. :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  > I see what you mean. How about implementing "down-arrow-changes-focus" only 
when not using dropdown completion?
  
  Seems ok. I'll implement it this way soon.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added a comment.


  In https://phabricator.kde.org/D8056#151057, @simgunz wrote:
  
  > > - When the text field has focus, make the down arrow key move to the list 
of found results so that you can select one with the keyboard and not have to 
use the mouse at all
  >
  > I like the idea and in my personal opinion it should behave as you suggest, 
but before implementing it I want to be sure not to break other patterns people 
may use. In particular, the text box can have different autocompletion mode 
selected by the user, among which dropdown completion. This requires the use of 
the down arrow key, implementing your idea will break this behavior. Is it 
tolerable?
  
  
  I see what you mean. How about implementing "down-arrow-changes-focus" only 
when not using dropdown completion?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz added a comment.


  > - When the text field has focus, make the down arrow key move to the list 
of found results so that you can select one with the keyboard and not have to 
use the mouse at all
  
  I like the idea and in my personal opinion it should behave as you suggest, 
but before implementing it I want to be sure not to break other patterns people 
may use. In particular, the text box can have different autocompletion mode 
selected by the user, among which dropdown completion. This requires the use of 
the down arrow key, implementing your idea will break this behavior. Is it 
tolerable?
  
  > - Pre-select the top result in the list of found results so that you can 
immediately hit the return key to select that application
  
  I think this can be implemented independently of the previous point. We 
select the first result, and hitting enter will use that result even if the 
focus is still in the textbox. So no matter wether we decide to implement point 
1, this should work.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  I gave this a shot and it's an excellent change--a huge improvement over the 
status quo. I have two suggestions for further ergonomic refinement:
  
  - When the text field has focus, make the down arrow key move to the list of 
found result so that you can select one with the keyboard and not have to use 
the mouse at all
  - Pre-select the top result  in the list so that you can immediately hit the 
return key to select that application

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: ngraham, alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Nathaniel Graham]

ngraham added reviewers: Frameworks, VDG.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure, #frameworks, #vdg
Cc: alexeymin, #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz edited the test plan for this revision.
simgunz added a reviewer: dfaure.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8056

To: simgunz, dfaure
Cc: #frameworks

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

[Simone Gaiarin]

simgunz created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  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.
  
  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
  
  My patch changes the behavior as follow:
  
  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
  
  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))
  
  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.
  
  I've tested the implementation, but I'm pretty sure I haven't covered all the 
possible cases, so a careful testing is necessary.
  
  Relevant discussions:
  https://bugs.kde.org/show_bug.cgi?id=359233
  https://forum.kde.org/viewtopic.php?f=285=131014

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

REVISION DETAIL
  https://phabricator.kde.org/D8056

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp
  src/widgets/kopenwithdialog_p.h

To: simgunz
Cc: #frameworks