Bug 1411402 - Change mouse cursor when hovering/clicking shapes highlighter markers. r=pbro draft
authorMike Park <mikeparkms@gmail.com>
Wed, 25 Oct 2017 14:36:03 -0400
changeset 696443 ca87f2b2c3404e3bf119313363751bec50010bf3
parent 696426 b0ded9bbd517b1cef61beef4245323d1441fedc3
child 696459 7d19129874be7830ec99bfa78a3b2145f9919c8d
child 697759 3c94b4e21127396447831a07f176c17f8f549463
push id88719
push userbmo:mpark@mozilla.com
push dateFri, 10 Nov 2017 18:40:14 +0000
reviewerspbro
bugs1411402
milestone58.0a1
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.