[KDE Usability] Review Request 127054: Add a Filter Toolbar Button for Dolphin
Emmanuel Pescosta
emmanuelpescosta099 at gmail.com
Fri Mar 18 17:35:50 GMT 2016
> On March 16, 2016, 8:07 p.m., Emmanuel Pescosta wrote:
> > dolphinmainwindow.cpp, line 1104
> > <https://git.reviewboard.kde.org/r/127054/diff/9/?file=453363#file453363line1104>
> >
> > This solution has some disadvantages, especially that it's not possible to change the filter bar shortcut anymore.
> >
> > A better solution would be to install an event filter [1] on the showFilterBar action. The event filter can then catch the shortcut event [2] and handle it if needed (see the KeyPressEater example in [1] how this can be done).
> >
> > [1] https://doc.qt.io/qt-5/qobject.html#installEventFilter
> > [2] https://doc.qt.io/qt-5/qshortcutevent.html
>
> arnav dhamija wrote:
> Cool solution! But I'm having trouble with implementing DolphinMainWindow::eventFilter(). The action which I am targetting is showFilterBar, but that is only available in the DolphinMainWindow::setupActions() function. Therefore, I cannot pass the QAction to the eventFilter() function.
>
> For example:
>
> bool DolphinMainWindow::eventFilter(QObject *obj, QEvent *event)
> {
> if (obj == ???) { //How should I pass showFilterBar? Should obj==QAction? Shuld I create a new class for the filter barand then check the object?
> if (event->type() == QEvent::Shortcut) {1
> ...
> //use the toggleFocus function and check for the precise shortcut
> return true;
> } else {
> return false;
> }
> }
>
> Emmanuel Pescosta wrote:
> > Shuld I create a new class for the filter barand then check the object?
>
> Yep, please create a new class for the event filter. We can then use the same event filter for the find action ;)
>
> Thomas Lübking wrote:
> I didn't actually try, but the shortcut event should be on every widget, ie. you won't require a filter but can just override ::event(QEvent *e) function of any widget in the window.
>
> You can make showFilterBar a member pointer or look up the action in the dict (but that's rather overheadish...)
>
> arnav dhamija wrote:
> That should take care of it, but I cannot figure out how to get around the fact that
>
> bool DolphinMainWindow::eventFilter(QObject *obj, QEvent *event)
>
> ...would need a QObject while I'm testing for a showFilterBar which is a QAction. How can I get around this problem?
>
> arnav dhamija wrote:
> Overloading the eventFilter function with different function arguements doesn't seem to work either and causes a lot of compilation problems.
>
> arnav dhamija wrote:
> I have figured out how to setup the eventFilter now and hence my previous question is no longer valid :) But, I see the problem is that now the QAction is no longer activatable by a shortcut when I use showFilterBar->installEventFilter(this) although my eventFilter works fine otherwise. Something seems to break shortcuts from working when I do this.
>
> Emmanuel Pescosta wrote:
> We can help you if you upload your latest patch ;)
>
> Thomas Lübking wrote:
> The shortcut event *should* be delivered to every object (or at least widget) in that window - that's why you likely can omit the filter and just re-implement the ::event() function of the window.
>
> Emmanuel Pescosta wrote:
> A short example [1] (not finished: ShortcutFilter should go into his own file, missing documentation, better names, ...) how I would implement it ;)
> I hope it's helpful for you.
>
> > re-implement the ::event()
>
> This would also work as far as I can see, but I think that it has some downsides as well.
> * Re-implementing it in DolphinMainWindow bloats the code of this class even more up (it is already really huge :/) and we need almost the same shortcut handling for the find action (-> code duplication?)
> * Extending the QAction makes it really static (inheritance hierarchy) and so we can't use it for KToggle- and other KWhateverActions -> Composition over inheritance [2] :)
>
> [1] https://paste.kde.org/pyni6m8ol
> [2] https://en.wikipedia.org/wiki/Composition_over_inheritance
> not finished:
Base class ctor invocation is missing as well...
- Emmanuel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127054/#review93616
-----------------------------------------------------------
On March 16, 2016, 5:53 p.m., arnav dhamija wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127054/
> -----------------------------------------------------------
>
> (Updated March 16, 2016, 5:53 p.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
> patch.txt
> https://git.reviewboard.kde.org/media/uploaded/files/2016/03/16/1eb43168-65dd-410b-a74b-f895f3035c83__patch.txt
>
>
> Thanks,
>
> arnav dhamija
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20160318/b0e5dc92/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-usability mailing list
kde-usability at kde.org
https://mail.kde.org/mailman/listinfo/kde-usability
More information about the kfm-devel
mailing list