Bug 1436343 - Simplified the font inspector's CSS; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Wed, 07 Feb 2018 23:07:19 +0100
changeset 402841 79604510fe9acc73136a9f926478ec83a841e699
parent 402840 fd087947d886001f066b7449fa46a28ee3f43766
child 402842 06b5d7476ebd6dd611f1d22c15f3be2d812fa51b
child 402892 0152b5f56940f1f2fb99a278938625235c5040ca
push id33404
push usershindli@mozilla.com
push dateThu, 08 Feb 2018 10:03:18 +0000
treeherdermozilla-central@06b5d7476ebd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1436343
milestone60.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 1436343 - Simplified the font inspector's CSS; r=gl MozReview-Commit-ID: CcvkW8NGh9L
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/test/browser_fontinspector.js
devtools/client/inspector/fonts/test/browser_fontinspector_text-node.js
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/Font.js
+++ b/devtools/client/inspector/fonts/components/Font.js
@@ -13,37 +13,23 @@ const Types = require("../types");
 
 class Font extends PureComponent {
   static get propTypes() {
     return PropTypes.shape(Types.font).isRequired;
   }
 
   constructor(props) {
     super(props);
-    this.getSectionClasses = this.getSectionClasses.bind(this);
     this.renderFontCSS = this.renderFontCSS.bind(this);
     this.renderFontCSSCode = this.renderFontCSSCode.bind(this);
     this.renderFontFormatURL = this.renderFontFormatURL.bind(this);
     this.renderFontName = this.renderFontName.bind(this);
     this.renderFontPreview = this.renderFontPreview.bind(this);
   }
 
-  getSectionClasses() {
-    let { font } = this.props;
-
-    let classes = ["font"];
-    classes.push((font.URI) ? "is-remote" : "is-local");
-
-    if (font.rule) {
-      classes.push("has-code");
-    }
-
-    return classes.join(" ");
-  }
-
   renderFontCSS(cssFamilyName) {
     return dom.p(
       {
         className: "font-css"
       },
       dom.span(
         {},
         getStr("fontinspector.usedAs")
@@ -54,22 +40,22 @@ class Font extends PureComponent {
           className: "font-css-name"
         },
         cssFamilyName
       ),
       "\""
     );
   }
 
-  renderFontCSSCode(rule, ruleText) {
+  renderFontCSSCode(ruleText) {
     return dom.pre(
       {
         className: "font-css-code"
       },
-      rule ? ruleText : null
+      ruleText
     );
   }
 
   renderFontFormatURL(url, format) {
     return dom.p(
       {
         className: "font-format-url"
       },
@@ -129,39 +115,23 @@ class Font extends PureComponent {
       format,
       name,
       previewUrl,
       rule,
       ruleText,
       URI,
     } = font;
 
-    return dom.section(
+    return dom.li(
       {
-        className: this.getSectionClasses(),
+        className: "font",
       },
       this.renderFontPreview(previewUrl),
-      dom.div(
-        {
-          className: "font-info",
-        },
-        this.renderFontName(name),
-        dom.span(
-          {
-            className: "font-is-local",
-          },
-          " " + getStr("fontinspector.system")
-        ),
-        dom.span(
-          {
-            className: "font-is-remote",
-          },
-          " " + getStr("fontinspector.remote")
-        ),
-        this.renderFontFormatURL(URI, format),
-        this.renderFontCSS(CSSFamilyName),
-        this.renderFontCSSCode(rule, ruleText)
-      )
+      this.renderFontName(name),
+      " " + (URI ? getStr("fontinspector.remote") : getStr("fontinspector.system")),
+      URI ? this.renderFontFormatURL(URI, format) : null,
+      this.renderFontCSS(CSSFamilyName),
+      rule ? this.renderFontCSSCode(ruleText) : null
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/test/browser_fontinspector.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector.js
@@ -38,71 +38,76 @@ add_task(function* () {
 
   let viewDoc = view.document;
 
   yield testBodyFonts(inspector, viewDoc);
   yield testDivFonts(inspector, viewDoc);
   yield testShowAllFonts(inspector, viewDoc);
 });
 
+function isRemote(fontLiEl) {
+  return fontLiEl.querySelectorAll(".font-format-url").length === 1;
+}
+
 function* testBodyFonts(inspector, viewDoc) {
-  let s = viewDoc.querySelectorAll("#all-fonts > section");
-  is(s.length, 5, "Found 5 fonts");
+  let lis = viewDoc.querySelectorAll("#all-fonts > li");
+  is(lis.length, 5, "Found 5 fonts");
 
   for (let i = 0; i < FONTS.length; i++) {
-    let section = s[i];
+    let li = lis[i];
     let font = FONTS[i];
-    is(section.querySelector(".font-name").textContent, font.name,
+    is(li.querySelector(".font-name").textContent, font.name,
        "font " + i + " right font name");
-    is(section.classList.contains("is-remote"), font.remote,
+
+    is(isRemote(li), font.remote,
        "font " + i + " remote value correct");
-    is(section.querySelector(".font-url").value, font.url,
+    is(li.querySelector(".font-url").value, font.url,
        "font " + i + " url correct");
-    is(section.querySelector(".font-format").hidden, !font.format,
+    is(li.querySelector(".font-format").hidden, !font.format,
        "font " + i + " format hidden value correct");
-    is(section.querySelector(".font-format").textContent,
+    is(li.querySelector(".font-format").textContent,
        font.format, "font " + i + " format correct");
-    is(section.querySelector(".font-css-name").textContent,
+    is(li.querySelector(".font-css-name").textContent,
        font.cssName, "font " + i + " css name correct");
   }
 
   // test that the bold and regular fonts have different previews
-  let regSrc = s[0].querySelector(".font-preview").src;
-  let boldSrc = s[1].querySelector(".font-preview").src;
+  let regSrc = lis[0].querySelector(".font-preview").src;
+  let boldSrc = lis[1].querySelector(".font-preview").src;
   isnot(regSrc, boldSrc, "preview for bold font is different from regular");
 
   // test system font
-  let localFontName = s[4].querySelector(".font-name").textContent;
-  let localFontCSSName = s[4].querySelector(".font-css-name").textContent;
+  let localFontName = lis[4].querySelector(".font-name").textContent;
+  let localFontCSSName = lis[4].querySelector(".font-css-name").textContent;
 
   // On Linux test machines, the Arial font doesn't exist.
   // The fallback is "Liberation Sans"
   ok((localFontName == "Arial") || (localFontName == "Liberation Sans"),
      "local font right font name");
-  ok(s[4].classList.contains("is-local"), "local font is local");
+  ok(!isRemote(lis[4]), "local font is local");
   ok((localFontCSSName == "Arial") || (localFontCSSName == "Liberation Sans"),
      "Arial", "local font has right css name");
 }
 
 function* testDivFonts(inspector, viewDoc) {
   let updated = inspector.once("fontinspector-updated");
   yield selectNode("div", inspector);
   yield updated;
 
-  let sections1 = viewDoc.querySelectorAll("#all-fonts > section");
-  is(sections1.length, 1, "Found 1 font on DIV");
-  is(sections1[0].querySelector(".font-name").textContent,
+  let lis = viewDoc.querySelectorAll("#all-fonts > li");
+  is(lis.length, 1, "Found 1 font on DIV");
+  is(lis[0].querySelector(".font-name").textContent,
      "Ostrich Sans Medium",
      "The DIV font has the right name");
 }
 
 function* testShowAllFonts(inspector, viewDoc) {
   info("testing showing all fonts");
 
   let updated = inspector.once("fontinspector-updated");
   viewDoc.querySelector("#font-showall").click();
   yield updated;
 
   // shouldn't change the node selection
   is(inspector.selection.nodeFront.nodeName, "DIV", "Show all fonts selected");
-  let sections = viewDoc.querySelectorAll("#all-fonts > section");
-  is(sections.length, 6, "Font inspector shows 6 fonts (1 from iframe)");
+  let lis = viewDoc.querySelectorAll("#all-fonts > li");
+  is(lis.length, 6, "Font inspector shows 6 fonts (1 from iframe)");
 }
--- a/devtools/client/inspector/fonts/test/browser_fontinspector_text-node.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_text-node.js
@@ -12,11 +12,11 @@ add_task(function* () {
   let { inspector, view } = yield openFontInspectorForURL(TEST_URI);
   let viewDoc = view.document;
 
   info("Selecting the first text-node first-child of <body>");
   let bodyNode = yield getNodeFront("body", inspector);
   let { nodes } = yield inspector.walker.children(bodyNode);
   yield selectNode(nodes[0], inspector);
 
-  let sections = viewDoc.querySelectorAll("#all-fonts > section");
+  let sections = viewDoc.querySelectorAll("#all-fonts > li");
   is(sections.length, 1, "Font inspector shows 1 font");
 });
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -28,30 +28,16 @@
   cursor: pointer;
   flex-shrink: 0;
 }
 
 #font-showall:hover {
   text-decoration: underline;
 }
 
-.dim > #font-container,
-.font:not(.has-code) .font-css-code,
-.font-is-local,
-.font-is-remote,
-.font.is-local .font-format-url,
-#font-template {
-  display: none;
-}
-
-.font.is-remote .font-is-remote,
-.font.is-local .font-is-local {
-  display: inline;
-}
-
 .font-format::before {
   content: "(";
 }
 
 .font-format::after {
   content: ")";
 }
 
@@ -93,20 +79,16 @@
 }
 
 .font-preview {
   margin-left: -4px;
   height: 60px;
   display: block;
 }
 
-.font-info {
-  display: block;
-}
-
 .font-name {
   display: inline;
 }
 
 .font-css-code {
   max-width: 100%;
   overflow: hidden;
   text-overflow: ellipsis;