Bug 1435373 - Simplify implmentation for mapping changes to Rule view. r=pbro draft
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 04 Apr 2018 21:57:02 +0200
changeset 777488 a47d192dd36893413c45ebb95e5afc43fd1c403e
parent 777487 7a44d4cd6f9ff842a41973f9479db3cc128676c4
push id105224
push userbmo:rcaliman@mozilla.com
push dateWed, 04 Apr 2018 21:01:42 +0000
reviewerspbro
bugs1435373
milestone61.0a1
Bug 1435373 - Simplify implmentation for mapping changes to Rule view. r=pbro MozReview-Commit-ID: GUWrVi891Is
devtools/client/inspector/rules/rules.js
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/inspector/shared/highlighters-overlay.js
devtools/client/inspector/shared/node-types.js
devtools/client/shared/widgets/ShapesInContextEditor.js
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -19,16 +19,17 @@ const ClassListPreviewer = require("devt
 const {getCssProperties} = require("devtools/shared/fronts/css-properties");
 const {
   VIEW_NODE_SELECTOR_TYPE,
   VIEW_NODE_PROPERTY_TYPE,
   VIEW_NODE_VALUE_TYPE,
   VIEW_NODE_IMAGE_URL_TYPE,
   VIEW_NODE_LOCATION_TYPE,
   VIEW_NODE_SHAPE_POINT_TYPE,
+  VIEW_NODE_SHAPE_SWATCH,
   VIEW_NODE_VARIABLE_TYPE,
   VIEW_NODE_FONT_TYPE,
 } = require("devtools/client/inspector/shared/node-types");
 const StyleInspectorMenu = require("devtools/client/inspector/shared/style-inspector-menu");
 const TooltipsOverlay = require("devtools/client/inspector/shared/tooltips-overlay");
 const {createChild, promiseWarn} = require("devtools/client/inspector/shared/utils");
 const {debounce} = require("devtools/shared/debounce");
 const EventEmitter = require("devtools/shared/event-emitter");
@@ -356,16 +357,23 @@ CssRuleView.prototype = {
         enabled: prop.enabled,
         overridden: prop.overridden,
         pseudoElement: prop.rule.pseudoElement,
         sheetHref: prop.rule.domRule.href,
         textProperty: prop,
         toggleActive: getShapeToggleActive(node),
         point: getShapePoint(node)
       };
+    } else if (classes.contains("ruleview-shapeswatch") && prop) {
+      type = VIEW_NODE_SHAPE_SWATCH;
+      value = {
+        enabled: prop.enabled,
+        overridden: prop.overridden,
+        textProperty: prop,
+      };
     } else if ((classes.contains("ruleview-variable") ||
                 classes.contains("ruleview-unmatched-variable")) && prop) {
       type = VIEW_NODE_VARIABLE_TYPE;
       value = {
         property: getPropertyNameAndValue(node).name,
         value: node.textContent,
         enabled: prop.enabled,
         overridden: prop.overridden,
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -88,18 +88,16 @@ function TextPropertyEditor(ruleEditor, 
   this._onExpandClicked = this._onExpandClicked.bind(this);
   this._onStartEditing = this._onStartEditing.bind(this);
   this._onNameDone = this._onNameDone.bind(this);
   this._onValueDone = this._onValueDone.bind(this);
   this._onSwatchCommit = this._onSwatchCommit.bind(this);
   this._onSwatchPreview = this._onSwatchPreview.bind(this);
   this._onSwatchRevert = this._onSwatchRevert.bind(this);
   this._onValidate = this.ruleView.debounce(this._previewValue, 10, this);
-  this.onInContextEditorCommit = this.onInContextEditorCommit.bind(this);
-  this.onInContextEditorPreview = this.onInContextEditorPreview.bind(this);
   this.update = this.update.bind(this);
   this.updatePropertyState = this.updatePropertyState.bind(this);
 
   this._create();
   this.update();
 }
 
 TextPropertyEditor.prototype = {
@@ -334,17 +332,17 @@ TextPropertyEditor.prototype = {
       return domRule.href || domRule.nodeHref;
     }
     return undefined;
   },
 
   /**
    * Populate the span based on changes to the TextProperty.
    */
-  update: async function() {
+  update: function() {
     if (this.ruleView.isDestroyed) {
       return;
     }
 
     this.updatePropertyState();
 
     let name = this.prop.name;
     this.nameSpan.textContent = name;
@@ -508,34 +506,16 @@ TextPropertyEditor.prototype = {
     }
 
     let shapeToggle = this.valueSpan.querySelector(".ruleview-shapeswatch");
     if (shapeToggle) {
       let mode = "css" + name.split("-").map(s => {
         return s[0].toUpperCase() + s.slice(1);
       }).join("");
       shapeToggle.setAttribute("data-mode", mode);
-
-      try {
-        let shapesEditor =
-          await this.ruleView.highlighters.getInContextEditor("shapesEditor");
-        if (!shapesEditor) {
-          return;
-        }
-
-        shapesEditor.link(this.prop, shapeToggle, {
-          onPreview: this.onInContextEditorPreview,
-          onCommit: this.onInContextEditorCommit
-        });
-        // Mark this toggle if this property is being currently edited; unmark otherwise.
-        shapeToggle.classList.toggle("active", shapesEditor.activeProperty === this.prop);
-      } catch (err) {
-        // Remove toggle and, with it, any expectations of triggering an editor.
-        shapeToggle.remove();
-      }
     }
 
     // Now that we have updated the property's value, we might have a pending
     // click on the value container. If we do, we have to trigger a click event
     // on the right element.
     if (this._hasPendingClick) {
       this._hasPendingClick = false;
       let elToClick;
@@ -1037,31 +1017,11 @@ TextPropertyEditor.prototype = {
    * Returns true if the property is a `display: [inline-]grid` declaration.
    *
    * @return {Boolean} true if the property is a `display: [inline-]grid` declaration.
    */
   isDisplayGrid: function() {
     return this.prop.name === "display" &&
       (this.prop.value === "grid" || this.prop.value === "inline-grid");
   },
-
-  /**
-   * Live preview this property, without committing changes.
-   *
-   * @param {String} value
-   *        The value to set the current property to.
-   */
-  onInContextEditorPreview(value) {
-    this.ruleEditor.rule.previewPropertyValue(this.prop, value);
-  },
-
-  /**
-   * Commit this property value. Triggers editor update.
-   *
-   * @param {String} value
-   *        The value to set the current property to.
-   */
-  onInContextEditorCommit(value) {
-    this.prop.setValue(value);
-  }
 };
 
 module.exports = TextPropertyEditor;
--- a/devtools/client/inspector/shared/highlighters-overlay.js
+++ b/devtools/client/inspector/shared/highlighters-overlay.js
@@ -125,20 +125,36 @@ class HighlightersOverlay {
 
     let el = view.element;
     el.removeEventListener("click", this.onClick, true);
     el.removeEventListener("mousemove", this.onMouseMove);
     el.removeEventListener("mouseout", this.onMouseOut);
   }
 
   /**
-   * Show the shapes highlighter for the given element with a shape.
-   * This method delegates to the in-context shapes editor which wraps the
-   * shapes highlighter with additional functionality required for passing changes
-   * back to the rule view.
+   * Toggle the shapes highlighter for the given node.
+
+   * @param  {NodeFront} node
+   *         The NodeFront of the element with a shape to highlight.
+   * @param  {Object} options
+   *         Object used for passing options to the shapes highlighter.
+   * @param {TextProperty} textProperty
+   *        TextProperty where to write changes.
+   */
+  async toggleShapesHighlighter(node, options, textProperty) {
+    let shapesEditor = await this.getInContextEditor("shapesEditor");
+    if (!shapesEditor) {
+      return;
+    }
+    shapesEditor.toggle(node, options, textProperty);
+  }
+
+  /**
+   * Show the shapes highlighter for the given node.
+   * This method delegates to the in-context shapes editor.
    *
    * @param  {NodeFront} node
    *         The NodeFront of the element with a shape to highlight.
    * @param  {Object} options
    *         Object used for passing options to the shapes highlighter.
    */
   async showShapesHighlighter(node, options) {
     let shapesEditor = await this.getInContextEditor("shapesEditor");
@@ -687,17 +703,17 @@ class HighlightersOverlay {
 
   /**
    * Does the current clicked node have the shapes highlighter toggle in the
    * rule-view.
    *
    * @param  {DOMNode} node
    * @return {Boolean}
    */
-  _isRuleViewShape(node) {
+  _isRuleViewShapeSwatch(node) {
     return this.isRuleView(node) && node.classList.contains("ruleview-shapeswatch");
   }
 
   /**
    * Is the current hovered node a css transform property value in the rule-view.
    *
    * @param  {Object} nodeInfo
    * @return {Boolean}
@@ -741,21 +757,35 @@ class HighlightersOverlay {
       let { store } = this.inspector;
       let { grids, highlighterSettings } = store.getState();
       let grid = grids.find(g => g.nodeFront == this.inspector.selection.nodeFront);
 
       highlighterSettings.color = grid ? grid.color : DEFAULT_GRID_COLOR;
 
       this.toggleGridHighlighter(this.inspector.selection.nodeFront, highlighterSettings,
         "rule");
-    } else if (this._isRuleViewDisplayFlex(event.target)) {
+    }
+
+    if (this._isRuleViewDisplayFlex(event.target)) {
       event.stopPropagation();
 
       this.toggleFlexboxHighlighter(this.inspector.selection.nodeFront);
     }
+
+    if (this._isRuleViewShapeSwatch(event.target)) {
+      event.stopPropagation();
+
+      const view = this.inspector.getPanel("ruleview").view;
+      const nodeInfo = view.getNodeInfo(event.target);
+
+      this.toggleShapesHighlighter(this.inspector.selection.nodeFront, {
+        mode: event.target.dataset.mode,
+        transformMode: event.metaKey || event.ctrlKey
+      }, nodeInfo.value.textProperty);
+    }
   }
 
   onMouseMove(event) {
     // Bail out if the target is the same as for the last mousemove.
     if (event.target === this._lastHovered) {
       return;
     }
 
--- a/devtools/client/inspector/shared/node-types.js
+++ b/devtools/client/inspector/shared/node-types.js
@@ -13,8 +13,9 @@
 exports.VIEW_NODE_SELECTOR_TYPE = 1;
 exports.VIEW_NODE_PROPERTY_TYPE = 2;
 exports.VIEW_NODE_VALUE_TYPE = 3;
 exports.VIEW_NODE_IMAGE_URL_TYPE = 4;
 exports.VIEW_NODE_LOCATION_TYPE = 5;
 exports.VIEW_NODE_SHAPE_POINT_TYPE = 6;
 exports.VIEW_NODE_VARIABLE_TYPE = 7;
 exports.VIEW_NODE_FONT_TYPE = 8;
+exports.VIEW_NODE_SHAPE_SWATCH = 9;
--- a/devtools/client/shared/widgets/ShapesInContextEditor.js
+++ b/devtools/client/shared/widgets/ShapesInContextEditor.js
@@ -5,247 +5,98 @@
 "use strict";
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const { debounce } = require("devtools/shared/debounce");
 
 /**
  * The ShapesInContextEditor:
  * - communicates with the ShapesHighlighter actor from the server;
- * - triggers the shapes highlighter on click on a swatch element (aka toggle icon)
- *   in the Rule view next to CSS properties like `shape-outside` and `clip-path`;
  * - listens to events for shape change and hover point coming from the shape-highlighter;
  * - writes shape value changes to the CSS declaration it was triggered from;
- * - synchroninses highlighting coordinate points on mouse over between the shapes
+ * - synchronises highlighting coordinate points on mouse over between the shapes
  *   highlighter and the shape value shown in the Rule view.
  *
- * It behaves like a singleton, though it is not yet implemented like that.
  * It is instantiated once in HighlightersOverlay by calls to .getInContextEditor().
- * It is requested by TextPropertyEditor instances for `clip-path` and `shape-outside`
- * CSS properties to register swatches that should trigger the shapes highlighter and to
- * which shape values should be written back.
  */
 class ShapesInContextEditor {
   constructor(highlighter, inspector, state) {
     EventEmitter.decorate(this);
 
-    this.activeSwatch = null;
-    this.activeProperty = null;
     this.inspector = inspector;
     this.highlighter = highlighter;
     // Refence to the NodeFront currently being highlighted.
     this.highlighterTargetNode = null;
     this.highligherEventHandlers = {};
     this.highligherEventHandlers["shape-change"] = this.onShapeChange;
     this.highligherEventHandlers["shape-hover-on"] = this.onShapeHover;
     this.highligherEventHandlers["shape-hover-off"] = this.onShapeHover;
-    // Map with data required to preview and persist shape value changes to the Rule view.
-    // keys - TextProperty instances relevant for shape editor (clip-path, shape-outside).
-    // values - objects with references to swatch elements that trigger the shape editor
-    //          and callbacks used to preview and persist shape value changes.
-    this.links = new Map();
+    // Mode for shapes highlighter: shape-outside or clip-path. Used to discern
+    // when toggling the highlighter on the same node for different CSS properties.
+    this.mode = null;
     // Reference to Rule view used to listen for changes
     this.ruleView = this.inspector.getPanel("ruleview").view;
     // Reference of |state| from HighlightersOverlay.
     this.state = state;
+    // Reference to DOM node of the toggle icon for shapes highlighter.
+    this.swatch = null;
+    // Reference to TextProperty where shape changes will be written.
+    this.textProperty = null;
 
     // Commit triggers expensive DOM changes in TextPropertyEditor.update()
     // so we debounce it.
     this.commit = debounce(this.commit, 200, this);
     this.onChangesApplied = this.onChangesApplied.bind(this);
     this.onHighlighterEvent = this.onHighlighterEvent.bind(this);
     this.onNodeFrontChanged = this.onNodeFrontChanged.bind(this);
     this.onRuleViewChanged = this.onRuleViewChanged.bind(this);
-    this.onSwatchClick = this.onSwatchClick.bind(this);
 
     this.highlighter.on("highlighter-event", this.onHighlighterEvent);
     this.ruleView.on("ruleview-changed", this.onRuleViewChanged);
   }
 
   /**
-  * The shapes in-context editor works by listening to shape value changes from the shapes
-  * highlighter and mapping them to the correct CSS property in the Rule view.
-  *
-  * In order to know where to map changes, the TextPropertyEditor instances register
-  * themselves in a map object internal to the ShapesInContextEditor.
-  * Calling the `ShapesInContextEditor.link()` method, they provide:
-  * - the TextProperty model to which shape value changes should map to (this is the key
-  * in the internal map object);
-  * - the swatch element that triggers the shapes highlighter,
-  * - callbacks that must be used in order to preview and persist the shape value changes.
-  *
-  * When the TextPropertyEditor updates, it rebuilds part of its DOM and destroys the
-  * original swatch element. Losing that reference to the swatch element breaks the
-  * ability to show the indicator for the shape editor that is on and prevents the preview
-  * functionality from working properly. Because of that, this link() method gets called
-  * on every TextPropertyEditor.update() and, if the TextProperty model used as a key is
-  * already registered, the old swatch element reference is replaced with the new one.
-  *
-  * @param {TextProperty} prop
-  *        TextProperty instance for clip-path or shape-outside from Rule view for the
-  *        selected element.
-  * @param {Node} swatch
-  *        Reference to DOM element next to shape value that toggles the shapes
-  *        highlighter. This element is destroyed after each call to |this.commit()|
-  *        because that rebuilds the DOM for the shape value in the Rule view.
-  *        Repeated calls to this method with the same prop (TextProperty) will
-  *        replace the swatch reference to the new element for consistent behaviour.
-  * @param {Object} callbacks
-  *        Collection of callbacks from the TextPropertyEditor:
-  *        - onPreview: method called to preview a shape value on the element
-  *        - onCommit: method called to commit a shape value to the Rule view.
-  */
-  link(prop, swatch, callbacks = {}) {
-    if (this.links.has(prop)) {
-      // Swatch element may have changed, replace with given reference.
-      this.replaceSwatch(prop, swatch);
-      return;
-    }
-    if (!callbacks.onPreview) {
-      callbacks.onPreview = function() {};
-    }
-    if (!callbacks.onCommit) {
-      callbacks.onCommit = function() {};
-    }
-
-    swatch.addEventListener("click", this.onSwatchClick);
-    this.links.set(prop, { swatch, callbacks });
-
-    // Event on HighlightersOverlay expected by tests to know when to click the swatch.
-    this.inspector.highlighters.emit("shapes-highlighter-armed");
-  }
-
-  /**
-  * Remove references to swatch and callbacks for the given TextProperty model so that
-  * shape value changes cannot map back to it and the shape editor cannot be triggered
-  * from its associated swatch element.
-  *
-  * @param {TextProperty} prop
-  *        TextProperty instance from Rule view.
-  */
-  async unlink(prop) {
-    let data = this.links.get(prop);
-    if (!data || !data.swatch) {
-      return;
-    }
-    if (this.activeProperty === prop) {
-      await this.hide();
-    }
-
-    data.swatch.classList.remove("active");
-    data.swatch.removeEventListener("click", this.onSwatchClick);
-    this.links.delete(prop);
-  }
-
-  /**
-  * Remove all linked references from TextPropertyEditor.
-  */
-  async unlinkAll() {
-    for (let [prop] of this.links) {
-      await this.unlink(prop);
-    }
-  }
-
-  /**
-  * If the given TextProperty exists as a key in |this.links|, replace the reference it
-  * has to the swatch element with a new node reference.
-  *
-  * @param {TextProperty} prop
-  *        TextProperty instance from Rule view.
-  * @param {Node} swatch
-  *        Reference to swatch DOM element.
-  */
-  replaceSwatch(prop, swatch) {
-    let data = this.links.get(prop);
-    if (data.swatch) {
-      // Cleanup old
-      data.swatch.removeEventListener("click", this.onSwatchClick);
-      data.swatch = undefined;
-      // Setup new
-      swatch.addEventListener("click", this.onSwatchClick);
-      data.swatch = swatch;
-    }
-  }
-
-  /**
-  * Check if the given swatch DOM element already exists in the collection of linked
-  * swatches.
-  *
-  * @param {Node} swatch
-  *        Reference to swatch DOM element.
-  * @return {Boolean}
-  *
-  */
-  hasSwatch(swatch) {
-    for (let [, data] of this.links) {
-      if (data.swatch == swatch) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  /**
   * Called when the element style changes from the Rule view.
   * If the TextProperty we're acting on isn't enabled anymore or overridden,
   * turn off the shapes highlighter.
   */
   async onRuleViewChanged() {
-    if (this.activeProperty &&
-      (!this.activeProperty.enabled || this.activeProperty.overridden)) {
+    if (this.textProperty &&
+      (!this.textProperty.enabled || this.textProperty.overridden)) {
       await this.hide();
     }
   }
 
   /**
-  * Called when a swatch element is clicked. Toggles shapes highlighter to show or hide.
-  * Sets the current swatch and corresponding TextProperty as the active ones. They will
-  * be immediately unset if the toggle action is to hide the shapes highlighter.
-  *
-  * @param {MouseEvent} event
-  *        Mouse click event.
-  */
-  onSwatchClick(event) {
-    event.stopPropagation();
-    for (let [prop, data] of this.links) {
-      if (data.swatch == event.target) {
-        this.activeSwatch = data.swatch;
-        this.activeSwatch.classList.add("active");
-        this.activeProperty = prop;
-        break;
-      }
-    }
-
-    let nodeFront = this.inspector.selection.nodeFront;
-    let options =  {
-      mode: event.target.dataset.mode,
-      transformMode: event.metaKey || event.ctrlKey
-    };
-
-    this.toggle(nodeFront, options);
-  }
-
-  /**
    * Toggle the shapes highlighter for the given element.
    *
    * @param {NodeFront} node
    *        The NodeFront of the element with a shape to highlight.
    * @param {Object} options
    *        Object used for passing options to the shapes highlighter.
    */
-  async toggle(node, options) {
-    if (node == this.highlighterTargetNode) {
+  async toggle(node, options, prop) {
+    // Same target node, same mode -> hide and exit OR switch to toggle transform mode.
+    if ((node == this.highlighterTargetNode) && (this.mode === options.mode)) {
       if (!options.transformMode) {
         await this.hide();
         return;
       }
 
       options.transformMode = !this.state.shapes.options.transformMode;
     }
 
+    // Same target node, dfferent modes -> toggle between shape-outside and clip-path.
+    // Hide highlighter for previous property, but continue and show for other property.
+    if ((node == this.highlighterTargetNode) && (this.mode !== options.mode)) {
+      await this.hide();
+    }
+
+    this.textProperty = prop;
+    this.findSwatch();
     await this.show(node, options);
   }
 
   /**
    * Show the shapes highlighter for the given element.
    *
    * @param {NodeFront} node
    *        The NodeFront of the element with a shape to highlight.
@@ -256,38 +107,51 @@ class ShapesInContextEditor {
     let isShown = await this.highlighter.show(node, options);
     if (!isShown) {
       return;
     }
 
     this.inspector.selection.on("detached-front", this.onNodeFrontChanged);
     this.inspector.selection.on("new-node-front", this.onNodeFrontChanged);
     this.highlighterTargetNode = node;
+    this.mode = options.mode;
     this.emit("show", { node, options });
   }
 
   /**
    * Hide the shapes highlighter.
    */
   async hide() {
     await this.highlighter.hide();
 
-    if (this.activeSwatch) {
-      this.activeSwatch.classList.remove("active");
+    if (this.swatch) {
+      this.swatch.classList.remove("active");
     }
-    this.activeSwatch = null;
-    this.activeProperty = null;
+    this.swatch = null;
+    this.textProperty = null;
 
     this.emit("hide", { node: this.highlighterTargetNode });
     this.inspector.selection.off("detached-front", this.onNodeFrontChanged);
     this.inspector.selection.off("new-node-front", this.onNodeFrontChanged);
     this.highlighterTargetNode = null;
   }
 
   /**
+   * Identify the swatch (aka toggle icon) DOM node from the TextPropertyEditor of the
+   * TextProperty we're working with. Whenever the TextPropertyEditor is updated (i.e.
+   * when committing the shape value to the Rule view), it rebuilds its DOM and the old
+   * swatch reference becomes invalid. Call this method to identify the current swatch.
+   */
+  findSwatch() {
+    const valueSpan = this.textProperty.editor.valueSpan;
+    this.swatch = valueSpan.querySelector(".ruleview-shapeswatch");
+    this.swatch.classList.add("active");
+  }
+
+  /**
    * Handle events emitted by the highlighter.
    * Find any callback assigned to the event type and call it with the given data object.
    *
    * @param {Object} data
    *        The data object sent in the event.
    */
   onHighlighterEvent(data) {
     const handler = this.highligherEventHandlers[data.type];
@@ -300,53 +164,53 @@ class ShapesInContextEditor {
 
   /**
   * Clean up when node selection changes because Rule view and TextPropertyEditor
   * instances are not automatically destroyed when selection changes.
   */
   async onNodeFrontChanged() {
     try {
       await this.hide();
-      await this.unlinkAll();
     } catch (err) {
       // Silent error.
     }
   }
 
   /**
-  * Called when there's an updated shape value from the highlighter.
+  * Handler for "shape-change" event from the shapes highlighter.
   *
   * @param  {Object} data
   *         Data associated with the "shape-change" event.
   *         Contains:
   *         - {String} value: the new shape value.
   *         - {String} type: the event type ("shape-change").
   */
   onShapeChange(data) {
     this.preview(data.value);
     this.commit(data.value);
   }
 
   /**
-  * Called when the mouse moves on or off of a coordinate point inside the shapes
-  * highlighter and marks/unmarks the corresponding coordinate node in the shape value
+  * Handler for "shape-hover-on" and "shape-hover-off" events from the shapes highlighter.
+  * Called when the mouse moves over or off of a coordinate point inside the shapes
+  * highlighter. Marks/unmarks the corresponding coordinate node in the shape value
   * from the Rule view.
   *
   * @param  {Object} data
   *         Data associated with the "shape-hover" event.
   *         Contains:
   *         - {String|null} point: coordinate to highlight or null if nothing to highlight
   *         - {String} type: the event type ("shape-hover-on" or "shape-hover-on").
   */
   onShapeHover(data) {
-    if (!this.activeProperty) {
+    if (!this.textProperty) {
       return;
     }
 
-    let shapeValueEl = this.links.get(this.activeProperty).swatch.nextSibling;
+    let shapeValueEl = this.swatch.nextSibling;
     if (!shapeValueEl) {
       return;
     }
     let pointSelector = ".ruleview-shape-point";
     // First, unmark all highlighted coordinate nodes from Rule view
     for (let node of shapeValueEl.querySelectorAll(`${pointSelector}.active`)) {
       node.classList.remove("active");
     }
@@ -370,60 +234,62 @@ class ShapesInContextEditor {
                   `${pointSelector}[data-point='${point}']`;
 
     for (let node of shapeValueEl.querySelectorAll(selector)) {
       node.classList.add("active");
     }
   }
 
   /**
-  * Preview this shape value on the element but do commit the changes to the Rule view.
+  * Preview a shape value on the element without committing the changes to the Rule view.
   *
   * @param {String} value
   *        The shape value to set the current property to
   */
   preview(value) {
-    if (!this.activeProperty) {
+    if (!this.textProperty) {
       return;
     }
-    let data = this.links.get(this.activeProperty);
-    // Update the element's styles to see live results.
-    data.callbacks.onPreview(value);
+    // Update the element's style to see live results.
+    this.textProperty.rule.previewPropertyValue(this.textProperty, value);
     // Update the text of CSS value in the Rule view. This makes it inert.
-    // When .commit() is called, the value is reparsed and its DOM structure rebuilt.
-    data.swatch.nextSibling.textContent = value;
+    // When commit() is called, the value is reparsed and its DOM structure rebuilt.
+    this.swatch.nextSibling.textContent = value;
   }
 
   /**
-  * Commit this shape value change which triggers an expensive operation that rebuilds
-  * part of the DOM of the TextPropertyEditor. Called in a debounced manner.
+  * Commit a shape value change which triggers an expensive operation that rebuilds
+  * part of the DOM of the TextPropertyEditor. Called in a debounced manner; see
+  * constructor.
   *
   * @param {String} value
-  *        The shape value to set the current property to
+  *        The shape value for the current property
   */
   commit(value) {
-    if (!this.activeProperty) {
+    if (!this.textProperty) {
       return;
     }
     this.ruleView.once("ruleview-changed", this.onChangesApplied);
-    let data = this.links.get(this.activeProperty);
-    data.callbacks.onCommit(value);
+    this.textProperty.setValue(value);
   }
 
   /**
-  * Called once after the shape value has been written to the element's style an Rule
-  * view updated. Triggers an event on the HighlightersOverlay that is listened to by
+  * Handler for "ruleview-changed" event triggered by the Rule view.
+  * Called once after the shape value has been written to the element's style and Rule
+  * view updated. Triggers an event on the HighlightersOverlay that is expected by
   * tests in order to check if the shape value has been correctly applied.
   */
   onChangesApplied() {
+    // When TextPropertyEditor updates it thrashes the previous swatch DOM node. Find and
+    // store the new swatch node.
+    this.findSwatch();
     this.inspector.highlighters.emit("shapes-highlighter-changes-applied");
   }
 
   async destroy() {
     await this.hide();
-    await this.unlinkAll();
     this.highlighter.off("highlighter-event", this.onHighlighterEvent);
     this.ruleView.off("ruleview-changed", this.onRuleViewChanged);
     this.highligherEventHandlers = {};
   }
 }
 
 module.exports = ShapesInContextEditor;