Review Request 123764: Fix conflict between context menu and toggle action on icons in layers widget

[Victor Wåhlström]

> On May 13, 2015, 7:51 a.m., Boudewijn Rempt wrote:
> > Thanks! As you said, we're sloppy checking for what particular mouse events mean, but I wonder though whether checking for QEvent::ContextMenu in bool KisDocumentSectionView::viewportEvent(QEvent *e) wouldn't help?

If that could be done it would indeed be a better solution for fixing this particular bug, or at least it wouldn't cause a change in behaviour (however minor). This was actually the first thing I tried.

Sadly, ContextMenu events are processed after MouseButtonPress events, so by the time we know that the context menu was triggered, the icon has already been toggled. If I could inspect the QApplication event queue, this may still be feasible; check if there is a ContextMenu event in the queue and early out, but as far as I know there's no way to inspect the event queue (correct me if I'm wrong, because that would be useful!)

Another thought is whether we want arbitrary mouse clicks to toggle buttons/icons? Typical UI convention dictates that only the primary mouse button does this. Looking at the code it appears that Qt::LeftButton is assumed to be the primary button, but if this is defined somewhere I'd much rather use that!

An alternative solution is to check for ContextMenu events and simply discard them if MouseButtonPress has been processed. This would mean that the context menu can't be used while the cursor is above an icon. And as mentioned I'm not sure we want to toggle icons on what is traditionally used as the context menu button, unless we have a valid use-case for that.


- Victor


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


On May 13, 2015, 3:18 a.m., Victor Wåhlström wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123764/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 3:18 a.m.)
> 
> 
> Review request for Calligra and Boudewijn Rempt.
> 
> 
> Bugs: 347541
>     http://bugs.kde.org/show_bug.cgi?id=347541
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch limits mouse click interaction with icons to left mouse button.
> 
> Note: This fix assumes that context menu events are triggered via right mouse button. This is in theory OS specific, but it looks like other code is already making this assumption.
> 
> 
> Diffs
> -----
> 
>   krita/ui/KisDocumentSectionDelegate.cpp 2efde33 
> 
> Diff: https://git.reviewboard.kde.org/r/123764/diff/
> 
> 
> Testing
> -------
> 
> I haven't done extensive testing outside of the layers widget. Worst case it'll disable middle mouse button and right mouse button as alternatives for clicking on icons, which should be fine.
> 
> Context menu events are handled elsewhere and still works as expected.
> 
> 
> Thanks,
> 
> Victor Wåhlström
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150513/6fd757bd/attachment.htm>