Bug 1435368 - Implement precision when rounding polygon coordinates on Shapes editor. r=gl draft
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 06 Feb 2018 11:35:55 -0500
changeset 753906 d2f91ffc1c4f8912922923a4271a6a20de99d724
parent 753815 3ee38289dac8838fe848f7234d75f3cef5d3dbc7
push id98718
push userbmo:rcaliman@mozilla.com
push dateMon, 12 Feb 2018 17:14:54 +0000
reviewersgl
bugs1435368
milestone60.0a1
Bug 1435368 - Implement precision when rounding polygon coordinates on Shapes editor. r=gl
devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
devtools/server/actors/highlighters/shapes.js
devtools/server/tests/unit/test_shapes_highlighter_helpers.js
--- a/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
@@ -82,17 +82,22 @@ function* testPolygonAddPoint(testActor,
   };
 
   info("Adding new polygon point");
   yield testActor.synthesizeMouse(options);
   yield testActor.reflow();
 
   let computedStyle = yield highlightedNode.getComputedStyle();
   let definition = computedStyle["clip-path"].value;
-  ok(definition.includes(`${newPointX * 100 / width}% ${newPointY * 100 / height}%`),
+  // Decimal precision for coordinates with percentage units is 2
+  let precision = 2;
+  // Round to the desired decimal precision and cast to Number to remove trailing zeroes.
+  newPointX = Number((newPointX * 100 / width).toFixed(precision));
+  newPointY = Number((newPointY * 100 / height).toFixed(precision));
+  ok(definition.includes(`${newPointX}% ${newPointY}%`),
      "Point successfuly added");
 }
 
 function* testPolygonRemovePoint(testActor, helper) {
   yield helper.show("#polygon", {mode: "cssClipPath"});
   let { highlightedNode } = helper;
 
   let points = yield helper.getElementAttribute("shapes-polygon", "points");
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -829,17 +829,22 @@ class ShapesHighlighter extends AutoRefr
   _transformPolygon() {
     let { pointsInfo } = this[_dragging];
 
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += pointsInfo.map(point => {
       let { unitX, unitY, valueX, valueY, ratioX, ratioY } = point;
       let vector = [valueX / ratioX, valueY / ratioY];
       let [newX, newY] = apply(this.transformMatrix, vector);
-      return `${newX * ratioX}${unitX} ${newY * ratioY}${unitY}`;
+      let precisionX = getDecimalPrecision(unitX);
+      let precisionY = getDecimalPrecision(unitY);
+      newX = (newX * ratioX).toFixed(precisionX);
+      newY = (newY * ratioY).toFixed(precisionY);
+
+      return `${newX}${unitX} ${newY}${unitY}`;
     }).join(", ");
     polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
                                       `polygon(${polygonDef})`;
 
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
 
   /**
@@ -960,31 +965,37 @@ class ShapesHighlighter extends AutoRefr
    * coords.
    * @param {Number} pageX the new x coordinate of the point
    * @param {Number} pageY the new y coordinate of the point
    */
   _handlePolygonMove(pageX, pageY) {
     let { point, unitX, unitY, valueX, valueY, ratioX, ratioY, x, y } = this[_dragging];
     let deltaX = (pageX - x) * ratioX;
     let deltaY = (pageY - y) * ratioY;
-    let newX = `${valueX + deltaX}${unitX}`;
-    let newY = `${valueY + deltaY}${unitY}`;
+    let precisionX = getDecimalPrecision(unitX);
+    let precisionY = getDecimalPrecision(unitY);
+    let newX = (valueX + deltaX).toFixed(precisionX);
+    let newY = (valueY + deltaY).toFixed(precisionY);
 
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += this.coordUnits.map((coords, i) => {
-      return (i === point) ? `${newX} ${newY}` : `${coords[0]} ${coords[1]}`;
+      return (i === point) ?
+        `${newX}${unitX} ${newY}${unitY}` : `${coords[0]} ${coords[1]}`;
     }).join(", ");
     polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
                                       `polygon(${polygonDef})`;
 
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
 
   /**
    * Set the inline style of the polygon, adding a new point.
+   * TODO: Bug 1436054 - Do not default to percentage unit when inserting new point.
+   * https://bugzilla.mozilla.org/show_bug.cgi?id=1436054
+   *
    * @param {Number} after the index of the point that the new point should be added after
    * @param {Number} x the x coordinate of the new point
    * @param {Number} y the y coordinate of the new point
    */
   _addPolygonPoint(after, x, y) {
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += this.coordUnits.map((coords, i) => {
       return (i === after) ? `${coords[0]} ${coords[1]}, ${x}% ${y}%` :
@@ -1562,17 +1573,19 @@ class ShapesHighlighter extends AutoRefr
       let distance = distanceToLine(x1, y1, x2, y2, pageX, pageY);
       if (distance <= clickWidth &&
           Math.min(x1, x2) - clickWidth <= pageX &&
           pageX <= Math.max(x1, x2) + clickWidth &&
           Math.min(y1, y2) - clickWidth <= pageY &&
           pageY <= Math.max(y1, y2) + clickWidth) {
         // Get the point on the line closest to the clicked point.
         let [newX, newY] = projection(x1, y1, x2, y2, pageX, pageY);
-        this._addPolygonPoint(i, newX, newY);
+        // Default unit for new points is percentages
+        let precision = getDecimalPrecision("%");
+        this._addPolygonPoint(i, newX.toFixed(precision), newY.toFixed(precision));
         return;
       }
     }
   }
 
   /**
    * Check if the center point or radius of the circle highlighter is at given coords
    * @param {Number} pageX the x coordinate on the page, in % relative to the element
@@ -2723,9 +2736,32 @@ const getAnchorPoint = (type) => {
     anchor = "n" + anchor;
   } else if (anchor === "n" || anchor === "s") {
     anchor = anchor + "w";
   }
 
   return anchor;
 };
 
+/**
+* Get the decimal point precision for values depending on unit type.
+* Used as argument for `toFixed()` on coordinate values when:
+* - transforming shapes
+* - inserting new points on a polygon.
+* Only handle pixels and falsy values for now. Round them to the nearest integer value.
+* All other unit types round to two decimal points.
+*
+* @param {String|undefined} unitType any one of the accepted CSS unit types for position.
+* @return {Number} decimal precision when rounding a value
+*/
+function getDecimalPrecision(unitType) {
+  switch (unitType) {
+    case "px":
+    case "":
+    case undefined:
+      return 0;
+    default:
+      return 2;
+  }
+}
+exports.getDecimalPrecision = getDecimalPrecision;
+
 exports.ShapesHighlighter = ShapesHighlighter;
--- a/devtools/server/tests/unit/test_shapes_highlighter_helpers.js
+++ b/devtools/server/tests/unit/test_shapes_highlighter_helpers.js
@@ -8,25 +8,27 @@
 "use strict";
 
 const {
   splitCoords,
   coordToPercent,
   evalCalcExpression,
   shapeModeToCssPropertyName,
   getCirclePath,
+  getDecimalPrecision,
   getUnit
 } = require("devtools/server/actors/highlighters/shapes");
 
 function run_test() {
   test_split_coords();
   test_coord_to_percent();
   test_eval_calc_expression();
   test_shape_mode_to_css_property_name();
   test_get_circle_path();
+  test_get_decimal_precision();
   test_get_unit();
   run_next_test();
 }
 
 function test_split_coords() {
   const tests = [{
     desc: "splitCoords for basic coordinate pair",
     expr: "30% 20%",
@@ -123,16 +125,39 @@ function test_get_circle_path() {
     expected: "M-2.5,0a2.5,1.25 0 1,0 5,0a2.5,1.25 0 1,0 -5,0"
   }];
 
   for (let { desc, size, cx, cy, width, height, zoom, expected } of tests) {
     equal(getCirclePath(size, cx, cy, width, height, zoom), expected, desc);
   }
 }
 
+function test_get_decimal_precision() {
+  const tests = [{
+    desc: "getDecimalPrecision with px",
+    expr: "px", expected: 0
+  }, {
+    desc: "getDecimalPrecision with %",
+    expr: "%", expected: 2
+  }, {
+    desc: "getDecimalPrecision with em",
+    expr: "em", expected: 2
+  }, {
+    desc: "getDecimalPrecision with undefined",
+    expr: undefined, expected: 0
+  }, {
+    desc: "getDecimalPrecision with empty string",
+    expr: "", expected: 0
+  }];
+
+  for (let { desc, expr, expected } of tests) {
+    equal(getDecimalPrecision(expr), expected, desc);
+  }
+}
+
 function test_get_unit() {
   const tests = [{
     desc: "getUnit with %",
     expr: "30%", expected: "%"
   }, {
     desc: "getUnit with px",
     expr: "400px", expected: "px"
   }, {