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.
Details
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.
Looks like this:
documentation/documentationview.cpp | ||
---|---|---|
54 ↗ | (On Diff #17539) | Ah, forgot to separate the solving of this TODO in another diff. |
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 :)
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.
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" |
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. |