Improve search in documentation view: kate-look, hide on ESC, live search
ClosedPublic

Authored by kossebau on Aug 2 2017, 7:31 AM.

Details

Summary

Search-while-typing and closing the search bar on ESC matches
user expectations some more, given behaviour in other current software.
Following the style, text & icons of recent kate searchbar
improves consistency some more.
Ideally typical shortcuts would also work, but those are already
globally registered, thus result in conflicts.
Needs some thinking and more work sadly.

Test Plan

Live search works nicely with different documenation catalogs.

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Aug 2 2017, 7:31 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptAug 2 2017, 7:31 AM

Looks like this:

documentation/documentationview.cpp
54 ↗(On Diff #17539)

Ah, forgot to separate the solving of this TODO in another diff.
Would do when pushing, unless you want that also separated for review.

There is a similar simpler review request here: https://git.reviewboard.kde.org/r/126856/diff/3#index_header
It was already reviewed by Milian Wolff. Feel free to pick anything useful from the old review diff into this one.

There is a similar simpler review request here: https://git.reviewboard.kde.org/r/126856/diff/3#index_header
It was already reviewed by Milian Wolff. Feel free to pick anything useful from the old review diff into this one.

Ah, that's a shame that review request had been ignored since. Seems the last version of that patch pretty much is covered by the patch here, just that this here drops the texts from the searchbar toolbuttons, given they have no use if an icon is set and a tooltip. So good to know we agree basically about what would be an improvement :)

The placeholder text might be worth to copy over, though Kate seems to have none (though Firefox has some). Undecided right now :)

kossebau updated this revision to Diff 17755.Aug 5 2017, 3:39 PM

now also with separator before home button restored

Updated look:

brauch added a subscriber: brauch.Aug 5 2017, 5:13 PM

Hm, sorry, but how do I actually open the search bar in the documentation view? Ctrl+F opens it in the editor for me ... :D

Hm, sorry, but how do I actually open the search bar in the documentation view? Ctrl+F opens it in the editor for me ... :D

Currently one has to use the search button in the docker toolview sadly

I had tried to fix that as well, but the short-cut(s) (tried to use QKeySequence::Find & Co.) would conflict with the one registered globally by the xmlgui mainwindow, as taken from the ktexteditor xmlgui client. I only managed to do that for the escape button, for closing the search bar.

Seems that needs some redesign in whatever way, unless I missed a nice solution.

brauch accepted this revision.Aug 5 2017, 10:38 PM

Looks good to me -- I think we should try to make the search more discoverable though, IMO in the navigation bar up there it looks like it is a global search (i.e. not on the current page). All the other buttons in that bar are for navigation between pages, just this one is for navigation inside the page.

documentation/standarddocumentationview.cpp
187

a bit unconventional naming, I'd simply go for something like "updateSearchPattern"

This revision is now accepted and ready to land.Aug 5 2017, 10:38 PM
This revision was automatically updated to reflect the committed changes.

Looks good to me -- I think we should try to make the search more discoverable though, IMO in the navigation bar up there it looks like it is a global search (i.e. not on the current page). All the other buttons in that bar are for navigation between pages, just this one is for navigation inside the page.

Agreed. No instance idea how to improve that though.

On Chromium and Firefox the search action is in the action menu and also triggerable by key shortcut. Given that we cannot have a key shortcut currently, moving the search action into another menu makes it more cumbersome to activate. For a start we should though at least put it also in the view context menu, which BTW seems currently broken, in not being ours. Will look into fixing that menu now at least.

Qt Assistant has the search button on the toolbar as well, but actually positioned away from the navigation tools, and also menuentry-texted and tooltipped with "Find in Text...". Guess that is something to follow, will give a try.

documentation/standarddocumentationview.cpp
187

Changed it at least to "searchIncremental" which seems more like commonly used lingo, cmp. https://en.wikipedia.org/wiki/Incremental_search

Idea with the naming was to describe the action that is done here. Would agree it still can be improved, but have no clear concept of the whole logic pattern, so lousy at naming, agreed.
Right now I would perhaps name it "updateIncrementalSearch"... perhaps something to improve the next time someone overhauls that code.