Bug 1463103 - Font Highlighter: Default isForCurrentElement to true and cleanup from component props. r=pbro draft
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 21 May 2018 15:40:28 +0200
changeset 797720 a5d11b6b757f58c1aa5c768baaf4719569da814c
parent 797719 646c56ea0ac8ca2b70b23b4d557650aeac1d3674
push id110546
push userbmo:rcaliman@mozilla.com
push dateMon, 21 May 2018 13:41:04 +0000
reviewerspbro
bugs1463103
milestone62.0a1
Bug 1463103 - Font Highlighter: Default isForCurrentElement to true and cleanup from component props. r=pbro MozReview-Commit-ID: J9w6bGwXBNt
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontList.js
devtools/client/inspector/fonts/components/FontMeta.js
devtools/client/inspector/fonts/components/FontOverview.js
devtools/client/inspector/fonts/fonts.js
--- a/devtools/client/inspector/fonts/components/Font.js
+++ b/devtools/client/inspector/fonts/components/Font.js
@@ -13,17 +13,16 @@ const FontPreview = createFactory(requir
 
 const Types = require("../types");
 
 class Font extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
-      isForCurrentElement: PropTypes.bool.isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
@@ -96,33 +95,32 @@ class Font extends PureComponent {
 
     return dom.span(attributes);
   }
 
   render() {
     let {
       font,
       fontOptions,
-      isForCurrentElement,
       onPreviewFonts,
       onToggleFontHighlight,
     } = this.props;
 
     let { previewText } = fontOptions;
 
     let {
       previewUrl,
       rule,
       ruleText,
     } = font;
 
     return dom.li(
       {
         className: "font",
       },
-      FontMeta({ font, isForCurrentElement, onToggleFontHighlight }),
+      FontMeta({ font, onToggleFontHighlight }),
       FontPreview({ previewText, previewUrl, onPreviewFonts }),
       this.renderFontCSSCode(rule, ruleText)
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -90,17 +90,17 @@ class FontEditor extends PureComponent {
           className: "font-control-label",
         },
         getStr("fontinspector.fontFamilyLabel")
       ),
       dom.div(
         {
           className: "font-control-box",
         },
-        FontMeta({ font, onToggleFontHighlight, isForCurrentElement: true })
+        FontMeta({ font, onToggleFontHighlight })
       )
     );
   }
 
   renderFontSize(value) {
     return FontSize({
       onChange: this.props.onPropertyChange,
       value,
--- a/devtools/client/inspector/fonts/components/FontList.js
+++ b/devtools/client/inspector/fonts/components/FontList.js
@@ -12,40 +12,37 @@ const Font = createFactory(require("./Fo
 
 const Types = require("../types");
 
 class FontList extends PureComponent {
   static get propTypes() {
     return {
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       fonts: PropTypes.arrayOf(PropTypes.shape(Types.font)).isRequired,
-      isCurrentElementFonts: PropTypes.bool.isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   render() {
     let {
       fonts,
       fontOptions,
-      isCurrentElementFonts,
       onPreviewFonts,
       onToggleFontHighlight
     } = this.props;
 
     return dom.ul(
       {
         className: "fonts-list"
       },
       fonts.map((font, i) => Font({
         key: i,
         font,
         fontOptions,
-        isForCurrentElement: isCurrentElementFonts,
         onPreviewFonts,
         onToggleFontHighlight,
       }))
     );
   }
 }
 
 module.exports = FontList;
--- a/devtools/client/inspector/fonts/components/FontMeta.js
+++ b/devtools/client/inspector/fonts/components/FontMeta.js
@@ -16,17 +16,16 @@ const Services = require("Services");
 const Types = require("../types");
 
 const FONT_HIGHLIGHTER_PREF = "devtools.inspector.fonthighlighter.enabled";
 
 class FontMeta extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
-      isForCurrentElement: PropTypes.bool.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
     this.onCopyURL = this.onCopyURL.bind(this);
     this.onNameMouseOver = this.onNameMouseOver.bind(this);
@@ -35,31 +34,29 @@ class FontMeta extends PureComponent {
 
   onCopyURL() {
     clipboardHelper.copyString(this.props.font.URI);
   }
 
   onNameMouseOver() {
     let {
       font,
-      isForCurrentElement,
       onToggleFontHighlight,
     } = this.props;
 
-    onToggleFontHighlight(font, true, isForCurrentElement);
+    onToggleFontHighlight(font, true);
   }
 
   onNameMouseOut() {
     let {
       font,
-      isForCurrentElement,
       onToggleFontHighlight,
     } = this.props;
 
-    onToggleFontHighlight(font, false, isForCurrentElement);
+    onToggleFontHighlight(font, false);
   }
 
   renderFontOrigin(url) {
     if (!url) {
       return dom.p(
         {
           className: "font-origin system"
         },
--- a/devtools/client/inspector/fonts/components/FontOverview.js
+++ b/devtools/client/inspector/fonts/components/FontOverview.js
@@ -19,30 +19,36 @@ class FontOverview extends PureComponent
     return {
       fontData: PropTypes.shape(Types.fontData).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
+  constructor(props) {
+    super(props);
+    this.onToggleFontHighlightGlobal = (font, show) => {
+      this.props.onToggleFontHighlight(font, show, false);
+    };
+  }
+
   renderElementFonts() {
     let {
       fontData,
       fontOptions,
       onPreviewFonts,
       onToggleFontHighlight,
     } = this.props;
     let { fonts } = fontData;
 
     return fonts.length ?
       FontList({
         fonts,
         fontOptions,
-        isCurrentElementFonts: true,
         onPreviewFonts,
         onToggleFontHighlight,
       })
       :
       dom.div(
         {
           className: "devtools-sidepanel-no-result"
         },
@@ -50,35 +56,33 @@ class FontOverview extends PureComponent
       );
   }
 
   renderOtherFonts() {
     let {
       fontData,
       fontOptions,
       onPreviewFonts,
-      onToggleFontHighlight,
     } = this.props;
     let { otherFonts } = fontData;
 
     if (!otherFonts.length) {
       return null;
     }
 
     return Accordion({
       items: [
         {
           header: getStr("fontinspector.otherFontsInPageHeader"),
           component: FontList,
           componentProps: {
             fontOptions,
             fonts: otherFonts,
-            isCurrentElementFonts: false,
             onPreviewFonts,
-            onToggleFontHighlight,
+            onToggleFontHighlight: this.onToggleFontHighlightGlobal
           },
           opened: false
         }
       ]
     });
   }
 
   render() {
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -472,19 +472,20 @@ class FontInspector {
   /**
    * Reveal a font's usage in the page.
    *
    * @param  {String} font
    *         The name of the font to be revealed in the page.
    * @param  {Boolean} show
    *         Whether or not to reveal the font.
    * @param  {Boolean} isForCurrentElement
-   *         Whether or not to reveal the font for the current element selection.
+   *         Optional. Default `true`. Whether or not to restrict revealing the font
+   *         just to the current element selection.
    */
-  async onToggleFontHighlight(font, show, isForCurrentElement) {
+  async onToggleFontHighlight(font, show, isForCurrentElement = true) {
     if (!this.fontsHighlighter) {
       try {
         this.fontsHighlighter = await this.inspector.toolbox.highlighterUtils
                                           .getHighlighterByType("FontsHighlighter");
       } catch (e) {
         // When connecting to an older server or when debugging a XUL document, the
         // FontsHighlighter won't be available. Silently fail here and prevent any future
         // calls to the function.