Bug 1411402 - Change mouse cursor when hovering/clicking shapes highlighter markers. r=pbro
authorMike Park <mikeparkms@gmail.com>
Wed, 25 Oct 2017 14:36:03 -0400
changeset 444610 4e0bc9f7b713ffe30665cc4c8a5fb1e4daaea97f
parent 444609 993f57169829ffce97249fba7e02787e723dd8a9
child 444611 cfce78c33f60e67a6c2068002e38721047eb3ba8
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1411402
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1411402 - Change mouse cursor when hovering/clicking shapes highlighter markers. r=pbro MozReview-Commit-ID: 9idXQmwatiW
devtools/server/actors/highlighters.css
devtools/server/actors/highlighters/shapes.js
--- a/devtools/server/actors/highlighters.css
+++ b/devtools/server/actors/highlighters.css
@@ -593,16 +593,20 @@
   border: 1px solid var(--toolbar-border);
 
   font: var(--highlighter-font-family);
   font-size: var(--highlighter-font-size);
 }
 
 /* Shapes highlighter */
 
+:-moz-native-anonymous .shapes-root {
+  pointer-events: auto;
+}
+
 :-moz-native-anonymous .shapes-shape-container {
   position: absolute;
   overflow: visible;
 }
 
 :-moz-native-anonymous .shapes-polygon,
 :-moz-native-anonymous .shapes-ellipse,
 :-moz-native-anonymous .shapes-rect,
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -209,16 +209,45 @@ class ShapesHighlighter extends AutoRefr
     return {
       top: dims.top / zoom,
       left: dims.left / zoom,
       width: dims.width / zoom,
       height: dims.height / zoom
     };
   }
 
+  /**
+   * Changes the appearance of the mouse cursor on the highlighter.
+   *
+   * Because we can't attach event handlers to individual elements in the
+   * highlighter, we determine if the mouse is hovering over a point by seeing if
+   * it's within 5 pixels of it. This creates a square hitbox that doesn't match
+   * perfectly with the circular markers. So if we were to use the :hover
+   * pseudo-class to apply changes to the mouse cursor, the cursor change would not
+   * always accurately reflect whether you can interact with the point. This is
+   * also the reason we have the hidden marker-hover element instead of using CSS
+   * to fill in the marker.
+   *
+   * In addition, the cursor CSS property is applied to .shapes-root because if
+   * it were attached to .shapes-marker, the cursor change no longer applies if
+   * you are for example resizing the shape and your mouse goes off the point.
+   * Also, if you are dragging a polygon point, the marker plays catch up to your
+   * mouse position, resulting in an undesirable visual effect where the cursor
+   * rapidly flickers between "grab" and "auto".
+   *
+   * @param {String} cursorType the name of the cursor to display
+   */
+  setCursor(cursorType) {
+    let container = this.getElement("root");
+    let style = container.getAttribute("style");
+    // remove existing cursor definitions in the style
+    style = style.replace(/cursor:.*?;/g, "");
+    container.setAttribute("style", `${style}cursor:${cursorType};`);
+  }
+
   handleEvent(event, id) {
     // No event handling if the highlighter is hidden
     if (this.areShapesHidden()) {
       return;
     }
 
     let { target, type, pageX, pageY } = event;
 
@@ -260,16 +289,17 @@ class ShapesHighlighter extends AutoRefr
           this._handleInsetClick(pageX, pageY);
         }
         event.stopPropagation();
         event.preventDefault();
         break;
       case "mouseup":
         if (this[_dragging]) {
           this[_dragging] = null;
+          this._handleMarkerHover(this.hoveredPoint);
         }
         break;
       case "mousemove":
         if (!this[_dragging]) {
           this._handleMouseMoveNotDragging(pageX, pageY);
           return;
         }
         event.stopPropagation();
@@ -669,16 +699,17 @@ class ShapesHighlighter extends AutoRefr
     let unitX = getUnit(x);
     let unitY = getUnit(y);
     let valueX = (isUnitless(x)) ? xComputed : parseFloat(x);
     let valueY = (isUnitless(y)) ? yComputed : parseFloat(y);
 
     let ratioX = (valueX / xComputed) || 1;
     let ratioY = (valueY / yComputed) || 1;
 
+    this.setCursor("grabbing");
     this[_dragging] = { point, unitX, unitY, valueX, valueY,
                         ratioX, ratioY, x: pageX, y: pageY };
   }
 
   /**
    * Set the inline style of the polygon, replacing the given point with the given x/y
    * coords.
    * @param {Number} pageX the new x coordinate of the point
@@ -747,16 +778,17 @@ class ShapesHighlighter extends AutoRefr
   _handleCircleClick(pageX, pageY) {
     let { width, height } = this.zoomAdjustedDimensions;
     let { percentX, percentY } = this.convertPageCoordsToPercent(pageX, pageY);
     let point = this.getCirclePointAt(percentX, percentY);
     if (!point) {
       return;
     }
 
+    this.setCursor("grabbing");
     if (point === "center") {
       let { cx, cy } = this.coordUnits;
       let cxComputed = this.coordinates.cx / 100 * width;
       let cyComputed = this.coordinates.cy / 100 * height;
       let unitX = getUnit(cx);
       let unitY = getUnit(cy);
       let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
       let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
@@ -828,16 +860,17 @@ class ShapesHighlighter extends AutoRefr
   _handleEllipseClick(pageX, pageY) {
     let { width, height } = this.zoomAdjustedDimensions;
     let { percentX, percentY } = this.convertPageCoordsToPercent(pageX, pageY);
     let point = this.getEllipsePointAt(percentX, percentY);
     if (!point) {
       return;
     }
 
+    this.setCursor("grabbing");
     if (point === "center") {
       let { cx, cy } = this.coordUnits;
       let cxComputed = this.coordinates.cx / 100 * width;
       let cyComputed = this.coordinates.cy / 100 * height;
       let unitX = getUnit(cx);
       let unitY = getUnit(cy);
       let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
       let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
@@ -927,16 +960,17 @@ class ShapesHighlighter extends AutoRefr
   _handleInsetClick(pageX, pageY) {
     let { width, height } = this.zoomAdjustedDimensions;
     let { percentX, percentY } = this.convertPageCoordsToPercent(pageX, pageY);
     let point = this.getInsetPointAt(percentX, percentY);
     if (!point) {
       return;
     }
 
+    this.setCursor("grabbing");
     let value = this.coordUnits[point];
     let size = (point === "left" || point === "right") ? width : height;
     let computedValue = this.coordinates[point] / 100 * size;
     let unit = getUnit(value);
     value = (isUnitless(value)) ? computedValue : parseFloat(value);
     let ratio = (value / computedValue) || 1;
     let origValue = (point === "left" || point === "right") ? pageX : pageY;
 
@@ -1016,71 +1050,82 @@ class ShapesHighlighter extends AutoRefr
       this.hoveredPoint = point ? point : null;
       if (this.hoveredPoint !== oldHoveredPoint) {
         this._emitHoverEvent(this.hoveredPoint);
       }
       this._handleMarkerHover(point);
     }
   }
 
+  /**
+   * Change the appearance of the given marker when the mouse hovers over it.
+   * @param {String|Number} point if the shape is a polygon, the integer index of the
+   *        point being hovered. Otherwise, a string identifying the point being hovered.
+   *        Integers < 0 and falsey values excluding 0 indicate no point is being hovered.
+   */
   _handleMarkerHover(point) {
     // Hide hover marker for now, will be shown if point is a valid hover target
     this.getElement("marker-hover").setAttribute("hidden", true);
-    if (point === null || point === undefined) {
+    // Catch all falsey values except when point === 0, as that's a valid point
+    if (!point && point !== 0) {
+      this.setCursor("auto");
       return;
     }
+    let hoverCursor = (this[_dragging]) ? "grabbing" : "grab";
 
     if (this.transformMode) {
-      if (!point) {
-        return;
-      }
       let { minX, minY, maxX, maxY } = this.boundingBox;
       let centerX = (minX + maxX) / 2;
       let centerY = (minY + maxY) / 2;
 
       const points = [
-        { pointName: "translate", x: centerX, y: centerY },
-        { pointName: "scale-se", x: maxX, y: maxY },
-        { pointName: "scale-ne", x: maxX, y: minY },
-        { pointName: "scale-sw", x: minX, y: maxY },
-        { pointName: "scale-nw", x: minX, y: minY },
+        { pointName: "translate", x: centerX, y: centerY, cursor: "move" },
+        { pointName: "scale-se", x: maxX, y: maxY, cursor: "nwse-resize" },
+        { pointName: "scale-ne", x: maxX, y: minY, cursor: "nesw-resize" },
+        { pointName: "scale-sw", x: minX, y: maxY, cursor: "nesw-resize" },
+        { pointName: "scale-nw", x: minX, y: minY, cursor: "nwse-resize" },
       ];
 
-      for (let { pointName, x, y } of points) {
+      for (let { pointName, x, y, cursor } of points) {
         if (point === pointName) {
           this._drawHoverMarker([[x, y]]);
+          this.setCursor(cursor);
         }
       }
     } else if (this.shapeType === "polygon") {
       if (point === -1) {
+        this.setCursor("auto");
         return;
       }
+      this.setCursor(hoverCursor);
       this._drawHoverMarker([this.coordinates[point]]);
     } else if (this.shapeType === "circle") {
+      this.setCursor(hoverCursor);
+
       let { cx, cy, rx } = this.coordinates;
       if (point === "radius") {
         this._drawHoverMarker([[cx + rx, cy]]);
       } else if (point === "center") {
         this._drawHoverMarker([[cx, cy]]);
       }
     } else if (this.shapeType === "ellipse") {
+      this.setCursor(hoverCursor);
+
       if (point === "center") {
         let { cx, cy } = this.coordinates;
         this._drawHoverMarker([[cx, cy]]);
       } else if (point === "rx") {
         let { cx, cy, rx } = this.coordinates;
         this._drawHoverMarker([[cx + rx, cy]]);
       } else if (point === "ry") {
         let { cx, cy, ry } = this.coordinates;
         this._drawHoverMarker([[cx, cy + ry]]);
       }
     } else if (this.shapeType === "inset") {
-      if (!point) {
-        return;
-      }
+      this.setCursor(hoverCursor);
 
       let { top, right, bottom, left } = this.coordinates;
       let centerX = (left + (100 - right)) / 2;
       let centerY = (top + (100 - bottom)) / 2;
       let points = point.split(",");
       let coords = points.map(side => {
         if (side === "top") {
           return [centerX, top];
@@ -1798,22 +1843,22 @@ class ShapesHighlighter extends AutoRefr
     } else if (this.shapeType === "circle") {
       this._updateCircleShape(width, height, zoom);
     } else if (this.shapeType === "ellipse") {
       this._updateEllipseShape(width, height, zoom);
     } else if (this.shapeType === "inset") {
       this._updateInsetShape(width, height, zoom);
     }
 
-    this._handleMarkerHover(this.hoveredPoint);
-
     let { width: winWidth, height: winHeight } = this._winDimensions;
     root.removeAttribute("hidden");
     root.setAttribute("style",
-      `position:absolute; width:${winWidth}px;height:${winHeight}px; overflow:hidden`);
+      `position:absolute; width:${winWidth}px;height:${winHeight}px; overflow:hidden;`);
+
+    this._handleMarkerHover(this.hoveredPoint);
 
     setIgnoreLayoutChanges(false, this.highlighterEnv.window.document.documentElement);
 
     return true;
   }
 
   /**
    * Update the SVGs for transform mode to fit the new shape.