Bug 1449568 - Grow the arrow box if it's partly hidden, and move the text to be visible; r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 22 May 2018 14:23:11 +0200
changeset 804040 3d542dce4195b673f4db0f18b51d889447c9a350
parent 803743 d8f180ab74921fd07a66d6868914a48e5f9ea797
push id112290
push userbmo:pbrosset@mozilla.com
push dateTue, 05 Jun 2018 13:06:57 +0000
reviewersgl
bugs1449568
milestone62.0a1
Bug 1449568 - Grow the arrow box if it's partly hidden, and move the text to be visible; r=gl MozReview-Commit-ID: EakAejm0Lhw
devtools/server/actors/highlighters/css-grid.js
devtools/server/actors/highlighters/utils/canvas.js
--- a/devtools/server/actors/highlighters/css-grid.js
+++ b/devtools/server/actors/highlighters/css-grid.js
@@ -66,19 +66,24 @@ const GRID_LINES_PROPERTIES = {
   }
 };
 
 const GRID_GAP_PATTERN_WIDTH = 14; // px
 const GRID_GAP_PATTERN_HEIGHT = 14; // px
 const GRID_GAP_PATTERN_LINE_DASH = [5, 3]; // px
 const GRID_GAP_ALPHA = 0.5;
 
-// 25 is a good margin distance between the document grid container edge without cutting
-// off parts of the arrow box container.
-const OFFSET_FROM_EDGE = 25;
+// This is the minimum distance a line can be to the edge of the document under which we
+// push the line number arrow to be inside the grid. This offset is enough to fit the
+// entire arrow + a stacked arrow behind it.
+const OFFSET_FROM_EDGE = 32;
+// This is how much inside the grid we push the arrow. This a factor of the arrow size.
+// The goal here is for a row and a column arrow that have both been pushed inside the
+// grid, in a corner, not to overlap.
+const FLIP_ARROW_INSIDE_FACTOR = 2.5;
 
 /**
  * Given an `edge` of a box, return the name of the edge one move to the right.
  */
 function rotateEdgeRight(edge) {
   switch (edge) {
     case "top": return "right";
     case "right": return "bottom";
@@ -1182,83 +1187,179 @@ class CssGridHighlighter extends AutoRef
 
     // Calculate the x & y coordinates for the line number container, so that its arrow
     // tip is centered on the line (or the gap if there is one), and is offset by the
     // calculated padding value from the grid container edge.
     let x, y;
 
     if (dimensionType === COLUMNS) {
       x = linePos + breadth / 2;
-      y = startPos;
-
-      if (lineNumber > 0) {
-        y -= offsetFromEdge;
-      } else {
-        y += offsetFromEdge;
-      }
+      y = lineNumber > 0 ? startPos - offsetFromEdge : startPos + offsetFromEdge;
     } else if (dimensionType === ROWS) {
-      x = startPos;
       y = linePos + breadth / 2;
-
-      if (lineNumber > 0) {
-        x -= offsetFromEdge;
-      } else {
-        x += offsetFromEdge;
-      }
+      x = lineNumber > 0 ? startPos - offsetFromEdge : startPos + offsetFromEdge;
     }
 
     [x, y] = apply(this.currentMatrix, [x, y]);
 
-    if (isStackedLine) {
-      // Offset the stacked line number by half of the box's width/height.
-      const xOffset = boxWidth / 4;
-      const yOffset = boxHeight / 4;
-
-      if (lineNumber > 0) {
-        x -= xOffset;
-        y -= yOffset;
-      } else {
-        x += xOffset;
-        y += yOffset;
-      }
-    }
-
     // Draw a bubble rectangular arrow with a border width of 2 pixels, a border color
     // matching the grid color and a white background (the line number will be written in
     // black).
     this.ctx.lineWidth = 2 * displayPixelRatio;
     this.ctx.strokeStyle = this.color;
     this.ctx.fillStyle = "white";
 
     // See param definitions of drawBubbleRect.
     const radius = 2 * displayPixelRatio;
     const margin = 2 * displayPixelRatio;
     const arrowSize = 8 * displayPixelRatio;
 
     const minBoxSize = arrowSize * 2 + padding;
     boxWidth = Math.max(boxWidth, minBoxSize);
     boxHeight = Math.max(boxHeight, minBoxSize);
 
-    // Determine default box edge to aim the line number arrow at.
-    let boxEdge;
-    if (dimensionType === COLUMNS) {
+    // Determine which edge of the box to aim the line number arrow at.
+    const boxEdge = this.getBoxEdge(dimensionType, lineNumber);
+
+    let { width, height } = this._winDimensions;
+    width *= displayPixelRatio;
+    height *= displayPixelRatio;
+
+    // Don't draw if the line is out of the viewport.
+    if ((dimensionType === ROWS && (y < 0 || y > height)) ||
+        (dimensionType === COLUMNS && (x < 0 || x > width))) {
+      this.ctx.restore();
+      return;
+    }
+
+    // If the arrow's edge (the one perpendicular to the line direction) is too close to
+    // the edge of the viewport. Push the arrow inside the grid.
+    const minOffsetFromEdge = OFFSET_FROM_EDGE * displayPixelRatio;
+    switch (boxEdge) {
+      case "left":
+        if (x < minOffsetFromEdge) {
+          x += FLIP_ARROW_INSIDE_FACTOR * boxWidth;
+        }
+        break;
+      case "right":
+        if ((width - x) < minOffsetFromEdge) {
+          x -= FLIP_ARROW_INSIDE_FACTOR * boxWidth;
+        }
+        break;
+      case "top":
+        if (y < minOffsetFromEdge) {
+          y += FLIP_ARROW_INSIDE_FACTOR * boxHeight;
+        }
+        break;
+      case "bottom":
+        if ((height - y) < minOffsetFromEdge) {
+          y -= FLIP_ARROW_INSIDE_FACTOR * boxHeight;
+        }
+        break;
+    }
+
+    // Offset stacked line numbers by a quarter of the box's width/height, so a part of
+    // them remains visible behind the number that sits at the top of the stack.
+    if (isStackedLine) {
+      const xOffset = boxWidth / 4;
+      const yOffset = boxHeight / 4;
+
       if (lineNumber > 0) {
-        boxEdge = "top";
+        x -= xOffset;
+        y -= yOffset;
       } else {
-        boxEdge = "bottom";
+        x += xOffset;
+        y += yOffset;
       }
     }
-    if (dimensionType === ROWS) {
-      if (lineNumber > 0) {
-        boxEdge = "left";
-      } else {
-        boxEdge = "right";
+
+    // If one the edges of the arrow that's parallel to the line is too close to the edge
+    // of the viewport (and therefore partly hidden), grow the arrow's size in the
+    // opposite direction.
+    // The goal is for the part that's not hidden to be exactly the size of a normal
+    // arrow and for the arrow to keep pointing at the line (keep being centered on it).
+    let grewBox = false;
+    const boxWidthBeforeGrowth = boxWidth;
+    const boxHeightBeforeGrowth = boxHeight;
+
+    if (dimensionType === ROWS && y <= boxHeight / 2) {
+      grewBox = true;
+      boxHeight = 2 * (boxHeight - y);
+    } else if (dimensionType === ROWS && y >= height - boxHeight / 2) {
+      grewBox = true;
+      boxHeight = 2 * (y - height + boxHeight);
+    } else if (dimensionType === COLUMNS && x <= boxWidth / 2) {
+      grewBox = true;
+      boxWidth = 2 * (boxWidth - x);
+    } else if (dimensionType === COLUMNS && x >= width - boxWidth / 2) {
+      grewBox = true;
+      boxWidth = 2 * (x - width + boxWidth);
+    }
+
+    // Draw the arrow box itself
+    drawBubbleRect(this.ctx, x, y, boxWidth, boxHeight, radius, margin, arrowSize,
+                   boxEdge);
+
+    // Determine the text position for it to be centered nicely inside the arrow box.
+    switch (boxEdge) {
+      case "left":
+        x -= (boxWidth + arrowSize + radius) - boxWidth / 2;
+        break;
+      case "right":
+        x += (boxWidth + arrowSize + radius) - boxWidth / 2;
+        break;
+      case "top":
+        y -= (boxHeight + arrowSize + radius) - boxHeight / 2;
+        break;
+      case "bottom":
+        y += (boxHeight + arrowSize + radius) - boxHeight / 2;
+        break;
+    }
+
+    // Do a second pass to adjust the position, along the other axis, if the box grew
+    // during the previous step, so the text is also centered on that axis.
+    if (grewBox) {
+      if (dimensionType === ROWS && y <= boxHeightBeforeGrowth / 2) {
+        y = boxHeightBeforeGrowth / 2;
+      } else if (dimensionType === ROWS && y >= height - boxHeightBeforeGrowth / 2) {
+        y = height - boxHeightBeforeGrowth / 2;
+      } else if (dimensionType === COLUMNS && x <= boxWidthBeforeGrowth / 2) {
+        x = boxWidthBeforeGrowth / 2;
+      } else if (dimensionType === COLUMNS && x >= width - boxWidthBeforeGrowth / 2) {
+        x = width - boxWidthBeforeGrowth / 2;
       }
     }
 
+    // Write the line number inside of the rectangle.
+    this.ctx.textAlign = "center";
+    this.ctx.textBaseline = "middle";
+    this.ctx.fillStyle = "black";
+    const numberText = isStackedLine ? "" : lineNumber;
+    this.ctx.fillText(numberText, x, y);
+    this.ctx.restore();
+  }
+
+  /**
+   * Determine which edge of a line number box to aim the line number arrow at.
+   *
+   * @param  {String} dimensionType
+   *         The grid line dimension type which is either the constant COLUMNS or ROWS.
+   * @param  {Number} lineNumber
+   *         The grid line number.
+   * @return {String} The edge of the box: top, right, bottom or left.
+   */
+  getBoxEdge(dimensionType, lineNumber) {
+    let boxEdge;
+
+    if (dimensionType === COLUMNS) {
+      boxEdge = lineNumber > 0 ? "top" : "bottom";
+    } else if (dimensionType === ROWS) {
+      boxEdge = lineNumber > 0 ? "left" : "right";
+    }
+
     // Rotate box edge as needed for writing mode and text direction.
     const { direction, writingMode } = getComputedStyle(this.currentNode);
 
     switch (writingMode) {
       case "horizontal-tb":
         // This is the initial value.  No further adjustment needed.
         break;
       case "vertical-rl":
@@ -1289,75 +1390,17 @@ class CssGridHighlighter extends AutoRef
         if (dimensionType === ROWS) {
           boxEdge = reflectEdge(boxEdge);
         }
         break;
       default:
         console.error(`Unexpected direction: ${direction}`);
     }
 
-    // Default to drawing outside the edge, but move inside when close to viewport.
-    const minOffsetFromEdge = OFFSET_FROM_EDGE * displayPixelRatio;
-    let { width, height } = this._winDimensions;
-    width *= displayPixelRatio;
-    height *= displayPixelRatio;
-
-    // Check if the x or y position of the line number's arrow is too close to the edge
-    // of the window.  If it is too close, adjust the position by 2 x boxWidth or
-    // boxHeight since we're now going the opposite direction.
-    switch (boxEdge) {
-      case "left":
-        if (x < minOffsetFromEdge) {
-          x += 2 * boxWidth;
-        }
-        break;
-      case "right":
-        if ((width - x) < minOffsetFromEdge) {
-          x -= 2 * boxWidth;
-        }
-        break;
-      case "top":
-        if (y < minOffsetFromEdge) {
-          y += 2 * boxHeight;
-        }
-        break;
-      case "bottom":
-        if ((height - y) < minOffsetFromEdge) {
-          y -= 2 * boxHeight;
-        }
-        break;
-    }
-
-    // Draw the bubble rect to show the arrow.
-    drawBubbleRect(this.ctx, x, y, boxWidth, boxHeight, radius, margin, arrowSize,
-                   boxEdge);
-
-    // Adjust position based on the edge.
-    switch (boxEdge) {
-      case "left":
-        x -= (boxWidth + arrowSize + radius) - boxWidth / 2;
-        break;
-      case "right":
-        x += (boxWidth + arrowSize + radius) - boxWidth / 2;
-        break;
-      case "top":
-        y -= (boxHeight + arrowSize + radius) - boxHeight / 2;
-        break;
-      case "bottom":
-        y += (boxHeight + arrowSize + radius) - boxHeight / 2;
-        break;
-    }
-
-    // Write the line number inside of the rectangle.
-    this.ctx.textAlign = "center";
-    this.ctx.textBaseline = "middle";
-    this.ctx.fillStyle = "black";
-    const numberText = isStackedLine ? "" : lineNumber;
-    this.ctx.fillText(numberText, x, y);
-    this.ctx.restore();
+    return boxEdge;
   }
 
   /**
    * Render the grid line on the css grid highlighter canvas.
    *
    * @param  {Number} linePos
    *         The line position along the x-axis for a column grid line and
    *         y-axis for a row grid line.
--- a/devtools/server/actors/highlighters/utils/canvas.js
+++ b/devtools/server/actors/highlighters/utils/canvas.js
@@ -101,27 +101,35 @@ function drawBubbleRect(ctx, x, y, width
   const originY = y;
 
   ctx.save();
   ctx.translate(originX, originY);
   ctx.rotate(angle * (Math.PI / 180));
   ctx.translate(-originX, -originY);
   ctx.translate(-width / 2, -height - arrowSize - margin);
 
+  // The contour of the bubble is drawn with a path. The canvas context will have taken
+  // care of transforming the coordinates before calling the function, so we just always
+  // draw with the arrow pointing down. The top edge has rounded corners too.
   ctx.beginPath();
+  // Start at the top/left corner (below the rounded corner).
   ctx.moveTo(x, y + radius);
-  ctx.lineTo(x, y + height - radius);
-  ctx.arcTo(x, y + height, x + radius, y + height, radius);
-  ctx.lineTo(x + width / 2 - arrowSize, y + height);
+  // Go down.
+  ctx.lineTo(x, y + height);
+  // Go down and the right, to draw the first half of the arrow tip.
   ctx.lineTo(x + width / 2, y + height + arrowSize);
-  ctx.lineTo(x + width / 2 + arrowSize, y + height);
-  ctx.arcTo(x + width, y + height, x + width, y + height - radius, radius);
+  // Go back up and to the right, to draw the second half of the arrow tip.
+  ctx.lineTo(x + width, y + height);
+  // Go up to just below the top/right rounded corner.
   ctx.lineTo(x + width, y + radius);
+  // Draw the top/right rounded corner.
   ctx.arcTo(x + width, y, x + width - radius, y, radius);
+  // Go to the left.
   ctx.lineTo(x + radius, y);
+  // Draw the top/left rounded corner.
   ctx.arcTo(x, y, x, y + radius, radius);
 
   ctx.stroke();
   ctx.fill();
 
   ctx.restore();
 }