Bug 1483575 - (Part 2) Add LineHeight component to font editor. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 16 Aug 2018 14:48:51 +0000
changeset 432105 961c73e8fac1b302699d3258a3b00aac7a66cb58
parent 432104 b4bfcefc0535849f47065f0cdde90c1abc20926f
child 432106 26c699458e1fbaa99c3915d9fb0bb3f04b559797
push id34460
push userdvarga@mozilla.com
push dateFri, 17 Aug 2018 21:51:39 +0000
treeherdermozilla-central@2f1bbddc826b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1483575
milestone63.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 1483575 - (Part 2) Add LineHeight component to font editor. r=gl MozReview-Commit-ID: JiMBI3QMKML Depends on D3498 Differential Revision: https://phabricator.services.mozilla.com/D3499
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontPropertyValue.js
devtools/client/inspector/fonts/components/LineHeight.js
devtools/client/inspector/fonts/components/moz.build
devtools/client/inspector/fonts/fonts.js
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -8,16 +8,17 @@ const { createFactory, PureComponent } =
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const FontMeta = createFactory(require("./FontMeta"));
 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");
 
 // Maximum number of font families to be shown by default. Any others will be hidden
 // under a collapsed <details> element with a toggle to reveal them.
 const MAX_FONTS = 3;
 
@@ -190,17 +191,25 @@ class FontEditor extends PureComponent {
           })
         );
       })
     );
   }
 
   renderFontSize(value) {
     return value && FontSize({
-      key: this.props.fontEditor.id,
+      key: `${this.props.fontEditor.id}:font-size`,
+      onChange: this.props.onPropertyChange,
+      value,
+    });
+  }
+
+  renderLineHeight(value) {
+    return value && LineHeight({
+      key: `${this.props.fontEditor.id}:line-height`,
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   renderFontStyle(value) {
     return value && FontStyle({
       onChange: this.props.onPropertyChange,
@@ -313,16 +322,18 @@ class FontEditor extends PureComponent {
         id: "font-editor"
       },
       // Always render UI for font family, format and font file URL.
       this.renderFontFamily(fonts, families),
       // Render UI for font variation instances if they are defined.
       hasFontInstances && this.renderInstances(font.variationInstances, instance),
       // Always render UI for font size.
       this.renderFontSize(properties["font-size"]),
+      // Always render UI for line height.
+      this.renderLineHeight(properties["line-height"]),
       // Render UI for font weight if no "wght" registered axis is defined.
       !hasWeightAxis && this.renderFontWeight(properties["font-weight"]),
       // Render UI for font style if no "slnt" or "ital" registered axis is defined.
       !hasSlantOrItalicAxis && this.renderFontStyle(properties["font-style"]),
       // Render UI for each variable font axis if any are defined.
       hasFontAxes && this.renderAxes(font.variationAxes, axes)
     );
   }
--- a/devtools/client/inspector/fonts/components/FontPropertyValue.js
+++ b/devtools/client/inspector/fonts/components/FontPropertyValue.js
@@ -251,17 +251,17 @@ class FontPropertyValue extends PureComp
     // The unit conversion function will use a 1-to-1 scale for unrecognized units.
     const options = this.props.unitOptions.includes(this.props.unit) ?
       this.props.unitOptions
       :
       this.props.unitOptions.concat([this.props.unit]);
 
     return dom.select(
       {
-        className: "font-unit-select",
+        className: "font-value-select",
         onChange: this.onUnitChange,
       },
       options.map(unit => {
         return dom.option(
           {
             selected: unit === this.props.unit,
             value: unit,
           },
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/components/LineHeight.js
@@ -0,0 +1,78 @@
+/* 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 { getStr } = require("../utils/l10n");
+const { getUnitFromValue, getStepForUnit } = require("../utils/font-utils");
+
+class LineHeight extends PureComponent {
+  static get propTypes() {
+    return {
+      onChange: PropTypes.func.isRequired,
+      value: PropTypes.string.isRequired,
+    };
+  }
+
+  constructor(props) {
+    super(props);
+    this.historicMax = {};
+  }
+
+  render() {
+    const value = parseFloat(this.props.value);
+    // When values for line-height are be unitless, getUnitFromValue() returns null.
+    // In that case, set the unit to an empty string for special treatment in conversion.
+    const unit = getUnitFromValue(this.props.value) || "";
+    let max;
+    switch (unit) {
+      case "":
+      case "em":
+      case "rem":
+        max = 2;
+        break;
+      case "vh":
+      case "vw":
+      case "vmin":
+      case "vmax":
+        max = 15;
+        break;
+      case "%":
+        max = 200;
+        break;
+      default:
+        max = 108;
+        break;
+    }
+
+    // Allow the upper bound to increase so it accomodates the out-of-bounds value.
+    max = Math.max(max, value);
+    // Ensure we store the max value ever reached for this unit type. This will be the
+    // max value of the input and slider. Without this memoization, the value and slider
+    // thumb get clamped at the upper bound while decrementing an out-of-bounds value.
+    this.historicMax[unit] = this.historicMax[unit]
+      ? Math.max(this.historicMax[unit], max)
+      : max;
+
+    return FontPropertyValue({
+      allowAutoIncrement: true,
+      label: getStr("fontinspector.lineHeightLabel"),
+      min: 0,
+      max: this.historicMax[unit],
+      name: "line-height",
+      onChange: this.props.onChange,
+      step: getStepForUnit(unit),
+      unit,
+      unitOptions: ["", "em", "%", "px"],
+      value,
+    });
+  }
+}
+
+module.exports = LineHeight;
--- a/devtools/client/inspector/fonts/components/moz.build
+++ b/devtools/client/inspector/fonts/components/moz.build
@@ -11,9 +11,10 @@ DevToolsModules(
     'FontMeta.js',
     'FontOverview.js',
     'FontPreview.js',
     'FontPropertyValue.js',
     'FontsApp.js',
     'FontSize.js',
     'FontStyle.js',
     'FontWeight.js',
+    'LineHeight.js',
 )
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -39,16 +39,17 @@ const CUSTOM_INSTANCE_NAME = getStr("fon
 const FONT_PROPERTIES = [
   "font-family",
   "font-optical-sizing",
   "font-size",
   "font-stretch",
   "font-style",
   "font-variation-settings",
   "font-weight",
+  "line-height",
 ];
 const PREF_FONT_EDITOR = "devtools.inspector.fonteditor.enabled";
 const REGISTERED_AXES_TO_FONT_PROPERTIES = {
   "ital": "font-style",
   "opsz": "font-optical-sizing",
   "slnt": "font-style",
   "wdth": "font-stretch",
   "wght": "font-weight",
@@ -392,24 +393,30 @@ class FontInspector {
     return properties;
   }
 
   /**
    * Get an array of keyword values supported by the following CSS properties:
    * - font-size
    * - font-weight
    * - font-stretch
+   * - line-height
    *
    * This list is used to filter out values when reading CSS font properties from rules.
    * Computed styles will be used instead of any of these values.
    *
    * @return {Array}
    */
   getFontPropertyValueKeywords() {
-    return ["font-size", "font-weight", "font-stretch"].reduce((acc, property) => {
+    return [
+      "font-size",
+      "font-weight",
+      "font-stretch",
+      "line-height"
+    ].reduce((acc, property) => {
       return acc.concat(this.cssProperties.getValues(property));
     }, []);
   }
 
   async getFontsForNode(node, options) {
     // In case we've been destroyed in the meantime
     if (!this.document) {
       return [];
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -69,8 +69,11 @@ fontinspector.familiesUnusedLabel=Unused
 fontinspector.showMore=Show more
 
 # LOCALIZATION NOTE (fontinspector.showLess): Label for an expanded list of fonts.
 fontinspector.showLess=Show less
 
 # LOCALIZATION NOTE (fontinspector.noPseduoWarning): Warning message shown when the
 # selected element is a pseudo-element that the font editor cannot work with.
 fontinspector.noPseduoWarning=Pseudo-elements are not currently supported.
+
+# LOCALIZATION NOTE (fontinspector.lineHeightLabel): Label for the UI to change the line height in the font editor.
+fontinspector.lineHeightLabel=Line height
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -257,17 +257,18 @@
   margin-left: 3px;
 }
 
 /* Make native number steppers darker to fit the dark theme */
 .theme-dark .font-value-input[type=number]::-moz-number-spin-box {
   filter: invert(25%);
 }
 
-/* Do not show number stepper for font-size */
+/* Do not show number stepper for line height and font-size */
+.font-value-input[name=line-height],
 .font-value-input[name=font-size] {
   -moz-appearance: textfield;
   padding-right: 5px;
   border-right: none;
 }
 
 /* Mock separator because inputs don't have distinguishable borders in dark theme */
 .theme-dark .font-value-input + .font-value-select {
@@ -279,16 +280,17 @@
   background-image: var(--select-arrow-image);
   background-repeat: no-repeat;
   background-position: right 4px center;
   fill: var(--theme-toolbar-photon-icon-color);
   -moz-context-properties: fill;
   -moz-appearance: none;
   box-shadow: none;
   padding: 1px 10px 1px 2px;
+  min-width: 3.8em;
 }
 
 .font-value-select:-moz-focusring {
   color: transparent;
   text-shadow: 0 0 0 var(--input-text-color);
 }
 
 .font-value-input:focus,