Bug 1455335 - Extract font name and origin into reusable component. r=gl draft
authorRazvan Caliman <rcaliman@mozilla.com>
Fri, 20 Apr 2018 17:52:08 +0200
changeset 787899 567883dccf15c81c473c16eafbbfb15af90e1529
parent 787860 7f6a582f00bfb5d0acb8d8bf7f8c79ca37c99b65
push id107831
push userbmo:rcaliman@mozilla.com
push dateWed, 25 Apr 2018 16:08:45 +0000
reviewersgl
bugs1455335
milestone61.0a1
Bug 1455335 - Extract font name and origin into reusable component. r=gl - Extract font name and origin (URL + copy URL) into FontMeta component. - Reuse FontMeta in font overview and font editor. - Tweak CSS for improved spacing and flex-grow behaviour. MozReview-Commit-ID: 4W2E48r8Yps
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontMeta.js
devtools/client/inspector/fonts/components/moz.build
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/Font.js
+++ b/devtools/client/inspector/fonts/components/Font.js
@@ -3,21 +3,19 @@
  * 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 FontPreview = createFactory(require("./FontPreview"));
 
-loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
-
-const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
 class Font extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
@@ -27,33 +25,28 @@ class Font extends PureComponent {
   constructor(props) {
     super(props);
 
     this.state = {
       isFontFaceRuleExpanded: false,
     };
 
     this.onFontFaceRuleToggle = this.onFontFaceRuleToggle.bind(this);
-    this.onCopyURL = this.onCopyURL.bind(this);
   }
 
   componentWillReceiveProps(newProps) {
     if (this.props.font.name === newProps.font.name) {
       return;
     }
 
     this.setState({
       isFontFaceRuleExpanded: false,
     });
   }
 
-  onCopyURL() {
-    clipboardHelper.copyString(this.props.font.URI);
-  }
-
   onFontFaceRuleToggle(event) {
     this.setState({
       isFontFaceRuleExpanded: !this.state.isFontFaceRuleExpanded
     });
     event.stopPropagation();
   }
 
   renderFontCSSCode(rule, ruleText) {
@@ -83,56 +76,16 @@ class Font extends PureComponent {
             className: "font-css-code-expander",
             onClick: this.onFontFaceRuleToggle,
           }
         ),
       trailing
     );
   }
 
-  renderFontOrigin(url) {
-    if (!url) {
-      return dom.p(
-        {
-          className: "font-origin system"
-        },
-        getStr("fontinspector.system")
-      );
-    }
-
-    return dom.p(
-      {
-        className: "font-origin remote",
-      },
-      dom.span(
-        {
-          className: "url",
-          title: url
-        },
-        url
-      ),
-      dom.button(
-        {
-          className: "copy-icon",
-          onClick: this.onCopyURL,
-          title: getStr("fontinspector.copyURL"),
-        }
-      )
-    );
-  }
-
-  renderFontName(name) {
-    return dom.h1(
-      {
-        className: "font-name"
-      },
-      name
-    );
-  }
-
   renderFontCSSCodeTwisty() {
     let { isFontFaceRuleExpanded } = this.state;
 
     let attributes = {
       className: "theme-twisty",
       onClick: this.onFontFaceRuleToggle,
     };
     if (isFontFaceRuleExpanded) {
@@ -147,28 +100,25 @@ class Font extends PureComponent {
       font,
       fontOptions,
       onPreviewFonts,
     } = this.props;
 
     let { previewText } = fontOptions;
 
     let {
-      name,
       previewUrl,
       rule,
       ruleText,
-      URI,
     } = font;
 
     return dom.li(
       {
         className: "font",
       },
-      this.renderFontName(name),
+      FontMeta({ font }),
       FontPreview({ previewText, previewUrl, onPreviewFonts }),
-      this.renderFontOrigin(URI),
       this.renderFontCSSCode(rule, ruleText)
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -4,16 +4,17 @@
 
 "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 FontMeta = createFactory(require("./FontMeta"));
 
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
 class FontEditor extends PureComponent {
   static get propTypes() {
     return {
       fontEditor: PropTypes.shape(Types.fontEditor).isRequired,
@@ -70,16 +71,36 @@ class FontEditor extends PureComponent {
         label: axis.name,
         name: axis.tag,
         onChange: this.props.onAxisUpdate,
         showInput: true
       });
     });
   }
 
+  renderFontFamily(font) {
+    return dom.label(
+      {
+        className: "font-control font-control-family",
+      },
+      dom.span(
+        {
+          className: "font-control-label",
+        },
+        getStr("fontinspector.fontFamilyLabel")
+      ),
+      dom.div(
+        {
+          className: "font-control-box",
+        },
+        FontMeta({ font })
+      )
+    );
+  }
+
   /**
    * Get a dropdown which allows selecting between variation instances defined by a font.
    *
    * @param {Array} fontInstances
    *        Named variation instances as provided with the font file.
    * @param {Object} selectedInstance
    *        Object with information about the currently selected variation instance.
    *        Example:
@@ -123,17 +144,17 @@ class FontEditor extends PureComponent {
     return dom.label(
       {
         className: "font-control",
       },
       dom.span(
         {
           className: "font-control-label",
         },
-        "Instances"
+        getStr("fontinspector.fontInstanceLabel")
       ),
       instanceSelect
     );
   }
 
   // Placeholder for non-variable font UI.
   // Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1450695
   renderPlaceholder() {
@@ -141,26 +162,27 @@ class FontEditor extends PureComponent {
   }
 
   render() {
     const { fonts, axes, instance } = this.props.fontEditor;
     // For MVP use ony first font to show axes if available.
     // Future implementations will allow switching between multiple fonts.
     const font = fonts[0];
     const fontAxes = (font && font.variationAxes) ? font.variationAxes : null;
-    const fontInstances = (font && font.variationInstances) ?
+    const fontInstances = (font && font.variationInstances.length) ?
       font.variationInstances
       :
       null;
 
     return dom.div(
       {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-fontinspector"
       },
+      this.renderFontFamily(font),
       fontInstances && this.renderInstances(fontInstances, instance),
       fontAxes ?
         this.renderAxes(fontAxes, axes)
         :
         this.renderPlaceholder()
     );
   }
 }
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/components/FontMeta.js
@@ -0,0 +1,87 @@
+/* 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 { 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");
+
+loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
+
+const { getStr } = require("../utils/l10n");
+const Types = require("../types");
+
+class FontMeta extends PureComponent {
+  static get propTypes() {
+    return {
+      font: PropTypes.shape(Types.font).isRequired,
+    };
+  }
+
+  constructor(props) {
+    super(props);
+    this.onCopyURL = this.onCopyURL.bind(this);
+  }
+
+  onCopyURL() {
+    clipboardHelper.copyString(this.props.font.URI);
+  }
+
+  renderFontOrigin(url) {
+    if (!url) {
+      return dom.p(
+        {
+          className: "font-origin system"
+        },
+        getStr("fontinspector.system")
+      );
+    }
+
+    return dom.p(
+      {
+        className: "font-origin remote",
+      },
+      dom.span(
+        {
+          className: "url",
+          title: url
+        },
+        url
+      ),
+      dom.button(
+        {
+          className: "copy-icon",
+          onClick: this.onCopyURL,
+          title: getStr("fontinspector.copyURL"),
+        }
+      )
+    );
+  }
+
+  renderFontName(name) {
+    return dom.h1(
+      {
+        className: "font-name"
+      },
+      name
+    );
+  }
+
+  render() {
+    const {
+      name,
+      URI,
+    } = this.props.font;
+
+    return createElement(Fragment,
+      null,
+      this.renderFontName(name),
+      this.renderFontOrigin(URI)
+    );
+  }
+}
+
+module.exports = FontMeta;
--- a/devtools/client/inspector/fonts/components/moz.build
+++ b/devtools/client/inspector/fonts/components/moz.build
@@ -4,12 +4,13 @@
 # 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',
+    'FontMeta.js',
     'FontOverview.js',
     'FontPreview.js',
     'FontsApp.js',
 )
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -28,8 +28,18 @@ fontinspector.editPreview=Click to edit 
 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
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -113,29 +113,40 @@
   display: flex;
   flex-direction: row;
   flex-wrap: nowrap;
   justify-content: space-between;
   align-items: center;
   padding: 5px 20px;
 }
 
+.font-control-family {
+  align-items: flex-start;
+  padding-top: 10px;
+  padding-bottom: 10px;
+}
+
+.font-control-box,
+.font-control-input {
+  flex: 4;
+  min-width: 100px;
+}
+
 .font-control-input {
   display: flex;
   flex-wrap: nowrap;
   align-items: center;
-  flex: 3;
-  max-width: 300px;
-  min-width: 100px;
 }
 
 .font-control-label {
   display: inline-block;
   flex: 1;
+  font-size: 12px;
   min-width: 80px;
+  margin-right: 10px;
 }
 
 .font-axis-input {
   margin-left: 10px;
   width: 60px;
 }
 
 .font-axis-slider {