Bug 1449885 - Write variation axis changes to rule and keep in sync with any manual edits to rule. r=gl draft
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 02 Apr 2018 17:40:21 +0200
changeset 779278 e8576d9fe0e5472897aaa6b999276840959d1d4c
parent 779277 ec5f0bde6a66952fa56a32f85e701526277b4ca4
push id105733
push userbmo:rcaliman@mozilla.com
push dateMon, 09 Apr 2018 16:11:10 +0000
reviewersgl
bugs1449885
milestone61.0a1
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/fonts.js
devtools/client/inspector/rules/models/rule.js
devtools/client/inspector/rules/models/text-property.js
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -8,16 +8,17 @@
 
 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 FontsApp = createFactory(require("./components/FontsApp"));
 
+const { throttle } = require("devtools/shared/throttle");
 const { LocalizationHelper } = require("devtools/shared/l10n");
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
 
 const { updateFonts } = require("./actions/fonts");
 const { updatePreviewText } = require("./actions/font-options");
 const { resetFontEditor, toggleFontEditor, updateAxis, updateFontEditor } =
   require("./actions/font-editor");
@@ -42,17 +43,19 @@ class FontInspector {
     this.store = this.inspector.store;
 
     this.update = this.update.bind(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.syncChanges = throttle(this.syncChanges, 100, this);
 
     this.init();
   }
 
   init() {
     if (!this.inspector) {
       return;
     }
@@ -152,26 +155,70 @@ class FontInspector {
    * Returns true if the font inspector panel is visible, and false otherwise.
    */
   isPanelVisible() {
     return this.inspector.sidebar &&
            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.
    */
   onAxisUpdate(tag, value) {
     this.store.dispatch(updateAxis(tag, value));
+    this.applyChanges();
   }
 
   /**
    * Selection 'new-node' event handler.
    */
   onNewNode() {
     if (this.isPanelVisible()) {
       this.update();
@@ -200,36 +247,46 @@ class FontInspector {
   async onRuleSelected(eventData) {
     const { editorId, rule } = eventData;
     if (editorId === FONT_EDITOR_ID) {
       const selector = rule.matchedSelectors[0];
       this.selectedRule = rule;
 
       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.
    * If previously selected for the font editor, release the reference to the rule and
    * hide the font editor panel.
    *
    * @param {Object} eventData
    *        Data payload for the event. Contains:
    *        - {String} editorId - id of the editor for which the rule was released
    *        - {Rule} rule - reference to rule that was released
    */
   onRuleUnselected(eventData) {
     const { editorId, rule } = eventData;
     if (editorId === FONT_EDITOR_ID && rule == this.selectedRule) {
       this.selectedRule = null;
       this.store.dispatch(toggleFontEditor(false));
       this.store.dispatch(resetFontEditor());
+      this.ruleView.off("property-value-updated", this.onRuleUpdated);
     }
   }
 
   /**
    * Handler for the "theme-switched" event.
    */
   onThemeChanged(frame) {
     if (frame === this.document.defaultView) {
--- a/devtools/client/inspector/rules/models/rule.js
+++ b/devtools/client/inspector/rules/models/rule.js
@@ -247,17 +247,17 @@ Rule.prototype = {
    */
   _applyPropertiesAuthored: function(modifications) {
     return modifications.apply().then(() => {
       // The rewriting may have required some other property values to
       // change, e.g., to insert some needed terminators.  Update the
       // 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) {
         if (!prop.invisible && prop.enabled) {
           prop.updateComputed();
           prop.updateEditor();
         }
       }
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -123,19 +123,22 @@ TextProperty.prototype = {
     }
 
     this.rule.setPropertyValue(this, value, priority);
     this.updateEditor();
   },
 
   /**
    * 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();
     }
   },
 
   setName: function(name) {
     let store = this.rule.elementStyle.store;