Review Request 128972: Properly check Shift toggling in DolphinRemoveAction

[Emmanuel Pescosta]

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


Ship it!




Nice, thanks!

- Emmanuel Pescosta


On Sept. 21, 2016, 12:47 p.m., Elvis Angelaccio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128972/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2016, 12:47 p.m.)
> 
> 
> Review request for Dolphin and Emmanuel Pescosta.
> 
> 
> Bugs: 354301
>     https://bugs.kde.org/show_bug.cgi?id=354301
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> Documentation of `QGuiApplication::keyboardModifiers()` says:
> >The current state is updated sychronously as the event queue is emptied of events that will spontaneously change the keyboard state (QEvent::KeyPress and QEvent::KeyRelease events).
> It should be noted this may not reflect the actual keys held on the input device at the time of calling but rather the modifiers as last reported in one of the above events.
> 
> Since this method is called in DolphinContextMenu's keyPressEvent() and keyReleaseEvent(), the first time that `keyboardModifiers()` is called it doesn't report that shift has been pressed.
> 
> Replacing this method with `queryKeyboardModifiers()` does the job because the latter doesn't care about the event queue.
> 
> Another solution could be adding a `bool isShiftPressed` argument to DolphinRemoveAction::update() and setting it to true in keyPressEvent() and false in keyReleaseEvent(). The problem is that update() is called also in other places, so it would become quite ugly...
> 
> 
> Diffs
> -----
> 
>   src/dolphinremoveaction.cpp c91d74579d76a30c69a1654146ea676bc7eb8e01 
> 
> Diff: https://git.reviewboard.kde.org/r/128972/diff/
> 
> 
> Testing
> -------
> 
> Toggling Shift when the context menu is open now works as expected.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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