Bug 1464796 - (Part 6) Pick a font from descendants if no declared font was used on node. r=pbro draft
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 14 Jun 2018 09:02:11 -0700
changeset 808655 e2ddbec7fc1d4efc407ad4173e96dc28629ddeea
parent 808654 2e223009c630c73a0012bb202ddb81fe856a1c44
child 808663 27f4a225036f7072cce0aa19870209b763fa3dd4
push id113452
push userbmo:rcaliman@mozilla.com
push dateWed, 20 Jun 2018 09:12:20 +0000
reviewerspbro
bugs1464796
milestone62.0a1
Bug 1464796 - (Part 6) Pick a font from descendants if no declared font was used on node. r=pbro MozReview-Commit-ID: 78NMQovDQLU
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontsApp.js
devtools/client/inspector/fonts/fonts.js
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -84,16 +84,20 @@ class FontEditor extends PureComponent {
    *
    * @param {Array} fonts
    *        Fonts used on selected node.
    * @param {Function} onToggleFontHighlight
    *        Callback to trigger in-context highlighting of text that uses a font.
    * @return {DOMNode}
    */
   renderFontFamily(fonts, onToggleFontHighlight) {
+    if (!fonts.length) {
+      return null;
+    }
+
     const fontList = dom.ul(
       {
         className: "fonts-list"
       },
       fonts.map(font => {
         return dom.li(
           {},
           FontMeta({ font, onToggleFontHighlight })
@@ -116,31 +120,31 @@ class FontEditor extends PureComponent {
           className: "font-control-box",
         },
         fontList
       )
     );
   }
 
   renderFontSize(value) {
-    return FontSize({
+    return value && FontSize({
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   renderFontStyle(value) {
-    return FontStyle({
+    return value && FontStyle({
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   renderFontWeight(value) {
-    return FontWeight({
+    return value && FontWeight({
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   /**
    * Get a dropdown which allows selecting between variation instances defined by a font.
    *
@@ -195,35 +199,48 @@ class FontEditor extends PureComponent {
           className: "font-control-label",
         },
         getStr("fontinspector.fontInstanceLabel")
       ),
       instanceSelect
     );
   }
 
+  renderWarning() {
+    return dom.div(
+      {
+        className: "devtools-sidepanel-no-result"
+      },
+      getStr("fontinspector.noFontsOnSelectedElement")
+    );
+  }
+
   render() {
     const { fontEditor, onToggleFontHighlight } = this.props;
     const { fonts, axes, instance, properties } = fontEditor;
     // Pick the first font to show editor controls regardless of how many fonts are used.
     const font = fonts[0];
     const hasFontAxes = font && font.variationAxes;
     const hasFontInstances = font && font.variationInstances
       && font.variationInstances.length > 0;
     const hasSlantOrItalicAxis = hasFontAxes && font.variationAxes.find(axis => {
       return axis.tag === "slnt" || axis.tag === "ital";
     });
     const hasWeightAxis = hasFontAxes && font.variationAxes.find(axis => {
       return axis.tag === "wght";
     });
+    // Check for falsy font-weight value (undefined or empty string).
+    const hasWeight = properties["font-weight"] != null;
 
     return dom.div(
       {
         id: "font-editor"
       },
+      // Render empty state message for nodes that don't have font properties.
+      !hasWeight && this.renderWarning(),
       // Always render UI for font family, format and font file URL.
       this.renderFontFamily(fonts, onToggleFontHighlight),
       // 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"]),
       // Render UI for font weight if no "wght" registered axis is defined.
       !hasWeightAxis && this.renderFontWeight(properties["font-weight"]),
--- a/devtools/client/inspector/fonts/components/FontsApp.js
+++ b/devtools/client/inspector/fonts/components/FontsApp.js
@@ -33,24 +33,22 @@ class FontsApp extends PureComponent {
       fontEditor,
       fontOptions,
       onInstanceChange,
       onPreviewFonts,
       onPropertyChange,
       onToggleFontHighlight,
     } = this.props;
 
-    const hasFonts = fontEditor.fonts.length > 0;
-
     return dom.div(
       {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-fontinspector"
       },
-      hasFonts && FontEditor({
+      FontEditor({
         fontEditor,
         onInstanceChange,
         onPropertyChange,
         onToggleFontHighlight,
       }),
       FontOverview({
         fontData,
         fontOptions,
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -640,20 +640,16 @@ class FontInspector {
 
     const options = {};
     if (this.pageStyle.supportsFontVariations) {
       options.includeVariations = true;
     }
 
     const node = this.inspector.selection.nodeFront;
     const fonts = await this.getFontsForNode(node, options);
-    if (!fonts.length) {
-      this.store.dispatch(resetFontEditor());
-      return;
-    }
 
     // Get computed styles for the selected node, but filter by CSS font properties.
     this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
       filterProperties: FONT_PROPERTIES
     });
 
     if (!this.nodeComputedStyle) {
       this.store.dispatch(resetFontEditor());
@@ -667,25 +663,36 @@ class FontInspector {
     this.selectedRule =
       this.ruleView.rules.find(rule => rule.domRule.type === ELEMENT_STYLE);
 
     const properties = this.getFontProperties();
     const familiesDeclared =
       properties["font-family"].split(",")
       .map(font => font.replace(/["']+/g, "").trim());
     // Subset of fonts used on the node whose family names exist in CSS font-family.
-    const fontsUsed = this.filterFontsUsed(fonts, familiesDeclared);
+    let fontsUsed = this.filterFontsUsed(fonts, familiesDeclared);
     // Object with font families groupped by used and not used.
     const families = this.groupFontFamilies(fontsUsed, familiesDeclared);
     // Assign writer methods to each axis defined in font-variation-settings.
     const axes = parseFontVariationAxes(properties["font-variation-settings"]);
     Object.keys(axes).map(axis => {
       this.writers.set(axis, this.getWriterForAxis(axis));
     });
 
+    // Pick fonts from descendants if no declared fonts were used on this node.
+    if (!fontsUsed.length && fonts.length) {
+      const otherVarFonts = fonts.filter(font => {
+        return (font.variationAxes && font.variationAxes.length);
+      });
+
+      // Prefer picking variable fonts if any were found on descendants of this node.
+      // The FontEditor component will render UI for the first font in the list.
+      fontsUsed = otherVarFonts.length ? otherVarFonts : fonts;
+    }
+
     this.store.dispatch(updateFontEditor(fontsUsed, families, properties));
     this.inspector.emit("fonteditor-updated");
   }
 
   /**
    * Capture the state of all variation axes. Allows the user to return to this state with
    * the "Custom" instance after they've selected a font-defined named variation instance.
    * This method is debounced. See constructor.