[KDE Usability] Review Request 127054: Add a Filter Toolbar Button for Dolphin

[Emmanuel Pescosta]

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




dolphinmainwindow.cpp (line 542)
<https://git.reviewboard.kde.org/r/127054/#comment63995>

    Wouldn't it be better to wire the toggleFilterBar to the toggled signal on only change the visibility state in this slot? 
    
    1. We can avoid the updateViewActions() call which updates not only the filter bar action but many more things
    2. The visibility state of the toggle bar always follows the action toggled state - which is easier to understand than changing the toggled state of his own trigger IMHO



dolphinmainwindow.cpp (lines 1478 - 1488)
<https://git.reviewboard.kde.org/r/127054/#comment63994>

    I still think that we should put this into a new object. 
    
    1. We can avoid the object == action("name") check
    2. We can reuse it outside of the main window


- Emmanuel Pescosta


On March 19, 2016, 5:04 a.m., arnav dhamija wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127054/
> -----------------------------------------------------------
> 
> (Updated March 19, 2016, 5:04 a.m.)
> 
> 
> 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/20160325/1eabc677/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-usability mailing list
kde-usability at kde.org
https://mail.kde.org/mailman/listinfo/kde-usability