Closed Bug 1450685 Opened 2 years ago Closed 2 years ago

The cursor remains an open hand when rotate point is clicked [Shape tool]

Categories

(DevTools :: Inspector, defect, P3, trivial)

defect

Tracking

(firefox61 verified, firefox62 verified)

VERIFIED FIXED
Firefox 61
Tracking Status
firefox61 --- verified
firefox62 --- verified

People

(Reporter: david.olah, Assigned: rcaliman, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

[Affected versions]: 
Nightly 61.0a1

[Affected platforms]:
Platforms: Windows 10 x 64, Windows 7 x 32, Mac OS X 10.12 and Ubuntu 16.04 x64.

[Steps to reproduce]:
1. Go to about:config and set "layout.css.shape-outside.enabled" to true.
2. Go to http://labs.jensimmons.com/2016/examples/shapes-3.html
3. Right click on the image and then click on "Inspect Element".
4. Ctrl + Click on the button displayed next to clip-path to toggle the shape tool on
5. Click on the middle point from above (the one that rotates the shape)

[Expected result]:
- The open hand cursor turns into a closed hand cursor

[Actual result]:
- The open hand cursor doesn't turn into a closed hand cursor
This should be a fairly simple bug for someone who wants to start contributing to Mozilla and DevTools in particular.
We have contribution docs here that should help get started: http://docs.firefox-dev.tools/
Once ready to fix, the files that you will need to modify are:
- the Shapes editor JavaScript code: /devtools/server/actors/highlighters/shapes.js
- the css code for it: /devtools/server/actors/highlighters.css (code starts after the /* Shapes highlighter */ section).

Razvan Caliman is currently the person who knows the most about this code, so I have marked him as a mentor on this bug.
If you are interested in fixing this, feel free to use the needinfo? flag to ask questions and either him or I can help you get started!
Mentor: rcaliman
Keywords: good-first-bug
Priority: -- → P3
First time contributor to mozilla here... I will pick it up. Thanks :)
Great, thank you for your interest. Feel free to ask any questions you may need, and submit a patch here for review.
Handling events in this implementation may look daunting and counter-intuitive, but there's a good reason for that. You'll notice that instead of event handlers attached to each element of the editor, there are global event handlers for "mousedown", "mouseup", "mousemove" and so forth. In each handler, there are checks to see which element of the editor exists at the x/y position of the mouse cursor, then appropriate action is taken. 

The main reason is that when moving a point, the cursor can move quicker than the UI updates. In that case, if handlers were attached directly to elements, rapid "mouseout" / "mouseover" event succession would occur with undesired effects. In this implementation, the cursor is actually set on a parent of the element to mitigate such problems.

The solution for this bug is straightforward:

1. Inside _handlePolygonTransformClick(), after setting this[_dragging], there needs to be a call to: this._handleMarkerHover(this.hoveredPoint)

Calling after this[_dragging] is set is important, because this[_dragging]'s content is used inside in the hover method.

2. Inside _handleMarkerHover(), in the `points` array, the entry for "rotate", needs to be adjusted from:
{ pointName: "rotate", x: rotatePoint[0], y: rotatePoint[1], cursor: "grab" }
to
{ pointName: "rotate", x: rotatePoint[0], y: rotatePoint[1], cursor: hoverCursor }

Basically, it uses either "grab" or "grabbing" depending on the existence of this[_dragging] which was set in the previous method.

3 Inside handleEvent(), for the "mouseup" case, the call to handleMarkedHover needs to be adjusted from:
this._handleMarkerHover(this.hoveredPoint);
to
this._handleMarkerHover(null);

This last one is an enhancement, but helps with the UX. When dragging, the cursor can easily move away from the rotation handle. When the click is released, the handle still mistakenly shows up as hovered even if the cursor is far away from it. The null in that call will trigger a correct UI update.
Assignee: nobody → rcaliman
Comment on attachment 8971313 [details]
Bug 1450685 - Ensure correct cursor is shown when rotating a polygon.

https://reviewboard.mozilla.org/r/240080/#review246062
Attachment #8971313 - Flags: review?(pbrosset) → review+
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e1f60508407
Ensure correct cursor is shown when rotating a polygon. r=pbro
https://hg.mozilla.org/mozilla-central/rev/0e1f60508407
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Build ID: 20180508231737
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0

Verified as fixed on Firefox Nightly 62.0a1 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Build ID: 20180507191226
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0

Verified as fixed on Firefox 61.0b3 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.