Bug 1483929 - (Part 4) Group used fonts by family name. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 22 Aug 2018 09:09:41 +0000
changeset 433148 ec5152902eb717a568b34accf03f2a520862c512
parent 433147 ab010295820f0f4b71d6c3d081dcd7da0c4e2f07
child 433149 15ae8f3946b48c38be495fee53bd2fb049ad898b
push id34500
push usertoros@mozilla.com
push dateFri, 24 Aug 2018 09:42:33 +0000
treeherdermozilla-central@043aff7fda61 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1483929
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 1483929 - (Part 4) Group used fonts by family name. r=gl This changes the structure of the used fonts list. It groups fonts by family name and removes URL and copy capabilities. Depends on D3805 Differential Revision: https://phabricator.services.mozilla.com/D3894
devtools/client/inspector/fonts/components/FontEditor.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
@@ -3,17 +3,16 @@
  * 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 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");
@@ -89,86 +88,91 @@ class FontEditor extends PureComponent {
         step: this.getAxisStep(axis.minValue, axis.maxValue),
         unit: null,
         value: editedAxes[axis.tag] || axis.defaultValue,
       });
     });
   }
 
   /**
-   * Render font family, font name, and metadata for all fonts used on selected node.
+   * Render fonts used on the selected node grouped by font-family.
    *
    * @param {Array} fonts
    *        Fonts used on selected node.
    * @return {DOMNode}
    */
-  renderFontFamily(fonts) {
+  renderUsedFonts(fonts) {
     if (!fonts.length) {
       return null;
     }
 
-    const topUsedFontsList = this.renderFontList(fonts.slice(0, MAX_FONTS));
-    const moreUsedFontsList = this.renderFontList(fonts.slice(MAX_FONTS, fonts.length));
-    const moreUsedFonts = moreUsedFontsList === null
+    // Group fonts by family name.
+    const fontGroups = fonts.reduce((acc, font) => {
+      const family = font.CSSFamilyName.toString();
+      acc[family] = acc[family] || [];
+      acc[family].push(font);
+      return acc;
+    }, {});
+
+    const renderedFontGroups = Object.keys(fontGroups).map(family => {
+      return this.renderFontGroup(family, fontGroups[family]);
+    });
+
+    const topFontsList = renderedFontGroups.slice(0, MAX_FONTS);
+    const moreFontsList = renderedFontGroups.slice(MAX_FONTS, renderedFontGroups.length);
+
+    const moreFonts = !moreFontsList.length
       ? null
       : dom.details({},
           dom.summary({},
             dom.span({ className: "label-open" }, getStr("fontinspector.showMore")),
             dom.span({ className: "label-close" }, getStr("fontinspector.showLess"))
           ),
-          moreUsedFontsList
+          moreFontsList
         );
 
     return dom.label(
       {
-        className: "font-control font-control-family",
+        className: "font-control font-control-used-fonts",
       },
       dom.span(
         {
           className: "font-control-label",
         },
-        getStr("fontinspector.fontFamilyLabel")
+        getStr("fontinspector.usedFontsLabel")
       ),
       dom.div(
         {
           className: "font-control-box",
         },
-        topUsedFontsList,
-        moreUsedFonts
+        topFontsList,
+        moreFonts
       )
     );
   }
 
-  /**
-   * Given an array of fonts, get an unordered list with rendered FontMeta components.
-   * If the array of fonts is empty, return null.
-   *
-   * @param {Array} fonts
-   *        Array of objects with information about fonts used on the selected node.
-   * @return {DOMNode|null}
-   */
-  renderFontList(fonts = []) {
-    if (!fonts.length) {
-      return null;
-    }
+  renderFontGroup(family, fonts = []) {
+    const group = fonts.map(font => {
+      return dom.span(
+        {
+          className: "font-name",
+        },
+        font.name);
+    });
 
-    return dom.ul(
+    return dom.div(
       {
-        className: "fonts-list"
+        className: "font-group"
       },
-      fonts.map(font => {
-        return dom.li(
-          {},
-          FontMeta({
-            font,
-            key: font.name,
-            onToggleFontHighlight: this.props.onToggleFontHighlight
-          })
-        );
-      })
+      dom.div(
+        {
+          className: "font-family-name"
+        },
+        family),
+      group
     );
   }
 
   renderFontSize(value) {
     return value && FontSize({
       key: `${this.props.fontEditor.id}:font-size`,
       onChange: this.props.onPropertyChange,
       value,
@@ -289,18 +293,18 @@ class FontEditor extends PureComponent {
     if (!font) {
       return this.renderWarning(warning);
     }
 
     return dom.div(
       {
         id: "font-editor"
       },
-      // Always render UI for font family, format and font file URL.
-      this.renderFontFamily(fonts),
+      // Always render UI for used fonts.
+      this.renderUsedFonts(fonts),
       // 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"]),
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -29,20 +29,16 @@ fontinspector.copyURL=Copy URL
 
 # LOCALIZATION NOTE (fontinspector.customInstanceName): Think of instances as presets
 # (groups of settings that apply in bulk to a thing). Instances have names. When the user
 # creates a new instance, it doesn't have a name. This is the text that appears as the
 # default name for a new instance. It shows up in a dropdown from which users can select
 # between predefined instances and this custom instance.
 fontinspector.customInstanceName=Custom
 
-# LOCALIZATION NOTE (fontinspector.fontFamilyLabel): This label is shown next to the
-# string that identifies the font family currently being edited in the font editor.
-fontinspector.fontFamilyLabel=Family
-
 # LOCALIZATION NOTE (fontinspector.fontInstanceLabel): This label is shown next to the UI
 # in the font editor which allows a user to select a font instance option from a
 # dropdown. An instance is like a preset. A "font instance" is the term used by the font
 # authors to mean a group of predefined font settings.
 fontinspector.fontInstanceLabel=Instance
 
 # LOCALIZATION NOTE (fontinspector.fontSizeLabel): This label is shown next to the UI
 # in the font editor which allows the user to change the font size.
@@ -67,8 +63,12 @@ fontinspector.showLess=Show less
 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
 
 # LOCALIZATION NOTE (fontinspector.allFontsOnPageHeader): Header for the section listing
 # all the fonts on the current page.
 fontinspector.allFontsOnPageHeader=All fonts on page
+
+# LOCALIZATION NOTE (fontinspector.usedFontsLabel): Label for the Font Editor section
+# which shows the fonts used on the selected element.
+fontinspector.usedFontsLabel=Fonts used
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -138,16 +138,34 @@
   color: var(--grey-50);
 }
 
 .font-family-name {
   margin-bottom: 0.2em;
   font-size: 1.2em;
 }
 
+.font-group {
+  margin-bottom: .5em;
+}
+
+.font-group .font-name {
+  display: inline-block;
+  white-space: unset;
+  margin-right: .5em;
+}
+
+.font-group .font-name::after {
+  content: ",";
+}
+
+.font-group .font-name:nth-last-child(1)::after {
+  content: "";
+}
+
 .font-css-code {
   direction: ltr;
   margin: 0;
   overflow: hidden;
   text-overflow: ellipsis;
   color: var(--theme-toolbar-color);
   grid-column: span 2;
   position: relative;
@@ -187,17 +205,17 @@
 
 /* Remove styles form all axis controls aside from the first one.
    Workaround for :first-of-type which doesn't work with class names. */
 .font-control-axis ~ .font-control-axis {
   border-top: unset;
   padding-top: unset;
 }
 
-.font-control-family {
+.font-control-used-fonts {
   align-items: flex-start;
   border-bottom: 1px solid var(--theme-splitter-color);
   margin-top: 0;
   margin-bottom: 1em;
   padding-top: 1em;
 }
 
 .font-control-box,