Bug 1449885 - Write variation axis changes to rule and keep in sync with any manual edits to rule. r=gl

MozReview-Commit-ID: LaZWwWf2CsX

devtools/client/inspector/fonts/components/FontEditor.js

@@ -24,12 +24,12 @@ class FontEditor extends PureComponent {
    * Naive implementation to get increment step for variable font axis that ensures
    * a wide spectrum of precision based on range of values between min and max.
    *
-   * @param {Number|String} min
-   *        Minumum value for range.
-   * @param {Number|String} max
-   *        Maximum value for range.
+   * @param  {Number|String} min
+   *         Minumum value for range.
+   * @param  {Number|String} max
+   *         Maximum value for range.
    * @return {String}
-   *        Step value used in range input for font axis.
+   *         Step value used in range input for font axis.
    */
   getAxisStep(min, max) {
     let step = 1;
@@ -50,11 +50,11 @@ class FontEditor extends PureComponent {
    * Get an array of FontAxis components for of the given variable font axis instances.
    * If an axis is defined in the fontEditor store, use its value, else use the default.
    *
-   * @param {Array} fontAxes
-   *        Array of font axis instances
-   * @param {Object} editedAxes
-   *        Object with axes and values edited by the user or predefined in the CSS
-   *        declaration for font-variation-settings.
+   * @param  {Array} fontAxes
+   *         Array of font axis instances
+   * @param  {Object} editedAxes
+   *         Object with axes and values edited by the user or predefined in the CSS
+   *         declaration for font-variation-settings.
    * @return {Array}
   *          Array of FontAxis components
    */

devtools/client/inspector/fonts/fonts.js

@@ -10,6 +10,7 @@ const { gDevTools } = require("devtools/client/framework/devtools");
 const { getColor } = require("devtools/client/shared/theme");
 const { createFactory, createElement } = require("devtools/client/shared/vendor/react");
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
+const { throttle } = require("devtools/shared/throttle");
 
 const FontsApp = createFactory(require("./components/FontsApp"));
 
@@ -41,13 +42,15 @@ class FontInspector {
     this.selectedRule = null;
     this.store = this.inspector.store;
 
-    this.update = this.update.bind(this);
+    this.syncChanges = throttle(this.syncChanges, 100, this);
     this.onAxisUpdate = this.onAxisUpdate.bind(this);
     this.onNewNode = this.onNewNode.bind(this);
     this.onPreviewFonts = this.onPreviewFonts.bind(this);
     this.onRuleSelected = this.onRuleSelected.bind(this);
     this.onRuleUnselected = this.onRuleUnselected.bind(this);
+    this.onRuleUpdated = this.onRuleUpdated.bind(this);
     this.onThemeChanged = this.onThemeChanged.bind(this);
+    this.update = this.update.bind(this);
 
     this.init();
   }
@@ -156,17 +159,61 @@ class FontInspector {
            this.inspector.sidebar.getCurrentTabID() === "fontinspector";
   }
 
+  /**
+   * Live preview all CSS font property values from the fontEditor store on the page
+   * and sync the changes to the Rule view.
+   */
+  applyChanges() {
+    const fontEditor = this.store.getState().fontEditor;
+    // Until registered axis values are supported as font property values,
+    // write all axes and their values to font-variation-settings.
+    // Bug 1449891: https://bugzilla.mozilla.org/show_bug.cgi?id=1449891
+    const name = "font-variation-settings";
+    const value = Object.keys(fontEditor.axes)
+      .map(tag => `"${tag}" ${fontEditor.axes[tag]}`)
+      .join(", ");
+
+    let textProperty = this.selectedRule.textProps.filter(prop => prop.name === name)[0];
+    if (!textProperty) {
+      textProperty = this.selectedRule.editor.addProperty(name, value, "", true);
+    }
+
+    // Prevent reacting to changes we caused.
+    this.ruleView.off("property-value-updated", this.onRuleUpdated);
+    // Live preview font property changes on the page.
+    this.selectedRule.previewPropertyValue(textProperty, value, "");
+    // Sync Rule view with changes reflected on the page (throttled).
+    this.syncChanges(textProperty, value);
+  }
+
+  /**
+   * Sync the Rule view with the styles from the page. Called in a throttled way
+   * (see constructor) after property changes are applied directly to the CSS style rule
+   * on the page circumventing TextProperty.setValue() which triggers expensive DOM
+   * operations in TextPropertyEditor.update().
+   *
+   * @param  {TextProperty} textProperty
+   *         Model of CSS declaration for a property in used in the rule view.
+   * @param  {String} value
+   *         Value of the CSS property that should be reflected in the rule view.
+   */
+  syncChanges(textProperty, value) {
+    textProperty.updateValue(value);
+    this.ruleView.on("property-value-updated", this.onRuleUpdated);
+  }
+
   /**
    * Handler for changes of font axis value. Updates the value in the store and previews
    * the change on the page.
    *
-   * @param {String} tag
-   *        Tag name of the font axis.
-   * @param {String} value
-   *        Value of the font axis.
+   * @param  {String} tag
+   *         Tag name of the font axis.
+   * @param  {String} value
+   *         Value of the font axis.
    */
   onAxisUpdate(tag, value) {
     this.store.dispatch(updateAxis(tag, value));
+    this.applyChanges();
   }
 
   /**
@@ -192,10 +239,10 @@ class FontInspector {
    * If selected for the font editor, hold a reference to the rule so we know where to
    * put property changes coming from the font editor and show the font editor panel.
    *
-   * @param {Object} eventData
-   *        Data payload for the event. Contains:
-   *        - {String} editorId - id of the editor for which the rule was selected
-   *        - {Rule} rule - reference to rule that was selected
+   * @param  {Object} eventData
+   *         Data payload for the event. Contains:
+   *         - {String} editorId - id of the editor for which the rule was selected
+   *         - {Rule} rule - reference to rule that was selected
    */
   async onRuleSelected(eventData) {
     const { editorId, rule } = eventData;
@@ -205,9 +252,18 @@ class FontInspector {
 
       await this.refreshFontEditor();
       this.store.dispatch(toggleFontEditor(true, selector));
+      this.ruleView.on("property-value-updated", this.onRuleUpdated);
     }
   }
 
+  /**
+   * Handler for "property-value-updated" event emitted from the rule view whenever a
+   * property value changes.
+   */
+  async onRuleUpdated() {
+    await this.refreshFontEditor();
+  }
+
   /**
    * Handler for "ruleview-rule-unselected" event emitted from the rule view when a rule
    * was released from being selected for an editor.
@@ -225,6 +281,7 @@ class FontInspector {
       this.selectedRule = null;
       this.store.dispatch(toggleFontEditor(false));
       this.store.dispatch(resetFontEditor());
+      this.ruleView.off("property-value-updated", this.onRuleUpdated);
     }
   }
 

devtools/client/inspector/rules/models/rule.js

@@ -252,7 +252,7 @@ Rule.prototype = {
       // relevant properties here.
       for (let index in modifications.changedDeclarations) {
         let newValue = modifications.changedDeclarations[index];
-        this.textProps[index].noticeNewValue(newValue);
+        this.textProps[index].updateValue(newValue);
       }
       // Recompute and redisplay the computed properties.
       for (let prop of this.textProps) {

devtools/client/inspector/rules/models/text-property.js

@@ -128,9 +128,12 @@ TextProperty.prototype = {
 
   /**
    * Called when the property's value has been updated externally, and
-   * the property and editor should update.
+   * the property and editor should update to reflect that value.
+   *
+   * @param {String} value
+   *        Property value
    */
-  noticeNewValue: function(value) {
+  updateValue: function(value) {
     if (value !== this.value) {
       this.value = value;
       this.updateEditor();