Bug 1486891 - (Part 2) Remove old font preview functionality from font editor. r=gl
☠☠ backed out by 4804e9ef844f ☠ ☠
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 29 Aug 2018 10:37:59 +0000
changeset 482279 cad015aaf131b90276537d91e2acf9379f4433d4
parent 482278 bce9cb28665c0bb100267cbb956290a00c447721
child 482280 1586964885f9449cfd189c41a684ebbaee433518
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersgl
bugs1486891
milestone63.0a1
Bug 1486891 - (Part 2) Remove old font preview functionality from font editor. r=gl Differential Revision: https://phabricator.services.mozilla.com/D4475
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/components/FontList.js
devtools/client/inspector/fonts/components/FontPreview.js
devtools/client/inspector/fonts/test/head.js
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
@@ -13,18 +13,17 @@ const FontOrigin = createFactory(require
 const FontPreview = createFactory(require("./FontPreview"));
 
 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,
+      onPreviewClick: PropTypes.func,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.state = {
@@ -103,23 +102,20 @@ class Font extends PureComponent {
     }
 
     return dom.div({ className: "font-family-name" }, family);
   }
 
   render() {
     const {
       font,
-      fontOptions,
-      onPreviewFonts,
+      onPreviewClick,
       onToggleFontHighlight,
     } = this.props;
 
-    const { previewText } = fontOptions;
-
     const {
       CSSFamilyName,
       previewUrl,
       rule,
       ruleText,
     } = font;
 
     return dom.li(
@@ -127,15 +123,15 @@ class Font extends PureComponent {
         className: "font",
       },
       dom.div(
         {},
         this.renderFontFamilyName(CSSFamilyName),
         FontName({ font, onToggleFontHighlight })
       ),
       FontOrigin({ font }),
-      FontPreview({ previewText, previewUrl, onPreviewFonts }),
+      FontPreview({ onPreviewClick, previewUrl }),
       this.renderFontCSSCode(rule, ruleText)
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/components/FontList.js
+++ b/devtools/client/inspector/fonts/components/FontList.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const Services = require("Services");
 const {
   createElement,
   createFactory,
+  createRef,
   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");
 
 const Font = createFactory(require("./Font"));
 const FontPreviewInput = createFactory(require("./FontPreviewInput"));
@@ -26,41 +27,58 @@ class FontList extends PureComponent {
     return {
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       fonts: PropTypes.arrayOf(PropTypes.shape(Types.font)).isRequired,
       onPreviewTextChange: PropTypes.func.isRequired,
       onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
+  constructor(props) {
+    super(props);
+
+    this.onPreviewClick = this.onPreviewClick.bind(this);
+    this.previewInputRef = createRef();
+  }
+
+  /**
+   * Handler for clicks on the font preview image.
+   * Requests the FontPreviewInput component, if one exists, to focus its input field.
+   */
+  onPreviewClick() {
+    this.previewInputRef.current && this.previewInputRef.current.focus();
+  }
+
   render() {
     const {
       fonts,
       fontOptions,
       onPreviewTextChange,
       onToggleFontHighlight
     } = this.props;
 
     const { previewText } = fontOptions;
+    const { onPreviewClick } = this;
 
     const list = dom.ul(
       {
         className: "fonts-list"
       },
       fonts.map((font, i) => Font({
         key: i,
         font,
-        fontOptions,
+        onPreviewClick,
         onToggleFontHighlight,
       }))
     );
 
     // Show the font preview input only when the font editor is enabled.
     const previewInput = Services.prefs.getBoolPref(PREF_FONT_EDITOR) ?
       FontPreviewInput({
+        ref: this.previewInputRef,
         onPreviewTextChange,
         previewText,
       })
       :
       null;
 
     return createElement(Fragment, null, previewInput, list);
   }
--- a/devtools/client/inspector/fonts/components/FontPreview.js
+++ b/devtools/client/inspector/fonts/components/FontPreview.js
@@ -4,93 +4,40 @@
 
 "use strict";
 
 const { 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 Types = require("../types");
-const { getStr } = require("../utils/l10n");
 
 class FontPreview extends PureComponent {
   static get propTypes() {
     return {
-      previewText: Types.fontOptions.previewText.isRequired,
+      onPreviewClick: PropTypes.func,
       previewUrl: Types.font.previewUrl.isRequired,
-      onPreviewFonts: PropTypes.func.isRequired,
     };
   }
 
-  constructor(props) {
-    super(props);
-
-    this.state = {
-      // Is the text preview input field currently focused?
-      isFocused: false,
+  static get defaultProps() {
+    return {
+      onPreviewClick: () => {},
     };
-
-    this.onBlur = this.onBlur.bind(this);
-    this.onClick = this.onClick.bind(this);
-    this.onChange = this.onChange.bind(this);
-  }
-
-  componentDidUpdate() {
-    if (this.state.isFocused) {
-      const input = this.fontPreviewInput;
-      input.focus();
-      input.selectionStart = input.selectionEnd = input.value.length;
-    }
-  }
-
-  onBlur() {
-    this.setState({ isFocused: false });
-  }
-
-  onClick(event) {
-    this.setState({ isFocused: true });
-    event.stopPropagation();
-  }
-
-  onChange(event) {
-    this.props.onPreviewFonts(event.target.value);
   }
 
   render() {
     const {
-      previewText,
+      onPreviewClick,
       previewUrl,
     } = this.props;
 
-    const { isFocused } = this.state;
-
-    return dom.div(
+    return dom.img(
       {
-        className: "font-preview-container",
-      },
-      isFocused ?
-        dom.input(
-          {
-            type: "search",
-            className: "font-preview-input devtools-searchinput",
-            value: previewText,
-            onBlur: this.onBlur,
-            onChange: this.onChange,
-            ref: input => {
-              this.fontPreviewInput = input;
-            }
-          }
-        )
-        :
-        null,
-      dom.img(
-        {
-          className: "font-preview",
-          src: previewUrl,
-          onClick: this.onClick,
-          title: !isFocused ? getStr("fontinspector.editPreview") : "",
-        }
-      )
+        className: "font-preview",
+        onClick: onPreviewClick,
+        src: previewUrl,
+      }
     );
   }
 }
 
 module.exports = FontPreview;
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -56,51 +56,44 @@ var openFontInspectorForURL = async func
     testActor,
     toolbox,
     inspector,
     view: inspector.fontinspector
   };
 };
 
 /**
- * Focus one of the preview inputs, clear it, type new text into it and wait for the
+ * Focus the preview input, clear it, type new text into it and wait for the
  * preview images to be updated.
  *
  * @param {FontInspector} view - The FontInspector instance.
  * @param {String} text - The text to preview.
  */
 async function updatePreviewText(view, text) {
   info(`Changing the preview text to '${text}'`);
 
   const doc = view.document;
-  const previewImg = doc.querySelector("#sidebar-panel-fontinspector .font-preview");
-
-  info("Clicking the font preview element to turn it to edit mode");
-  const onClick = once(doc, "click");
-  previewImg.click();
-  await onClick;
-
-  const input = previewImg.parentNode.querySelector("input");
-  is(doc.activeElement, input, "The input was focused.");
+  const input = doc.querySelector("#font-preview-input-container input");
+  input.focus();
 
   info("Blanking the input field.");
   while (input.value.length) {
     const update = view.inspector.once("fontinspector-updated");
     EventUtils.sendKey("BACK_SPACE", doc.defaultView);
     await update;
   }
 
   if (text) {
-    info("Typing the specified text to the input field.");
-    const update = waitForNEvents(view.inspector, "fontinspector-updated", text.length);
+    info(`Typing "${text}" into the input field.`);
+    const update = view.inspector.once("fontinspector-updated");
     EventUtils.sendString(text, doc.defaultView);
     await update;
   }
 
-  is(input.value, text, "The input now contains the correct text.");
+  is(input.value, text, `The input now contains "${text}".`);
 }
 
 /**
 *  Get all of the <li> elements for the fonts used on the currently selected element.
 *
 *  NOTE: This method is used by tests which check the old Font Inspector. It, along with
 *  the tests should be removed once the Font Editor reaches Firefox Stable.
 *  @see https://bugzilla.mozilla.org/show_bug.cgi?id=1485324
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -12,21 +12,16 @@ fontinspector.system=system
 # LOCALIZATION NOTE (fontinspector.noFontsOnSelectedElement): This label is shown when
 # no fonts found on the selected element.
 fontinspector.noFontsOnSelectedElement=No fonts were found for the current element.
 
 # LOCALIZATION NOTE (fontinspector.otherFontsInPageHeader): This is the text for the
 # header of a collapsible section containing other fonts used in the page.
 fontinspector.otherFontsInPageHeader=Other fonts in page
 
-# LOCALIZATION NOTE (fontinspector.editPreview): This is the text that appears in a
-# tooltip on hover of a font preview string. Clicking on the string opens a text input
-# where users can type to change the preview text.
-fontinspector.editPreview=Click to edit preview
-
 # LOCALIZATION NOTE (fontinspector.copyURL): This is the text that appears in a tooltip
 # displayed when the user hovers over the copy icon next to the font URL.
 # Clicking the copy icon copies the full font URL to the user's clipboard
 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
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -79,65 +79,35 @@
 }
 
 #font-container .theme-twisty {
   display: inline-block;
   cursor: pointer;
   vertical-align: bottom;
 }
 
-.font-preview-container {
-  grid-column: 2;
-  grid-row: 1 / span 2;
-  overflow: hidden;
-  display: grid;
-  place-items: center end;
-  position: relative;
-}
-
-.font-preview {
-  height: 50px;
-  display: block;
-}
-
-.font-preview:hover {
-  cursor: text;
-  background-image: linear-gradient(to right,
-    var(--grey-40) 3px, transparent 3px, transparent);
-  background-size: 6px 1px;
-  background-repeat: repeat-x;
-  background-position-y: 45px;
-}
 #font-preview-input-container {
   background: var(--preview-input-background);
   border-bottom: 1px solid var(--theme-splitter-color);
   display: flex;
   height: 23px;
 }
 
-#font-container .font-preview-input {
-  position: absolute;
-  top: 5px;
-  left: 0;
-  width: calc(100% - 5px);
-  height: calc(100% - 10px);
-  background: transparent;
-  color: transparent;
-  border-radius: 0;
-  padding: 0;
-}
 #font-preview-input-container input {
   background-image: none;
   flex: 1;
   padding-left: 14px;
 }
 
-.font-preview-input::-moz-selection {
-  background: transparent;
-  color: transparent;
+.font-preview {
+  grid-column: 2;
+  grid-row: 1 / span 2;
+  object-fit: contain;
+  height: 50px;
+  width: 100%;
 }
 
 .font-name,
 .font-family-name {
   font-weight: normal;
   white-space: nowrap;
 }