Bug 1464336 - Font Editor: Show font family AND font name in editor and overview. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 20 Jun 2018 12:09:17 +0200
changeset 477472 7110e2638671135ef6149864614f35355494c6f3
parent 477471 686fc84d4ffbe8d1bef16767fdd8c867479258a0
child 477473 8819c39005301de2d6f97b2e828e89096972ea05
push id9385
push userdluca@mozilla.com
push dateFri, 22 Jun 2018 15:47:18 +0000
treeherdermozilla-beta@82a9a1027e2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1464336
milestone62.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 1464336 - Font Editor: Show font family AND font name in editor and overview. r=pbro - Show font family name as prominent identifier. - Show font name underneath family in a smaller type. - Update test to match correct strings. MozReview-Commit-ID: FWqSdXi4HiY
devtools/client/inspector/fonts/components/FontMeta.js
devtools/client/inspector/fonts/test/browser_fontinspector.js
devtools/client/inspector/fonts/test/head.js
devtools/client/inspector/fonts/types.js
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/FontMeta.js
+++ b/devtools/client/inspector/fonts/components/FontMeta.js
@@ -80,38 +80,41 @@ class FontMeta extends PureComponent {
           className: "copy-icon",
           onClick: this.onCopyURL,
           title: getStr("fontinspector.copyURL"),
         }
       )
     );
   }
 
-  renderFontName(name) {
+  renderFontName(name, family) {
+    let options = {};
+
     if (Services.prefs.getBoolPref(FONT_HIGHLIGHTER_PREF)) {
-      return dom.h1(
-        {
-          className: "font-name",
-          onMouseOver: this.onNameMouseOver,
-          onMouseOut: this.onNameMouseOut,
-        },
-        name
-      );
+      options = {
+        onMouseOver: this.onNameMouseOver,
+        onMouseOut: this.onNameMouseOut,
+      };
     }
 
-    return dom.h1({ className: "font-name" }, name);
+    return dom.div(
+      options,
+      dom.div({ className: "font-family-name" }, family),
+      dom.div({ className: "font-name" }, name)
+    );
   }
 
   render() {
     const {
+      CSSFamilyName,
       name,
       URI,
     } = this.props.font;
 
     return createElement(Fragment,
       null,
-      this.renderFontName(name),
+      this.renderFontName(name, CSSFamilyName),
       this.renderFontOrigin(URI)
     );
   }
 }
 
 module.exports = FontMeta;
--- a/devtools/client/inspector/fonts/test/browser_fontinspector.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector.js
@@ -1,12 +1,12 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
-/* global getURL */
+/* global getURL, getFamilyName */
 "use strict";
 
 requestLongerTimeout(2);
 
 const TEST_URI = URL_ROOT + "browser_fontinspector.html";
 
 add_task(async function() {
   await pushPref("devtools.inspector.fonteditor.enabled", true);
@@ -33,23 +33,49 @@ async function testBodyFonts(inspector, 
   // The fallback is "Liberation Sans"
   is(lis.length, 1, "Found 1 font on BODY");
   ok((localFontName == "Arial") || (localFontName == "Liberation Sans"),
      "local font right font name");
   ok(!isRemote(lis[0]), "local font is local");
 }
 
 async function testDivFonts(inspector, viewDoc) {
-  await selectNode("div", inspector);
-
-  const font = {
+  const FONTS = [{
+    selector: "div",
+    familyName: "bar",
+    name: "Ostrich Sans Medium",
+    remote: true,
+    url: URL_ROOT + "ostrich-regular.ttf",
+  },
+  {
+    selector: ".normal-text",
+    familyName: "barnormal",
     name: "Ostrich Sans Medium",
     remote: true,
     url: URL_ROOT + "ostrich-regular.ttf",
-  };
+  },
+  {
+    selector: ".bold-text",
+    familyName: "bar",
+    name: "Ostrich Sans Black",
+    remote: true,
+    url: URL_ROOT + "ostrich-black.ttf",
+  }, {
+    selector: ".black-text",
+    familyName: "bar",
+    name: "Ostrich Sans Black",
+    remote: true,
+    url: URL_ROOT + "ostrich-black.ttf",
+  }];
 
-  const lis = getUsedFontsEls(viewDoc);
-  const li = lis[0];
-  is(lis.length, 1, "Found 1 font on DIV");
-  is(getName(li), font.name, "The DIV font has the right name");
-  is(isRemote(li), font.remote, `font remote value correct`);
-  is(getURL(li), font.url, `font url correct`);
+  for (let i = 0; i < FONTS.length; i++) {
+    await selectNode(FONTS[i].selector, inspector);
+    const lis = getUsedFontsEls(viewDoc);
+    const li = lis[0];
+    const font = FONTS[i];
+
+    is(lis.length, 1, `Found 1 font on ${FONTS[i].selector}`);
+    is(getName(li), font.name, "The DIV font has the right name");
+    is(getFamilyName(li), font.familyName, `font has the right family name`);
+    is(isRemote(li), font.remote, `font remote value correct`);
+    is(getURL(li), font.url, `font url correct`);
+  }
 }
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -147,8 +147,20 @@ function getName(fontEl) {
  * @param  {DOMNode} fontEl
  *         The font element.
  * @return {String}
  *         The URL where the font was loaded from as shown in the UI.
  */
 function getURL(fontEl) {
   return fontEl.querySelector(".font-origin").textContent;
 }
+
+/**
+ * Given a font element, return its family name.
+ *
+ * @param  {DOMNode} fontEl
+ *         The font element.
+ * @return {String}
+ *         The name of the font family as shown in the UI.
+ */
+function getFamilyName(fontEl) {
+  return fontEl.querySelector(".font-family-name").textContent;
+}
--- a/devtools/client/inspector/fonts/types.js
+++ b/devtools/client/inspector/fonts/types.js
@@ -44,16 +44,19 @@ const fontVariationInstance = exports.fo
   // The font variation values for the variation instance of the font
   values: PropTypes.arrayOf(PropTypes.shape(fontVariationInstanceValue)),
 };
 
 /**
  * A single font.
  */
 const font = exports.font = {
+  // Font family name
+  CSSFamilyName: PropTypes.string,
+
   // The format of the font
   format: PropTypes.string,
 
   // The name of the font
   name: PropTypes.string,
 
   // URL for the font preview
   previewUrl: PropTypes.string,
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -23,16 +23,17 @@
 
 .font {
   border: 1px solid var(--theme-splitter-color);
   border-width: 0 1px 1px 0;
   display: grid;
   grid-template-columns: 1fr auto;
   grid-column-gap: 10px;
   padding: 10px 20px;
+  overflow: auto;
 }
 
 #font-container .theme-twisty {
   display: inline-block;
   cursor: pointer;
   vertical-align: bottom;
 }
 
@@ -71,23 +72,33 @@
   padding: 0;
 }
 
 .font-preview-input::-moz-selection {
   background: transparent;
   color: transparent;
 }
 
-.font-name {
-  margin: 0;
-  font-size: 1.2em;
+.font-name,
+.font-family-name {
   font-weight: normal;
   white-space: nowrap;
 }
 
+.font-name {
+  margin-bottom: 0.6em;
+  font-size: 1em;
+  color: var(--grey-50);
+}
+
+.font-family-name {
+  margin-bottom: 0.2em;
+  font-size: 1.2em;
+}
+
 .font-css-code {
   direction: ltr;
   margin: 0;
   overflow: hidden;
   text-overflow: ellipsis;
   color: var(--theme-toolbar-color);
   grid-column: span 2;
   position: relative;