Review Request 129662: Dolphin: "Show Filter Bar" action should be a toggle

[Sergey Kalinichev]

> On March 1, 2017, 1:45 p.m., Kai Uwe Broulik wrote:
> > This broke keyboard interaction with the filter bar.
> > 
> > I have it configured to always show the filter bar. Now when I press Ctrl+I with the filter bar shown but not focused, it just closes it, rather than moving focus to it...
> 
> Kai Uwe Broulik wrote:
>     I suggest to revert this until we have a better solution as this is a serious usability regression. I'd rather not have a visible checkbox there than breaking a decade of muscle memory.
> 
> Emmanuel Pescosta wrote:
>     +1
>     
>     And here is the better version https://git.reviewboard.kde.org/r/127054/ which I have completely forgotten ...
> 
> Don Nguyen wrote:
>     We should revert then.  I implemented it thinking that was the desired behavior.
> 
> Sergey Kalinichev wrote:
>     I've just stumbled upon this too. Still in master branch, so does anyone planning to revert this?
> 
> Don Nguyen wrote:
>     Do you need me to submit a new patch to revert it or can it be done some other way?

Ok, I've just revert it, hope you don't mind


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129662/#review102691
-----------------------------------------------------------


On Feb. 19, 2017, 1:05 a.m., Don Nguyen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129662/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2017, 1:05 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> The existing code suggests that the show filter bar action should be a
>     KToggleAction.  DolphinViewContainer emits a showFilterBarChanged signal
>     that is connected to DolphinMainWindow::updateFilterBarAction() to call
>     setChecked() on the show filter bar action, which currently does nothing
>     because the show filter bar action is not a checkable action.  I've
>     changed the show filter bar action to a KToggleAction, and updated the
>     code to make it function as a toggle as I believe it was intended.  If
>     this was not the intention, then the existing code related to
>     DolphinMainWindow::updateFilterBarAction() should be removed as they are
>     not doing anything.
> 
> 
> Diffs
> -----
> 
>   src/dolphinmainwindow.h 7003e94 
>   src/dolphinmainwindow.cpp e5103fd 
>   src/dolphinviewcontainer.cpp 4e625c7 
> 
> Diff: https://git.reviewboard.kde.org/r/129662/diff/
> 
> 
> Testing
> -------
> 
> manual testing.  can turn on/off bar with keyboard shortcut, toolbar button.  toolbar button and menu state reflects the status of the filter bar.
> 
> 
> File Attachments
> ----------------
> 
> filter bar off
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/12/16/b38ead2b-910a-4226-b081-24a7c53c7651__filteroff.png
> filter bar on
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/12/16/d702a124-b909-4302-9848-ccc6e6223828__filteron.png
> 
> 
> Thanks,
> 
> Don Nguyen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20170312/e5446423/attachment.htm>