Toggle search and filter bar

Authored by mmustac on Feb 2 2018, 9:48 AM.


Group Reviewers

BUG: 344617

Test Plan

Clicked on the Icons in the toolbar and menu:
1st click: bar appears
2st click: bar disappears again
3rd: Shortcuts toggle also nice

Diff Detail

R318 Dolphin
Lint Skipped
Unit Tests Skipped
mmustac requested review of this revision.Feb 2 2018, 9:48 AM
mmustac created this revision.
ngraham added a subscriber: ngraham.Feb 2 2018, 2:20 PM

This comes up a lot, but always runs into opposition because some people like keeping the search (and especially filter) bars open and switching focus to them using a keyboard shortcut. If the keyboard shortcuts also toggle them open and closed, that workflow breaks, and I predict that same complaints will re-emerge.

I'd like this too, in principle, but it seems like we need to figure out a way around this impasse. Thoughts?

mmustac added a comment.EditedFeb 3 2018, 1:47 PM

Well not really because I can't really imagine what use case this would be and further more when you hit Ctrl + F twice the bar appears again and you can type again. So when anyone know in which situations it would help when the shortcut does not toggle the bar it be great to be enlightened.

My personal use case is:
Hit Ctrl + F
Begin to type to search for files
Hit Ctrl + F again and notice that it not disappear and I have to use the mouse to close it again even it started with a shortcut without any mouse action.

ngraham added a subscriber: broulik.Feb 3 2018, 5:10 PM

Personally I'm fine with it, becasue I don't keep these bars open all the time, but I want to make sure we don't step on anyone'e toes. @broulik?

rkflx added a subscriber: rkflx.EditedFeb 3 2018, 5:36 PM

In my book the keyboard shortcut behaviour is perfectly fine right now in Dolphin, and in line with Kate and Firefox: Ctrl+F will always open or focus the search bar, Enter will execute and move focus away, will close (needs focus).

Regarding the button: I guess toggling makes more sense. However, your patch is still off here: It only performs the toggling action, but the visual state does not indicate you have opened the search bar (like the Preview button does).

Henrik is right. Perhaps a sane compromise is for the button to be a toggle, but for the keyboard shortcut to open and focus on the text field, but not close.

The filter bar indicates the toggle state very nice as it was my plan to be consistent with the preview button ;)
I wanted the same also for the search but was not able to figure it out until then.

Will take another look at this.
And don't know if I will be able to achive the suggested solution but I will try my best.

mmustac updated this revision to Diff 26775.Feb 8 2018, 4:43 PM

Search button indicates now also the toggle state.
Shortcuts still hide search and filter bars ...

Okay I don't know what to do now. I wanted to add a new setting to dolphins general settings menu -> no problem at all.
Then I wanted to extend the find() and showfilterbar() function with an optional parameter so that I would be able to set them active then the shortcut was used (and the option off which would be the default) or toggle them when the button in the toolbar or menu was clicked.
First I tried to extend the connect() call with this parameter but I noticed that this is not so easy possible and then I tought "Is this the right way at all? This will not help at all because the signal will be forwarded anyway if the shortcut was pressed or the button was clicked ..."
My next idea was to make a new Action out of it but I don't want to have two actions in the menus and furthermore the shortcut could only handle one of it which would be really weird and confusing.
And so I am sticked and searching for a way to find out if the call from find() or showfilterbar() was made trough a button or the shortcut to make this different behavior possible. Any ideas?

@mmustac We landed D23232 that supersedes this patch. Can you abandon it?

Restricted Application added a project: Dolphin. · View Herald TranscriptSun, Sep 1, 8:12 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
mmustac abandoned this revision.Mon, Sep 9, 5:50 AM