Bug 1488031 - Trim axis labels, show variation axis tag when available. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 12 Sep 2018 14:27:16 +0000
changeset 436134 075ad27d064b29f300852050a7bc958e8e1e6e17
parent 436133 4d081dbefe2f1ac8ef8ffcb97b85a6dd6f88b588
child 436135 757bbd2e31766ef88dabab6b89e5516ec223383f
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 - Trim axis labels, show variation axis tag when available. r=gl Differential Revision: https://phabricator.services.mozilla.com/D5087
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/components/moz.build
devtools/client/themes/fonts.css
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/components/FontAxis.js
@@ -0,0 +1,66 @@
+/* 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/. */
+
+"use strict";
+
+const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
+const FontPropertyValue = createFactory(require("./FontPropertyValue"));
+
+const Types = require("../types");
+
+class FontAxis extends PureComponent {
+  static get propTypes() {
+    return {
+      axis: PropTypes.shape(Types.fontVariationAxis),
+      onChange: PropTypes.func.isRequired,
+      value: PropTypes.string.isRequired,
+    };
+  }
+
+  /**
+   * Naive implementation to get increment step for variable font axis that ensures
+   * fine grained control based on range of values between min and max.
+   *
+   * @param  {Number} min
+   *         Minumum value for range.
+   * @param  {Number} max
+   *         Maximum value for range.
+   * @return {Number}
+   *         Step value used in range input for font axis.
+   */
+  getAxisStep(min, max) {
+    let step = 1;
+    const delta = parseInt(max, 10) - parseInt(min, 10);
+
+    if (delta <= 1) {
+      step = 0.001;
+    } else if (delta <= 10) {
+      step = 0.01;
+    } else if (delta <= 100) {
+      step = 0.1;
+    }
+
+    return step;
+  }
+
+  render() {
+    const { axis, value, onChange } = this.props;
+
+    return FontPropertyValue({
+      className: "font-control-axis",
+      label: axis.name,
+      min: axis.minValue,
+      max: axis.maxValue,
+      name: axis.tag,
+      nameLabel: true,
+      onChange,
+      step: this.getAxisStep(axis.minValue, axis.maxValue),
+      value,
+    });
+  }
+}
+
+module.exports = FontAxis;
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -3,18 +3,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { createFactory, 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 FontAxis = createFactory(require("./FontAxis"));
 const FontName = createFactory(require("./FontName"));
-const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 const FontSize = createFactory(require("./FontSize"));
 const FontStyle = createFactory(require("./FontStyle"));
 const FontWeight = createFactory(require("./FontWeight"));
 const LineHeight = createFactory(require("./LineHeight"));
 
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
@@ -28,71 +28,38 @@ class FontEditor extends PureComponent {
       fontEditor: PropTypes.shape(Types.fontEditor).isRequired,
       onInstanceChange: PropTypes.func.isRequired,
       onPropertyChange: PropTypes.func.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   /**
-   * Naive implementation to get increment step for variable font axis that ensures
-   * a wide spectrum of precision based on range of values between min and max.
-   *
-   * @param  {Number|String} min
-   *         Minumum value for range.
-   * @param  {Number|String} max
-   *         Maximum value for range.
-   * @return {String}
-   *         Step value used in range input for font axis.
-   */
-  getAxisStep(min, max) {
-    let step = 1;
-    const delta = parseInt(max, 10) - parseInt(min, 10);
-
-    if (delta <= 1) {
-      step = 0.001;
-    } else if (delta <= 10) {
-      step = 0.01;
-    } else if (delta <= 100) {
-      step = 0.1;
-    }
-
-    return step.toString();
-  }
-
-  /**
-   * Get an array of FontPropertyValue components with editing controls
-   * for of the given variable font axes. If no axes were given, return null.
-   * If an axis has a value in the fontEditor store (i.e.: it was declared in CSS or
-   * it was changed using the font editor), use its value, otherwise use the font axis
-   * default.
+   * Get an array of FontAxis components with editing controls for of the given variable
+   * font axes. If no axes were given, return null.
+   * If an axis' value was declared on the font-variation-settings CSS property or was
+   * changed using the font editor, use that value, otherwise use the axis default.
    *
    * @param  {Array} fontAxes
    *         Array of font axis instances
    * @param  {Object} editedAxes
-   *         Object with axes and values edited by the user or predefined in the CSS
+   *         Object with axes and values edited by the user or defined in the CSS
    *         declaration for font-variation-settings.
    * @return {Array|null}
    */
   renderAxes(fontAxes = [], editedAxes) {
     if (!fontAxes.length) {
       return null;
     }
 
     return fontAxes.map(axis => {
-      return FontPropertyValue({
+      return FontAxis({
         key: axis.tag,
-        className: "font-control-axis",
-        label: axis.name,
-        min: axis.minValue,
-        max: axis.maxValue,
-        name: axis.tag,
+        axis,
         onChange: this.props.onPropertyChange,
-        step: this.getAxisStep(axis.minValue, axis.maxValue),
-        unit: null,
         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
@@ -1,45 +1,52 @@
 /* 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/. */
 
 "use strict";
 
-const { PureComponent } = require("devtools/client/shared/vendor/react");
+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");
 
 // 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.oneOfType([ PropTypes.string, PropTypes.number ]),
+      defaultValue: PropTypes.number,
       label: PropTypes.string.isRequired,
       min: PropTypes.number.isRequired,
       max: PropTypes.number.isRequired,
       name: PropTypes.string.isRequired,
+      nameLabel: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
       onChange: PropTypes.func.isRequired,
       step: PropTypes.number,
-      unit: PropTypes.oneOfType([ PropTypes.string, null ]),
+      unit: PropTypes.string,
       unitOptions: PropTypes.array,
       value: PropTypes.number,
     };
   }
 
   static get defaultProps() {
     return {
       autoIncrement: false,
       className: "",
+      nameLabel: false,
       step: 1,
+      unit: null,
       unitOptions: []
     };
   }
 
   constructor(props) {
     super(props);
     // Interval ID for the auto-increment operation.
     this.interval = null;
@@ -352,16 +359,47 @@ class FontPropertyValue extends PureComp
             value: unit,
           },
           unit
         );
       })
     );
   }
 
+  renderLabelContent() {
+    const {
+      label,
+      name,
+      nameLabel,
+    } = this.props;
+
+    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.
+    const detailEl = nameLabel ?
+      dom.span(
+        {
+          className: "font-control-label-detail",
+          id: `detail-${name}`
+        },
+        typeof nameLabel === "boolean" ? name : nameLabel
+      )
+      :
+      null;
+
+    return createElement(Fragment, null, labelEl, detailEl);
+  }
+
   render() {
     // Guard against bad axis data.
     if (this.props.min === this.props.max) {
       return null;
     }
 
     const propsValue = this.props.value !== null
       ? this.props.value
@@ -404,21 +442,22 @@ class FontPropertyValue extends PureComp
         type: "number",
       }
     );
 
     return dom.label(
       {
         className: `font-control ${this.props.className}`,
       },
-      dom.span(
+      dom.div(
         {
           className: "font-control-label",
+          title: this.props.label,
         },
-        this.props.label
+        this.renderLabelContent()
       ),
       dom.div(
         {
           className: "font-control-input"
         },
         range,
         input,
         this.renderUnitSelect()
--- a/devtools/client/inspector/fonts/components/moz.build
+++ b/devtools/client/inspector/fonts/components/moz.build
@@ -1,16 +1,17 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 DevToolsModules(
     'Font.js',
+    'FontAxis.js',
     'FontEditor.js',
     'FontList.js',
     'FontName.js',
     'FontOrigin.js',
     'FontOverview.js',
     'FontPreview.js',
     'FontPreviewInput.js',
     'FontPropertyValue.js',
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -216,21 +216,35 @@
 .font-control-input .devtools-checkbox-toggle {
   margin: 2px 0;
 }
 
 .font-control-label {
   display: inline-block;
   flex: 1;
   font-size: 12px;
-  max-width: 70px;
+  min-width: 70px;
   margin-right: 10px;
+  text-transform: capitalize;
   -moz-user-select: none;
 }
 
+.font-control-label-text {
+  display: block;
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
+.font-control-label-detail {
+  color: var(--grey-45);
+  font-size: smaller;
+  text-transform: none;
+}
+
 .font-value-input {
   margin-left: 10px;
   text-align: right;
   width: 60px;
   padding: 2px 3px;
 }
 
 .font-value-input,