Review Request 127054: Add a Filter Toolbar Button for Dolphin

[Thomas Lübking]

> On März 14, 2016, 11 vorm., Thomas Lübking wrote:
> > dolphinviewcontainer.cpp, line 320
> > <https://git.reviewboard.kde.org/r/127054/diff/7/?file=450012#file450012line320>
> >
> >     tarzan speech - try "filterBarHasFocus"
> >     
> >     nb. that this shortcut solution only works as long as any non-shortcut action trigger (toolbutton, menu) acquires the input focus on interaction.
> >     
> >     That's not guaranteed and should at least be commented as a caveat.
> 
> arnav dhamija wrote:
>     I have made this change and I have included a warning, but I don't understand why a non-shortcut action trigger won't guarantee setting the focus to the filter bar. Why won't setFocus() always work?

No, that's a misunderstanding (and incredibly bad wording from my side ;-)

Show the filter (no matter how), focus something different (eg. the url editor or the icon view) and click the toggle button.
The filter will not hide.

Now assume any action trigger would require to move the focus away from the filter to be invoked - the trigger could never toggle (it would unfocus the filter, the code figure the filter isn't focused and move the focus there instead of hiding it as could be expected from a toggling toolbutton)

The correct™ (Thomas P.?) behavior would be to only move the focus to a visible filter if the action is triggered by the shortcut, what however requires forementioned event handler to check what triggered the action (whether it's a shortcut, to be precise)


- Thomas


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


On März 15, 2016, 9:47 vorm., arnav dhamija wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127054/
> -----------------------------------------------------------
> 
> (Updated März 15, 2016, 9:47 vorm.)
> 
> 
> Review request for Dolphin, KDE Usability and Emmanuel Pescosta.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> This patch was created to implement the feature suggested here: [https://todo.kde.org/?controller=task&action=show&task_id=966](https://todo.kde.org/?controller=task&action=show&task_id=966)
> 
> This patch adds a Filter Button to the toolbar of Dolphin. This button can be toggled to show/hide the Filter Bar.
> 
> In this patch, one might notice that there is now show_filter_bar and show_filter_bar_button in dolphinui.rc file. This was done because I found that using the same action for the menu entry and the toolbar entry would cause something to break (eg, shortcuts would stop working). 
> 
> A screenshot of the toolbar is also attached.
> 
> 
> Diffs
> -----
> 
>   dolphinmainwindow.h 7003e94 
>   dolphinmainwindow.cpp f7a7613 
>   dolphinviewcontainer.h 62f9110 
>   dolphinviewcontainer.cpp 8fea3ba 
> 
> Diff: https://git.reviewboard.kde.org/r/127054/diff/
> 
> 
> Testing
> -------
> 
> manual
> 
> 
> File Attachments
> ----------------
> 
> dolphintoolbar.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/02/12/d936e5d0-557c-45c5-9a2d-f849e5d284a6__dolphintoolbar.png
> 
> 
> Thanks,
> 
> arnav dhamija
> 
>

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