acked out changeset ee4b2d3fd97a (bug 1488031) for failing dt at devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Thu, 13 Sep 2018 18:38:11 +0300
changeset 436166 11a9019dbafa1b066bcbcace23ddbbdcfdd4d9a5
parent 436165 8a13d8a20947908486d7e74e8f5cbdef95a30daf
child 436167 d965284aca1416b104471807b2cb48b85de0bf59
push id69232
push userapavel@mozilla.com
push dateThu, 13 Sep 2018 15:38:29 +0000
treeherderautoland@11a9019dbafa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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
acked out changeset ee4b2d3fd97a (bug 1488031) for failing dt at devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js on a CLOSED TREE
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,19 +48,17 @@ 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,18 +50,16 @@ 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,51 +8,42 @@ 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,
-      // Whether to show the `name` prop value as an extra label (used to show axis tags).
-      nameLabel: PropTypes.bool,
+      nameLabel: PropTypes.oneOfType([PropTypes.string, 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) {
@@ -98,32 +89,16 @@ 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}
@@ -399,24 +374,25 @@ class FontPropertyValue extends PureComp
     const labelEl = dom.span(
       {
         className: "font-control-label-text",
         "aria-describedby": nameLabel ? `detail-${name}` : null,
       },
       label
     );
 
-    // Show the `name` prop value as an additional label if the `nameLabel` prop is true.
+    // 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}`
         },
-        this.getPropLabel("name")
+        typeof nameLabel === "boolean" ? name : nameLabel
       )
       :
       null;
 
     return createElement(Fragment, null, labelEl, detailEl);
   }
 
   render() {
@@ -477,24 +453,17 @@ class FontPropertyValue extends PureComp
           title: this.props.label,
         },
         this.renderLabelContent()
       ),
       dom.div(
         {
           className: "font-control-input"
         },
-        dom.div(
-          {
-            className: "font-value-slider-container",
-            "data-min": this.getPropLabel("min"),
-            "data-max": this.getPropLabel("max"),
-          },
-          range
-        ),
+        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,27 +87,10 @@ 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,31 +1,29 @@
 /* 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 {
-  --highlight-color: var(--blue-55);
+  --slider-thumb-color: var(--grey-50);
+  --slider-track-color: var(--grey-30);
   --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%;
@@ -232,17 +230,17 @@
 .font-control-label-text {
   display: block;
   overflow: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
 }
 
 .font-control-label-detail {
-  color: var(--secondary-label-color);
+  color: var(--grey-45);
   font-size: smaller;
   text-transform: none;
 }
 
 .font-value-input {
   margin-left: 10px;
   text-align: right;
   width: 60px;
@@ -305,57 +303,24 @@
 
 .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(--highlight-color);
+  outline: 1px solid var(--blue-55);
   outline-offset: -1px;
 }
 
-.font-value-slider-container {
+.font-value-slider {
   flex: 1;
+  margin: 0;
   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;
 }
 
 /*
   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;
@@ -379,17 +344,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(--highlight-color);
+  background-color: var(--blue-55);
 }
 
 .font-value-slider::-moz-range-track {
   background-color: var(--slider-track-color);
   height: 3px;
 }
 
 .font-origin {