Bug 1488031 - Font editor: show labels for min and max values for axes on hover and focus. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 13 Sep 2018 11:59:50 +0000
changeset 436147 ee4b2d3fd97ab313008588dd78d0c2766158836b
parent 436146 4335c690a98c5ed7d8b892ed8fa59aa64fc11d0c
child 436148 8a13d8a20947908486d7e74e8f5cbdef95a30daf
push id34630
push usernerli@mozilla.com
push dateThu, 13 Sep 2018 21:58:06 +0000
treeherdermozilla-central@71736f1f0f3e [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/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/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 {