Fix issues with cursors and unwanted actions when right-clicking
ClosedPublic

Authored by rkflx on Jun 25 2018, 9:44 PM.

Details

Summary

After showing and closing the context menu by right-clicking in
View mode, the cursor would be set to ClosedHandCursor. Only
after another click the cursor would be set back to the regular
OpenHandCursor.

The issue can be fixed by not changing the cursor shape in the first
place for showing the context menu, i.e. when right-clicking.

While using the Red Eye Reduction tool, the patch will prevent
the unwanted placement of corrections on the canvas when right-clicking.

Fullscreen mode could be toggled by double-clicking the middle
mouse button, but it should only work for the left button.

Test Plan

Right-click on an image in View mode to show the context menu and
dismiss it again. The cursor should look as before. Also try right-clicking
over the Bird's eye view and in the Crop tool.

In the Red Eye Reduction tool, right-clicking should not change the
position of the correction anymore.

Entering Fullscreen should only work for double-clicking with the left
mouse button. Note that accidentally clicking on the Fullscreen checkbox
of the context menu when right-clicking and moving the cursor slightly
might still be possible, but having that action as the first entry is
more important than the very minor imperfection in that case.

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx requested review of this revision.Jun 25 2018, 9:44 PM
rkflx created this revision.
rkflx added a comment.Jul 7 2018, 11:09 AM

cursor/fix-context-menu (branched from Applications/18.04)

Friendly reminder that tagging is on Monday at 23:59 UTC.

huoni added a subscriber: huoni.Jul 8 2018, 12:42 AM

cursor/fix-context-menu (branched from Applications/18.04)

Friendly reminder that tagging is on Monday at 23:59 UTC.

Hopefully I got here in time!

LGTM.

For the Red Eye Reduction tool this will at the same time prevent
the unwanted displacement of previously drawn corrections.

I can't reproduce this problem. Previously drawn corrections move with the image, and I couldn't replicate any situation where the correction was drawn displaced or the image unintentionally dragged.

Side note: I think we should also check for left mouse button for double-click fullscreen :)

huoni accepted this revision.Jul 8 2018, 12:42 AM
This revision is now accepted and ready to land.Jul 8 2018, 12:42 AM
rkflx updated this revision to Diff 37396.EditedJul 8 2018, 10:40 PM
rkflx retitled this revision from Fix wrong cursor after showing context menu to Fix issues with cursors and unwanted actions when right-clicking.
rkflx edited the summary of this revision. (Show Details)
rkflx edited the test plan for this revision. (Show Details)

Hopefully I got here in time!

No problem, thanks for your reviews. I was mainly after the trivial fixes for the next stable release, but having the master patches reviewed is awesome too. Only https://phabricator.kde.org/D13894 left, or maybe Peter can look into it on Monday…

For the Red Eye Reduction tool this will at the same time prevent
the unwanted displacement of previously drawn corrections.

I can't reproduce this problem. Previously drawn corrections move with the image, and I couldn't replicate any situation where the correction was drawn displaced or the image unintentionally dragged.

So you don't experience the following bug without the patch?:

It might be that the description in my summary was a little misleading, I adapted it slightly now.

Side note: I think we should also check for left mouse button for double-click fullscreen :)

Good idea, added it to the patch.

huoni added a comment.Jul 9 2018, 9:35 AM

It might be that the description in my summary was a little misleading, I adapted it slightly now.

The video makes it clear, and indeed I misunderstood. I do experience the same thing.

Side note: I think we should also check for left mouse button for double-click fullscreen :)

Good idea, added it to the patch.

Nice

This revision was automatically updated to reflect the committed changes.