Bug 1521674 - Implement property name editor in the new rules view. r=rcaliman
authorGabriel Luong <gabriel.luong@gmail.com>
Tue, 22 Jan 2019 22:20:05 -0500
changeset 514970 9da9417d2cb94573b490905bfea14b454568a489
parent 514969 48afe4849336998668fc7761597f0c74b57a656a
child 514971 8c488b723dc7a5ee007d3081b651d0b55e2b0883
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrcaliman
bugs1521674
milestone66.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 1521674 - Implement property name editor in the new rules view. r=rcaliman
devtools/client/inspector/rules/components/Declaration.js
devtools/client/inspector/rules/components/Declarations.js
devtools/client/inspector/rules/components/Rule.js
devtools/client/inspector/rules/components/Rules.js
devtools/client/inspector/rules/components/RulesApp.js
devtools/client/inspector/rules/models/element-style.js
devtools/client/inspector/rules/models/rule.js
devtools/client/inspector/rules/models/text-property.js
devtools/client/inspector/rules/new-rules.js
--- a/devtools/client/inspector/rules/components/Declaration.js
+++ b/devtools/client/inspector/rules/components/Declaration.js
@@ -1,41 +1,62 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const { PureComponent } = require("devtools/client/shared/vendor/react");
+const { createRef, 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 { editableItem } = require("devtools/client/shared/inplace-editor");
 
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
 class Declaration extends PureComponent {
   static get propTypes() {
     return {
       declaration: PropTypes.shape(Types.declaration).isRequired,
+      isUserAgentStyle: PropTypes.bool.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
+      showDeclarationNameEditor: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.state = {
       // Whether or not the computed property list is expanded.
       isComputedListExpanded: false,
     };
 
+    this.nameSpanRef = createRef();
+
     this.onComputedExpanderClick = this.onComputedExpanderClick.bind(this);
     this.onToggleDeclarationClick = this.onToggleDeclarationClick.bind(this);
   }
 
+  componentDidMount() {
+    if (this.props.isUserAgentStyle) {
+      // Declaration is not editable.
+      return;
+    }
+
+    const { declaration } = this.props;
+
+    editableItem({
+      element: this.nameSpanRef.current,
+    }, element => {
+      this.props.showDeclarationNameEditor(element, declaration.ruleId,
+        declaration.id);
+    });
+  }
+
   onComputedExpanderClick(event) {
     event.stopPropagation();
 
     this.setState(prevState => {
       return { isComputedListExpanded: !prevState.isComputedListExpanded };
     });
   }
 
@@ -139,17 +160,24 @@ class Declaration extends PureComponent 
         dom.div({ className: "ruleview-propertycontainer" },
           dom.div({
             className: "ruleview-enableproperty theme-checkbox" +
                         (isEnabled ? " checked" : ""),
             onClick: this.onToggleDeclarationClick,
             tabIndex: -1,
           }),
           dom.span({ className: "ruleview-namecontainer" },
-            dom.span({ className: "ruleview-propertyname theme-fg-color3" }, name),
+            dom.span(
+              {
+                className: "ruleview-propertyname theme-fg-color3",
+                ref: this.nameSpanRef,
+                tabIndex: 0,
+              },
+              name
+            ),
             ": "
           ),
           dom.span({
             className: "ruleview-expander theme-twisty" +
                        (this.state.isComputedListExpanded ? " open" : ""),
             onClick: this.onComputedExpanderClick,
             style: { display: computedProperties.length ? "inline-block" : "none" },
           }),
--- a/devtools/client/inspector/rules/components/Declarations.js
+++ b/devtools/client/inspector/rules/components/Declarations.js
@@ -11,37 +11,43 @@ const PropTypes = require("devtools/clie
 const Declaration = createFactory(require("./Declaration"));
 
 const Types = require("../types");
 
 class Declarations extends PureComponent {
   static get propTypes() {
     return {
       declarations: PropTypes.arrayOf(PropTypes.shape(Types.declaration)).isRequired,
+      isUserAgentStyle: PropTypes.bool.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
+      showDeclarationNameEditor: PropTypes.func.isRequired,
     };
   }
 
   render() {
     const {
       declarations,
+      isUserAgentStyle,
       onToggleDeclaration,
+      showDeclarationNameEditor,
     } = this.props;
 
     if (!declarations.length) {
       return null;
     }
 
     return (
       dom.ul({ className: "ruleview-propertylist" },
         declarations.map(declaration => {
           return Declaration({
             key: declaration.id,
             declaration,
+            isUserAgentStyle,
             onToggleDeclaration,
+            showDeclarationNameEditor,
           });
         })
       )
     );
   }
 }
 
 module.exports = Declarations;
--- a/devtools/client/inspector/rules/components/Rule.js
+++ b/devtools/client/inspector/rules/components/Rule.js
@@ -16,25 +16,27 @@ const SourceLink = createFactory(require
 const Types = require("../types");
 
 class Rule extends PureComponent {
   static get propTypes() {
     return {
       onToggleDeclaration: PropTypes.func.isRequired,
       onToggleSelectorHighlighter: PropTypes.func.isRequired,
       rule: PropTypes.shape(Types.rule).isRequired,
+      showDeclarationNameEditor: PropTypes.func.isRequired,
       showSelectorEditor: PropTypes.func.isRequired,
     };
   }
 
   render() {
     const {
       onToggleDeclaration,
       onToggleSelectorHighlighter,
       rule,
+      showDeclarationNameEditor,
       showSelectorEditor,
     } = this.props;
     const {
       declarations,
       id,
       isUnmatched,
       isUserAgentStyle,
       selector,
@@ -65,17 +67,19 @@ class Rule extends PureComponent {
                 selector,
               })
               :
               null,
             dom.span({ className: "ruleview-ruleopen" }, " {")
           ),
           Declarations({
             declarations,
+            isUserAgentStyle,
             onToggleDeclaration,
+            showDeclarationNameEditor,
           }),
           dom.div({ className: "ruleview-ruleclose" }, "}")
         )
       )
     );
   }
 }
 
--- a/devtools/client/inspector/rules/components/Rules.js
+++ b/devtools/client/inspector/rules/components/Rules.js
@@ -12,33 +12,36 @@ const Rule = createFactory(require("./Ru
 const Types = require("../types");
 
 class Rules extends PureComponent {
   static get propTypes() {
     return {
       onToggleDeclaration: PropTypes.func.isRequired,
       onToggleSelectorHighlighter: PropTypes.func.isRequired,
       rules: PropTypes.arrayOf(PropTypes.shape(Types.rule)).isRequired,
+      showDeclarationNameEditor: PropTypes.func.isRequired,
       showSelectorEditor: PropTypes.func.isRequired,
     };
   }
 
   render() {
     const {
       onToggleDeclaration,
       onToggleSelectorHighlighter,
       rules,
+      showDeclarationNameEditor,
       showSelectorEditor,
     } = this.props;
 
     return rules.map(rule => {
       return Rule({
         key: rule.id,
         onToggleDeclaration,
         onToggleSelectorHighlighter,
         rule,
+        showDeclarationNameEditor,
         showSelectorEditor,
       });
     });
   }
 }
 
 module.exports = Rules;
--- a/devtools/client/inspector/rules/components/RulesApp.js
+++ b/devtools/client/inspector/rules/components/RulesApp.js
@@ -30,24 +30,26 @@ class RulesApp extends PureComponent {
     return {
       onAddClass: PropTypes.func.isRequired,
       onSetClassState: PropTypes.func.isRequired,
       onToggleClassPanelExpanded: PropTypes.func.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
       onTogglePseudoClass: PropTypes.func.isRequired,
       onToggleSelectorHighlighter: PropTypes.func.isRequired,
       rules: PropTypes.arrayOf(PropTypes.shape(Types.rule)).isRequired,
+      showDeclarationNameEditor: PropTypes.func.isRequired,
       showSelectorEditor: PropTypes.func.isRequired,
     };
   }
 
   getRuleProps() {
     return {
       onToggleDeclaration: this.props.onToggleDeclaration,
       onToggleSelectorHighlighter: this.props.onToggleSelectorHighlighter,
+      showDeclarationNameEditor: this.props.showDeclarationNameEditor,
       showSelectorEditor: this.props.showSelectorEditor,
     };
   }
 
   renderInheritedRules(rules) {
     if (!rules.length) {
       return null;
     }
--- a/devtools/client/inspector/rules/models/element-style.js
+++ b/devtools/client/inspector/rules/models/element-style.js
@@ -5,16 +5,17 @@
 "use strict";
 
 const promise = require("promise");
 const Rule = require("devtools/client/inspector/rules/models/rule");
 const UserProperties = require("devtools/client/inspector/rules/models/user-properties");
 const { ELEMENT_STYLE } = require("devtools/shared/specs/styles");
 
 loader.lazyRequireGetter(this, "promiseWarn", "devtools/client/inspector/shared/utils", true);
+loader.lazyRequireGetter(this, "parseDeclarations", "devtools/shared/css/parsing-utils", true);
 loader.lazyRequireGetter(this, "isCssVariable", "devtools/shared/fronts/css-properties", true);
 
 /**
  * ElementStyle is responsible for the following:
  *   Keeps track of which properties are overridden.
  *   Maintains a list of Rule objects for a given element.
  *
  * @param  {Element} element
@@ -328,16 +329,52 @@ ElementStyle.prototype = {
       // overridden state has changed for the text property.
       if (this._updatePropertyOverridden(textProp)) {
         textProp.updateEditor();
       }
     }
   },
 
   /**
+   * Given the id of the rule and the new declaration name, modifies the existing
+   * declaration name to the new given value.
+   *
+   * @param  {String} ruleID
+   *         The Rule id of the given CSS declaration.
+   * @param  {String} declarationId
+   *         The TextProperty id for the CSS declaration.
+   * @param  {String} name
+   *         The new declaration name.
+   */
+  modifyDeclarationName: async function(ruleID, declarationId, name) {
+    const rule = this.getRule(ruleID);
+    if (!rule) {
+      return;
+    }
+
+    const declaration = rule.getDeclaration(declarationId);
+    if (!declaration || declaration.name === name) {
+      return;
+    }
+
+    // Adding multiple rules inside of name field overwrites the current
+    // property with the first, then adds any more onto the property list.
+    const declarations = parseDeclarations(this.cssProperties.isKnown, name);
+    if (!declarations.length) {
+      return;
+    }
+
+    await declaration.setName(declarations[0].name);
+
+    if (!declaration.enabled) {
+      await declaration.setEnabled(true);
+    }
+  },
+
+  /**
    * Modifies the existing rule's selector to the new given value.
    *
    * @param {String} ruleId
    *        The id of the Rule to be modified.
    * @param {String} selector
    *        The new selector value.
    */
   modifySelector: async function(ruleId, selector) {
--- a/devtools/client/inspector/rules/models/rule.js
+++ b/devtools/client/inspector/rules/models/rule.js
@@ -382,26 +382,27 @@ Rule.prototype = {
 
   /**
    * Renames a property.
    *
    * @param {TextProperty} property
    *        The property to rename.
    * @param {String} name
    *        The new property name (such as "background" or "border-top").
+   * @return {Promise}
    */
   setPropertyName: function(property, name) {
     if (name === property.name) {
-      return;
+      return Promise.resolve();
     }
 
     const oldName = property.name;
     property.name = name;
     const index = this.textProps.indexOf(property);
-    this.applyProperties((modifications) => {
+    return this.applyProperties(modifications => {
       modifications.renameProperty(index, oldName, name);
     });
   },
 
   /**
    * Sets the value and priority of a property, then reapply all properties.
    *
    * @param {TextProperty} property
@@ -416,17 +417,17 @@ Rule.prototype = {
     if (value === property.value && priority === property.priority) {
       return Promise.resolve();
     }
 
     property.value = value;
     property.priority = priority;
 
     const index = this.textProps.indexOf(property);
-    return this.applyProperties((modifications) => {
+    return this.applyProperties(modifications => {
       modifications.setProperty(index, property.name, value, priority);
     });
   },
 
   /**
    * Just sets the value and priority of a property, in order to preview its
    * effect on the content document.
    *
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -157,25 +157,24 @@ TextProperty.prototype = {
    */
   updateValue: function(value) {
     if (value !== this.value) {
       this.value = value;
       this.updateEditor();
     }
   },
 
-  setName: function(name) {
-    const store = this.rule.elementStyle.store;
-
-    if (name !== this.name) {
+  setName: async function(name) {
+    if (name !== this.name && this.editor) {
+      const store = this.rule.elementStyle.store;
       store.userProperties.setProperty(this.rule.domRule, name,
                                        this.editor.committed.value);
     }
 
-    this.rule.setPropertyName(this, name);
+    await this.rule.setPropertyName(this, name);
     this.updateEditor();
   },
 
   setEnabled: function(value) {
     this.rule.setPropertyEnabled(this, value);
     this.updateEditor();
   },
 
--- a/devtools/client/inspector/rules/new-rules.js
+++ b/devtools/client/inspector/rules/new-rules.js
@@ -26,16 +26,17 @@ const {
 
 const RulesApp = createFactory(require("./components/RulesApp"));
 
 const { LocalizationHelper } = require("devtools/shared/l10n");
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
 
 loader.lazyRequireGetter(this, "ClassList", "devtools/client/inspector/rules/models/class-list");
+loader.lazyRequireGetter(this, "AutocompletePopup", "devtools/client/shared/autocomplete-popup");
 loader.lazyRequireGetter(this, "InplaceEditor", "devtools/client/shared/inplace-editor", true);
 
 const PREF_UA_STYLES = "devtools.inspector.showUserAgentStyles";
 
 class RulesView {
   constructor(inspector, window) {
     this.cssProperties = inspector.cssProperties;
     this.doc = window.document;
@@ -50,16 +51,17 @@ class RulesView {
 
     this.onAddClass = this.onAddClass.bind(this);
     this.onSelection = this.onSelection.bind(this);
     this.onSetClassState = this.onSetClassState.bind(this);
     this.onToggleClassPanelExpanded = this.onToggleClassPanelExpanded.bind(this);
     this.onToggleDeclaration = this.onToggleDeclaration.bind(this);
     this.onTogglePseudoClass = this.onTogglePseudoClass.bind(this);
     this.onToggleSelectorHighlighter = this.onToggleSelectorHighlighter.bind(this);
+    this.showDeclarationNameEditor = this.showDeclarationNameEditor.bind(this);
     this.showSelectorEditor = this.showSelectorEditor.bind(this);
     this.updateClassList = this.updateClassList.bind(this);
     this.updateRules = this.updateRules.bind(this);
 
     this.inspector.sidebar.on("select", this.onSelection);
     this.selection.on("detached-front", this.onSelection);
     this.selection.on("new-node-front", this.onSelection);
 
@@ -75,16 +77,17 @@ class RulesView {
 
     const rulesApp = RulesApp({
       onAddClass: this.onAddClass,
       onSetClassState: this.onSetClassState,
       onToggleClassPanelExpanded: this.onToggleClassPanelExpanded,
       onToggleDeclaration: this.onToggleDeclaration,
       onTogglePseudoClass: this.onTogglePseudoClass,
       onToggleSelectorHighlighter: this.onToggleSelectorHighlighter,
+      showDeclarationNameEditor: this.showDeclarationNameEditor,
       showSelectorEditor: this.showSelectorEditor,
     });
 
     const provider = createElement(Provider, {
       id: "ruleview",
       key: "ruleview",
       store: this.store,
       title: INSPECTOR_L10N.getStr("inspector.sidebar.ruleViewTitle"),
@@ -94,16 +97,21 @@ class RulesView {
     this.provider = provider;
   }
 
   destroy() {
     this.inspector.sidebar.off("select", this.onSelection);
     this.selection.off("detached-front", this.onSelection);
     this.selection.off("new-node-front", this.onSelection);
 
+    if (this._autocompletePopup) {
+      this._autocompletePopup.destroy();
+      this._autocompletePopup = null;
+    }
+
     if (this._classList) {
       this._classList.off("current-node-class-changed", this.refreshClassList);
       this._classList.destroy();
       this._classList = null;
     }
 
     if (this._selectHighlighter) {
       this._selectorHighlighter.finalize();
@@ -123,16 +131,32 @@ class RulesView {
     this.selection = null;
     this.showUserAgentStyles = null;
     this.store = null;
     this.telemetry = null;
     this.toolbox = null;
   }
 
   /**
+   * Get an instance of the AutocompletePopup.
+   *
+   * @return {AutocompletePopup}
+   */
+  get autocompletePopup() {
+    if (!this._autocompletePopup) {
+      this._autocompletePopup = new AutocompletePopup(this.doc, {
+        autoSelect: true,
+        theme: "auto",
+      });
+    }
+
+    return this._autocompletePopup;
+  }
+
+  /**
    * Get an instance of the ClassList model used to manage the list of CSS classes
    * applied to the element.
    *
    * @return {ClassList} used to manage the list of CSS classes applied to the element.
    */
   get classList() {
     if (!this._classList) {
       this._classList = new ClassList(this.inspector);
@@ -323,16 +347,47 @@ class RulesView {
       this.highlighters.selectorHighlighterShown = null;
       this.store.dispatch(updateHighlightedSelector(""));
       // This event is emitted for testing purposes.
       this.emit("ruleview-selectorhighlighter-toggled", false);
     }
   }
 
   /**
+   * Handler for showing the inplace editor when an editable property name is clicked in
+   * the rules view.
+   *
+   * @param  {DOMNode} element
+   *         The declaration name span element to be edited.
+   * @param  {String} ruleId
+   *         The id of the Rule object to be edited.
+   * @param  {String} declarationId
+   *         The id of the TextProperty object to be edited.
+   */
+  showDeclarationNameEditor(element, ruleId, declarationId) {
+    new InplaceEditor({
+      advanceChars: ":",
+      contentType: InplaceEditor.CONTENT_TYPES.CSS_PROPERTY,
+      cssProperties: this.cssProperties,
+      done: (name, commit) => {
+        if (!commit) {
+          return;
+        }
+
+        this.elementStyle.modifyDeclarationName(ruleId, declarationId, name);
+        this.telemetry.recordEvent("edit_rule", "ruleview", null, {
+          "session_id": this.toolbox.sessionId,
+        });
+      },
+      element,
+      popup: this.autocompletePopup,
+    });
+  }
+
+  /**
    * Shows the inplace editor for the a selector.
    *
    * @param  {DOMNode} element
    *         The selector's span element to show the inplace editor.
    * @param  {String} ruleId
    *         The id of the Rule to be modified.
    */
   showSelectorEditor(element, ruleId) {