Bug 1488031 - Font editor: show labels for min and max values for axes on hover and focus. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Fri, 14 Sep 2018 09:37:15 +0000
changeset 436328 cacbaf825e3a274df69c1b8e9fd64e54b467797d
parent 436327 bff55a91da7c7685a4fb7b4db7e65d7db91940da
child 436344 0bd6b228ce72083efd4763784e497c9a810bf697
push id69293
push userrcaliman@mozilla.com
push dateFri, 14 Sep 2018 09:38:20 +0000
treeherderautoland@cacbaf825e3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1488031
milestone64.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 1488031 - Font editor: show labels for min and max values for axes on hover and focus. r=gl Differential Revision: https://phabricator.services.mozilla.com/D5143
devtools/client/inspector/fonts/components/FontAxis.js
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontPropertyValue.js
devtools/client/inspector/fonts/test/head.js
devtools/client/inspector/fonts/utils/font-utils.js
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/FontAxis.js
+++ b/devtools/client/inspector/fonts/components/FontAxis.js
@@ -48,17 +48,19 @@ class FontAxis extends PureComponent {
 
   render() {
     const { axis, value, onChange } = this.props;
 
     return FontPropertyValue({
       className: "font-control-axis",
       label: axis.name,
       min: axis.minValue,
+      minLabel: true,
       max: axis.maxValue,
+      maxLabel: true,
       name: axis.tag,
       nameLabel: true,
       onChange,
       step: this.getAxisStep(axis.minValue, axis.maxValue),
       value,
     });
   }
 }
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -50,16 +50,18 @@ class FontEditor extends PureComponent {
       return null;
     }
 
     return fontAxes.map(axis => {
       return FontAxis({
         key: axis.tag,
         axis,
         onChange: this.props.onPropertyChange,
+        minLabel: true,
+        maxLabel: true,
         value: editedAxes[axis.tag] || axis.defaultValue,
       });
     });
   }
 
   /**
    * Render fonts used on the selected node grouped by font-family.
    *
--- a/devtools/client/inspector/fonts/components/FontPropertyValue.js
+++ b/devtools/client/inspector/fonts/components/FontPropertyValue.js
@@ -8,42 +8,51 @@ const {
   createElement,
   Fragment,
   PureComponent,
 } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { KeyCodes } = require("devtools/client/shared/keycodes");
 
+const { toFixed } = require("../utils/font-utils");
+
 // Milliseconds between auto-increment interval iterations.
 const AUTOINCREMENT_DELAY = 1000;
 
 class FontPropertyValue extends PureComponent {
   static get propTypes() {
     return {
       autoIncrement: PropTypes.bool,
       className: PropTypes.string,
       defaultValue: PropTypes.number,
       label: PropTypes.string.isRequired,
       min: PropTypes.number.isRequired,
+      // Whether to show the `min` prop value as a label.
+      minLabel: PropTypes.bool,
       max: PropTypes.number.isRequired,
+      // Whether to show the `max` prop value as a label.
+      maxLabel: PropTypes.bool,
       name: PropTypes.string.isRequired,
-      nameLabel: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
+      // Whether to show the `name` prop value as an extra label (used to show axis tags).
+      nameLabel: PropTypes.bool,
       onChange: PropTypes.func.isRequired,
       step: PropTypes.number,
       unit: PropTypes.string,
       unitOptions: PropTypes.array,
       value: PropTypes.number,
     };
   }
 
   static get defaultProps() {
     return {
       autoIncrement: false,
       className: "",
+      minLabel: false,
+      maxLabel: false,
       nameLabel: false,
       step: 1,
       unit: null,
       unitOptions: []
     };
   }
 
   constructor(props) {
@@ -89,16 +98,32 @@ class FontPropertyValue extends PureComp
    * Increment the current value with a step of the next order of magnitude.
    */
   autoIncrement() {
     const value = this.props.value + this.props.step * 10;
     this.updateValue(value);
   }
 
   /**
+   * Given a `prop` key found on the component's props, check the matching `propLabel`.
+   * If `propLabel` is true, return the `prop` value; Otherwise, return null.
+   *
+   * @param {String} prop
+   *        Key found on the component's props.
+   * @return {Number|null}
+   */
+  getPropLabel(prop) {
+    const label = this.props[`${prop}Label`];
+    // Decimal count used to limit numbers in labels.
+    const decimals = Math.abs(Math.log10(this.props.step));
+
+    return label ? toFixed(this.props[prop], decimals) : null;
+  }
+
+  /**
    * Check if the given value is at or exceeds the maximum value for the slider and number
    * inputs. Using Math.floor() on maximum value because unit conversion can yield numbers
    * with decimals that can't be reached with the step increment for the converted unit.
    * For example: max = 1000.75% and step = 1
    *
    * @param  {Number} value
    *         Numeric value.
    * @return {Boolean}
@@ -374,25 +399,24 @@ class FontPropertyValue extends PureComp
     const labelEl = dom.span(
       {
         className: "font-control-label-text",
         "aria-describedby": nameLabel ? `detail-${name}` : null,
       },
       label
     );
 
-    // If this.props.nameLabel is boolean true, the detail label text is the property
-    // name. If it's a string, use that. Otherwise, do not show the additional label text.
+    // Show the `name` prop value as an additional label if the `nameLabel` prop is true.
     const detailEl = nameLabel ?
       dom.span(
         {
           className: "font-control-label-detail",
           id: `detail-${name}`
         },
-        typeof nameLabel === "boolean" ? name : nameLabel
+        this.getPropLabel("name")
       )
       :
       null;
 
     return createElement(Fragment, null, labelEl, detailEl);
   }
 
   render() {
@@ -453,17 +477,24 @@ class FontPropertyValue extends PureComp
           title: this.props.label,
         },
         this.renderLabelContent()
       ),
       dom.div(
         {
           className: "font-control-input"
         },
-        range,
+        dom.div(
+          {
+            className: "font-value-slider-container",
+            "data-min": this.getPropLabel("min"),
+            "data-max": this.getPropLabel("max"),
+          },
+          range
+        ),
         input,
         this.renderUnitSelect()
       )
     );
   }
 }
 
 module.exports = FontPropertyValue;
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -216,22 +216,22 @@ function getFamilyName(fontEl) {
  *         Host document of the font inspector panel.
  * @param  {String} name
  *         Font property name or axis tag
  * @return {Object}
  *         Object with the value and unit of the given font property or axis tag
  *         from the corresponding input fron the font editor.
  *         @Example:
  *         {
- *          value: {Number|String|null}
+ *          value: {String|null}
  *          unit: {String|null}
  *         }
  */
 function getPropertyValue(viewDoc, name) {
-  const selector = `#font-editor .font-value-slider[name=${name}]`;
+  const selector = `#font-editor .font-value-input[name=${name}]`;
   return {
     // Ensure value input exists before querying its value
     value: viewDoc.querySelector(selector) &&
            parseFloat(viewDoc.querySelector(selector).value),
     // Ensure unit dropdown exists before querying its value
     unit: viewDoc.querySelector(selector + ` ~ .font-value-select`) &&
           viewDoc.querySelector(selector + ` ~ .font-value-select`).value
   };
--- a/devtools/client/inspector/fonts/utils/font-utils.js
+++ b/devtools/client/inspector/fonts/utils/font-utils.js
@@ -87,10 +87,27 @@ module.exports = {
         // Axis tags shorter or longer than 4 characters are invalid. Whitespace is valid.
         if (tag.length === 4) {
           acc[tag] = value;
         }
         return acc;
       }, {});
 
     return axes;
+  },
+
+  /**
+   * Limit the decimal count of a number. Used instead of Number.toFixed() which pads
+   * integers with zeroes. If the input is not a number, it is returned as is.
+   *
+   * @param {Number} number
+   * @param {Number} decimals
+   *        Decimal count in the output number. Default to one decimal.
+   * @return {Number}
+   */
+  toFixed(number, decimals = 1) {
+    if (typeof number !== "number") {
+      return number;
+    }
+
+    return Math.floor(number * Math.pow(10, decimals)) / Math.pow(10, decimals);
   }
 };
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -1,29 +1,31 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* CSS Variables specific to the font editor that aren't defined by the themes */
 :root {
-  --slider-thumb-color: var(--grey-50);
-  --slider-track-color: var(--grey-30);
+  --highlight-color: var(--blue-55);
   --input-background-color: white;
   --input-border-color: var(--grey-30);
   --input-text-color: var(--grey-90);
   --preview-input-background: var(--theme-toolbar-background);
+  --secondary-label-color: var(--grey-45);
+  --slider-thumb-color: var(--grey-50);
+  --slider-track-color: var(--grey-30);
 }
 
 :root.theme-dark {
-  --slider-thumb-color: var(--grey-40);
-  --slider-track-color: var(--grey-60);
   --input-background-color: var(--grey-70);
   --input-border-color: var(--grey-70);
   --input-text-color: var(--grey-40);
   --preview-input-background: #222225;
+  --slider-thumb-color: var(--grey-40);
+  --slider-track-color: var(--grey-60);
 }
 
 #sidebar-panel-fontinspector {
   margin: 0;
   display: flex;
   flex-direction: column;
   width: 100%;
   height: 100%;
@@ -230,17 +232,17 @@
 .font-control-label-text {
   display: block;
   overflow: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
 }
 
 .font-control-label-detail {
-  color: var(--grey-45);
+  color: var(--secondary-label-color);
   font-size: smaller;
   text-transform: none;
 }
 
 .font-value-input {
   margin-left: 10px;
   text-align: right;
   width: 60px;
@@ -303,24 +305,57 @@
 
 .font-value-select:-moz-focusring {
   color: transparent;
   text-shadow: 0 0 0 var(--input-text-color);
 }
 
 .font-value-input:focus,
 .font-value-select:focus {
-  outline: 1px solid var(--blue-55);
+  outline: 1px solid var(--highlight-color);
   outline-offset: -1px;
 }
 
-.font-value-slider {
+.font-value-slider-container {
   flex: 1;
+  min-width: 50px;
+  position: relative;
+}
+
+/* Firefox doesn't support pseudo-elements on inputs. Using the container instead. */
+.font-value-slider-container::before,
+.font-value-slider-container::after {
+  -moz-user-select: none;
+  color: var(--secondary-label-color);
+  font-size: smaller;
+  position: absolute;
+  bottom: -.6em;
+  visibility: hidden;
+}
+
+.font-control-input:hover .font-value-slider-container::before,
+.font-control-input:hover .font-value-slider-container::after,
+.font-control-input:focus-within .font-value-slider-container::before,
+.font-control-input:focus-within .font-value-slider-container::after{
+  visibility: visible;
+}
+
+.font-value-slider-container::before {
+  content: attr(data-min);
+  left: .3em;
+}
+
+.font-value-slider-container::after {
+  content: attr(data-max);
+  right: .3em;
+}
+
+.font-value-slider {
+  width: 100%;
   margin: 0;
-  min-width: 50px;
 }
 
 /*
   The value of font-weight goes from 100 to 900 in increments of 100.
   Decorate the slider for font-weight to have 9 vertical notches using a linear gradient.
 */
 .font-value-slider[name=font-weight] {
   --notch-size: 3px;
@@ -344,17 +379,17 @@
 }
 
 .font-value-slider::-moz-range-thumb {
   background-color: var(--slider-thumb-color);
   border: 0;
 }
 
 .font-value-slider:focus::-moz-range-thumb {
-  background-color: var(--blue-55);
+  background-color: var(--highlight-color);
 }
 
 .font-value-slider::-moz-range-track {
   background-color: var(--slider-track-color);
   height: 3px;
 }
 
 .font-origin {