Move from the searchbox to the search results listbox/view using the
down arrow key in addition to the existing methods using the tab key,
return key, or the mouse.
Details
- Reviewers
ngraham - Group Reviewers
Dolphin - Commits
- R318:196f4553e68a: Move from the searchbox to the results with the down arrow key
use ctrl+f to search in a directory tree, press down arrow key
to go to the results
Diff Detail
- Repository
- R318 Dolphin
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
As far as I can tell, currently the focus will change from the searchbar to the view when you press <Enter>.
Would that be an acceptable workflow?
Thanks! That indeed appears to be working even without the patch. But I couldn't figure it out by myself. Using Enter is an acceptable workflow as far as I'm concerned, but my patch may be indicative of a UI issue.
Given that the down arrow key currently does nothing here, I think that this makes sense conceptually. I'm not a huge fan of this setView() function though. Instead, pressing the down arrow key should emit focusViewRequest(); This is how the filter bar implements this functionality.
src/search/dolphinsearchbox.cpp | ||
---|---|---|
223 | don't assert; instead check for m_view being valid, and if it isn't, just do nothing. |
Thanks, this is much better technically.
One thing I notice is that moving the focus to the view using focusViewRequest() doesn't actually make an item become pre-selected if it wasn't before. Given that the whole point of this feature is to quickly select an item in the view after filtering or searching, maybe it would also make sense to create a new function that will do that which we can call from the filter bar and search field when they request to move focus to the view.
Then again personally I kind of think that focusing the main view should *always* select an item, since otherwise that's just a manual step the user has to do for themselves. In this case, we would have DolphinViewContainer::requestFocus() always automatically select the item with implicit focus, if it's not already selected.
What do you think, @elvisangelaccio?
I agree. And this behavior should be also implemented for the Return case which is lacking this at the moment.
A requestFocusAndSelectFirstItem for instance.
@ngraham : hi! I am interested in finishing it up, but will be fine with someone else doing that. I'll try to get to it soon.
@ngraham @meven : here is a new patch: https://www.shlomifish.org/Files/files/code/dolphin--shlomif-patch-D26362--modified-v0.2.0.patch but note that it makes no difference and om fedora 32 x64 it seems that pressing enter twice invokes the top item in the results which was already selected. It also can be done as "down arrow; enter" using either of my patches.
Thanks, but you need to update the diff using arc rather than providing a link to a new patch file.
@ngraham : well, I didn't want to override the existing patch because the new one does not provide extra functionality - it is just more complex code-wise
So what's the status of this patch and the relationship between it and the other one you've mentioned? I'm confused.
Well, the new patch was an attempt at revising the patch tracked by this phabricator ticket for implementing your commentary that it should make an item in the search results become pre-selected . However, this seems to also be the case while using the phabricator tracked patch and the new path makes no difference, so it is safe to ignore the https-hyperlinked-linker and I just shared it for completeness sake.
Actually I see a change that's needed: when the return key is pressed, it triggers DolphinSearchBox::returnPressed(), but when the down arrow key is pressed, it triggers DolphinSearchBox::focusViewRequest() And I notice that returnPressed() emits the search result too. So now with this patch we have two inconsistent codepaths to get to the same thing. We should probably unify these so that both trigger your new focusViewRequest() signal, and make sure that it emits the search result too.
@ngraham : I applied the change now, but was unable to build and test the change due to git master requiring newer KDE deps. I hope it works.