Bug 1519988 - Implement toggling on/off of a CSS declaration. r=rcaliman
authorGabriel Luong <gabriel.luong@gmail.com>
Wed, 16 Jan 2019 21:05:25 -0500
changeset 514209 83c3a433bd154745fcf7ec32cf35556bc6a4fe62
parent 514207 913880b6d5c60c9fc4c2484036ca37c633bcd2b9
child 514210 cc1733154e3d61af0f0ceb4e759cba8eb191d737
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
bugs1519988
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 1519988 - Implement toggling on/off of a CSS declaration. 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
devtools/client/inspector/rules/reducers/rules.js
--- a/devtools/client/inspector/rules/components/Declaration.js
+++ b/devtools/client/inspector/rules/components/Declaration.js
@@ -9,38 +9,46 @@ const dom = require("devtools/client/sha
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const Types = require("../types");
 
 class Declaration extends PureComponent {
   static get propTypes() {
     return {
       declaration: PropTypes.shape(Types.declaration).isRequired,
+      onToggleDeclaration: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.state = {
       // Whether or not the computed property list is expanded.
       isComputedListExpanded: false,
     };
 
     this.onComputedExpanderClick = this.onComputedExpanderClick.bind(this);
+    this.onToggleDeclarationClick = this.onToggleDeclarationClick.bind(this);
   }
 
   onComputedExpanderClick(event) {
     event.stopPropagation();
 
     this.setState(prevState => {
       return { isComputedListExpanded: !prevState.isComputedListExpanded };
     });
   }
 
+  onToggleDeclarationClick(event) {
+    event.stopPropagation();
+    const { id, ruleId } = this.props.declaration;
+    this.props.onToggleDeclaration(ruleId, id);
+  }
+
   renderComputedPropertyList() {
     const { computedProperties } = this.props.declaration;
 
     if (!computedProperties.length) {
       return null;
     }
 
     return (
@@ -126,16 +134,17 @@ class Declaration extends PureComponent 
           className: "ruleview-property" +
                      (!isEnabled || !isKnownProperty || isOverridden ?
                       " ruleview-overridden" : ""),
         },
         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-color5" }, name),
             ": "
           ),
           dom.span({
             className: "ruleview-expander theme-twisty" +
--- a/devtools/client/inspector/rules/components/Declarations.js
+++ b/devtools/client/inspector/rules/components/Declarations.js
@@ -11,32 +11,37 @@ 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,
+      onToggleDeclaration: PropTypes.func.isRequired,
     };
   }
 
   render() {
-    const { declarations } = this.props;
+    const {
+      declarations,
+      onToggleDeclaration,
+    } = this.props;
 
     if (!declarations.length) {
       return null;
     }
 
     return (
       dom.ul({ className: "ruleview-propertylist" },
         declarations.map(declaration => {
           return Declaration({
             key: declaration.id,
             declaration,
+            onToggleDeclaration,
           });
         })
       )
     );
   }
 }
 
 module.exports = Declarations;
--- a/devtools/client/inspector/rules/components/Rule.js
+++ b/devtools/client/inspector/rules/components/Rule.js
@@ -12,22 +12,26 @@ const Declarations = createFactory(requi
 const Selector = createFactory(require("./Selector"));
 const SourceLink = createFactory(require("./SourceLink"));
 
 const Types = require("../types");
 
 class Rule extends PureComponent {
   static get propTypes() {
     return {
+      onToggleDeclaration: PropTypes.func.isRequired,
       rule: PropTypes.shape(Types.rule).isRequired,
     };
   }
 
   render() {
-    const { rule } = this.props;
+    const {
+      onToggleDeclaration,
+      rule,
+    } = this.props;
     const {
       declarations,
       selector,
       sourceLink,
       type,
     } = rule;
 
     return (
@@ -37,17 +41,20 @@ class Rule extends PureComponent {
         dom.div({ className: "ruleview-code" },
           dom.div({},
             Selector({
               selector,
               type,
             }),
             dom.span({ className: "ruleview-ruleopen" }, " {")
           ),
-          Declarations({ declarations }),
+          Declarations({
+            declarations,
+            onToggleDeclaration,
+          }),
           dom.div({ className: "ruleview-ruleclose" }, "}")
         )
       )
     );
   }
 }
 
 module.exports = Rule;
--- a/devtools/client/inspector/rules/components/Rules.js
+++ b/devtools/client/inspector/rules/components/Rules.js
@@ -9,23 +9,30 @@ const PropTypes = require("devtools/clie
 
 const Rule = createFactory(require("./Rule"));
 
 const Types = require("../types");
 
 class Rules extends PureComponent {
   static get propTypes() {
     return {
+      onToggleDeclaration: PropTypes.func.isRequired,
       rules: PropTypes.arrayOf(PropTypes.shape(Types.rule)).isRequired,
     };
   }
 
   render() {
-    return this.props.rules.map(rule => {
+    const {
+      onToggleDeclaration,
+      rules,
+    } = this.props;
+
+    return rules.map(rule => {
       return Rule({
         key: rule.id,
+        onToggleDeclaration,
         rule,
       });
     });
   }
 }
 
 module.exports = Rules;
--- a/devtools/client/inspector/rules/components/RulesApp.js
+++ b/devtools/client/inspector/rules/components/RulesApp.js
@@ -23,16 +23,17 @@ const Toolbar = createFactory(require(".
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
 const SHOW_PSEUDO_ELEMENTS_PREF = "devtools.inspector.show_pseudo_elements";
 
 class RulesApp extends PureComponent {
   static get propTypes() {
     return {
+      onToggleDeclaration: PropTypes.func.isRequired,
       onTogglePseudoClass: PropTypes.func.isRequired,
       rules: PropTypes.arrayOf(PropTypes.shape(Types.rule)).isRequired,
     };
   }
 
   renderInheritedRules(rules) {
     if (!rules.length) {
       return null;
@@ -45,17 +46,20 @@ class RulesApp extends PureComponent {
       if (rule.inheritance.inherited !== lastInherited) {
         lastInherited = rule.inheritance.inherited;
 
         output.push(
           dom.div({ className: "ruleview-header" }, rule.inheritance.inheritedSource)
         );
       }
 
-      output.push(Rule({ rule }));
+      output.push(Rule({
+        onToggleDeclaration: this.props.onToggleDeclaration,
+        rule,
+      }));
     }
 
     return output;
   }
 
   renderKeyframesRules(rules) {
     if (!rules.length) {
       return null;
@@ -70,16 +74,17 @@ class RulesApp extends PureComponent {
       }
 
       lastKeyframes = rule.keyframesRule.id;
 
       const items = [
         {
           component: Rules,
           componentProps: {
+            onToggleDeclaration: this.props.onToggleDeclaration,
             rules: rules.filter(r => r.keyframesRule.id === lastKeyframes),
           },
           header: rule.keyframesRule.keyframesName,
           opened: true,
         },
       ];
 
       output.push(Accordion({ items }));
@@ -88,28 +93,34 @@ class RulesApp extends PureComponent {
     return output;
   }
 
   renderStyleRules(rules) {
     if (!rules.length) {
       return null;
     }
 
-    return Rules({ rules });
+    return Rules({
+      onToggleDeclaration: this.props.onToggleDeclaration,
+      rules,
+    });
   }
 
   renderPseudoElementRules(rules) {
     if (!rules.length) {
       return null;
     }
 
     const items = [
       {
         component: Rules,
-        componentProps: { rules },
+        componentProps: {
+          onToggleDeclaration: this.props.onToggleDeclaration,
+          rules,
+        },
         header: getStr("rule.pseudoElement"),
         opened: Services.prefs.getBoolPref(SHOW_PSEUDO_ELEMENTS_PREF),
         onToggled: () => {
           const opened = Services.prefs.getBoolPref(SHOW_PSEUDO_ELEMENTS_PREF);
           Services.prefs.setBoolPref(SHOW_PSEUDO_ELEMENTS_PREF, !opened);
         },
       },
     ];
--- a/devtools/client/inspector/rules/models/element-style.js
+++ b/devtools/client/inspector/rules/models/element-style.js
@@ -125,16 +125,27 @@ ElementStyle.prototype = {
       }
       return promiseWarn(e);
     });
     this.populated = populated;
     return this.populated;
   },
 
   /**
+   * Returns the Rule object of the given rule id.
+   *
+   * @param  {String} id
+   *         The id of the Rule object.
+   * @return {Rule|undefined} of the given rule id or undefined if it cannot be found.
+   */
+  getRule: function(id) {
+    return this.rules.find(rule => rule.domRule.actorID === id);
+  },
+
+  /**
    * Get the font families in use by the element.
    *
    * Returns a promise that will be resolved to a list of CSS family
    * names. The list might have duplicates.
    */
   getUsedFontFamilies: function() {
     return new Promise((resolve, reject) => {
       this.ruleView.styleWindow.requestIdleCallback(async () => {
@@ -317,16 +328,38 @@ ElementStyle.prototype = {
       // overridden state has changed for the text property.
       if (this._updatePropertyOverridden(textProp)) {
         textProp.updateEditor();
       }
     }
   },
 
   /**
+   * Toggles the enabled state of the given CSS declaration.
+   *
+   * @param {String} ruleId
+   *        The Rule id of the given CSS declaration.
+   * @param {String} declarationId
+   *        The TextProperty id for the CSS declaration.
+   */
+  toggleDeclaration: function(ruleId, declarationId) {
+    const rule = this.getRule(ruleId);
+    if (!rule) {
+      return;
+    }
+
+    const declaration = rule.getDeclaration(declarationId);
+    if (!declaration) {
+      return;
+    }
+
+    declaration.setEnabled(!declaration.enabled);
+  },
+
+  /**
    * Mark a given TextProperty as overridden or not depending on the
    * state of its computed properties. Clears the _overriddenDirty state
    * on all computed properties.
    *
    * @param  {TextProperty} prop
    *         The text property to update.
    * @return {Boolean} true if the TextProperty's overridden state (or any of
    *         its computed properties overridden state) changed.
--- a/devtools/client/inspector/rules/models/rule.js
+++ b/devtools/client/inspector/rules/models/rule.js
@@ -164,16 +164,28 @@ Rule.prototype = {
   /**
    * The rule's column within a stylesheet
    */
   get ruleColumn() {
     return this.domRule ? this.domRule.column : null;
   },
 
   /**
+   * Returns the TextProperty with the given id or undefined if it cannot be found.
+   *
+   * @param {String} id
+   *        A TextProperty id.
+   * @return {TextProperty|undefined} with the given id in the current Rule or undefined
+   * if it cannot be found.
+   */
+  getDeclaration: function(id) {
+    return this.textProps.find(textProp => textProp.id === id);
+  },
+
+  /**
    * Returns true if the rule matches the creation options
    * specified.
    *
    * @param {Object} options
    *        Creation options. See the Rule constructor for documentation.
    */
   matches: function(options) {
     return this.domRule === options.rule;
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -1,16 +1,18 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ts=2 et sw=2 tw=80: */
 /* 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 { generateUUID } = require("devtools/shared/generate-uuid");
+
 loader.lazyRequireGetter(this, "escapeCSSComment", "devtools/shared/css/parsing-utils", true);
 
 /**
  * TextProperty is responsible for the following:
  *   Manages a single property from the authoredText attribute of the
  *     relevant declaration.
  *   Maintains a list of computed properties that come from this
  *     property declaration.
@@ -30,16 +32,17 @@ loader.lazyRequireGetter(this, "escapeCS
  * @param {Boolean} invisible
  *        Whether the property is invisible.  An invisible property
  *        does not show up in the UI; these are needed so that the
  *        index of a property in Rule.textProps is the same as the index
  *        coming from parseDeclarations.
  */
 function TextProperty(rule, name, value, priority, enabled = true,
                       invisible = false) {
+  this.id = name + "_" + generateUUID().toString();
   this.rule = rule;
   this.name = name;
   this.value = value;
   this.priority = priority;
   this.enabled = !!enabled;
   this.invisible = invisible;
   this.cssProperties = this.rule.elementStyle.ruleView.cssProperties;
   this.panelDoc = this.rule.elementStyle.ruleView.inspector.panelDoc;
--- a/devtools/client/inspector/rules/new-rules.js
+++ b/devtools/client/inspector/rules/new-rules.js
@@ -27,36 +27,40 @@ const PREF_UA_STYLES = "devtools.inspect
 class RulesView {
   constructor(inspector, window) {
     this.cssProperties = inspector.cssProperties;
     this.doc = window.document;
     this.inspector = inspector;
     this.pageStyle = inspector.pageStyle;
     this.selection = inspector.selection;
     this.store = inspector.store;
+    this.telemetry = inspector.telemetry;
     this.toolbox = inspector.toolbox;
 
     this.showUserAgentStyles = Services.prefs.getBoolPref(PREF_UA_STYLES);
 
     this.onSelection = this.onSelection.bind(this);
+    this.onToggleDeclaration = this.onToggleDeclaration.bind(this);
     this.onTogglePseudoClass = this.onTogglePseudoClass.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);
 
     this.init();
   }
 
   init() {
     if (!this.inspector) {
       return;
     }
 
     const rulesApp = RulesApp({
+      onToggleDeclaration: this.onToggleDeclaration,
       onTogglePseudoClass: this.onTogglePseudoClass,
     });
 
     const provider = createElement(Provider, {
       id: "ruleview",
       key: "ruleview",
       store: this.store,
       title: INSPECTOR_L10N.getStr("inspector.sidebar.ruleViewTitle"),
@@ -79,16 +83,17 @@ class RulesView {
     this.cssProperties = null;
     this.doc = null;
     this.elementStyle = null;
     this.inspector = null;
     this.pageStyle = null;
     this.selection = null;
     this.showUserAgentStyles = null;
     this.store = null;
+    this.telemetry = null;
     this.toolbox = null;
   }
 
   /**
    * Creates a dummy element in the document that helps get the computed style in
    * TextProperty.
    *
    * @return {Element} used to get the computed style for text properties.
@@ -128,16 +133,31 @@ class RulesView {
       this.update();
       return;
     }
 
     this.update(this.selection.nodeFront);
   }
 
   /**
+   * Handler for toggling the enabled property for a given CSS declaration.
+   *
+   * @param  {String} ruleId
+   *         The Rule id of the given CSS declaration.
+   * @param  {String} declarationId
+   *         The TextProperty id for the CSS declaration.
+   */
+  onToggleDeclaration(ruleId, declarationId) {
+    this.elementStyle.toggleDeclaration(ruleId, declarationId);
+    this.telemetry.recordEvent("edit_rule", "ruleview", null, {
+      "session_id": this.toolbox.sessionId,
+    });
+  }
+
+  /**
    * Handler for toggling a pseudo class in the pseudo class panel. Toggles on and off
    * a given pseudo class value.
    *
    * @param  {String} value
    *         The pseudo class to toggle on or off.
    */
   onTogglePseudoClass(value) {
     this.store.dispatch(togglePseudoClass(value));
@@ -156,16 +176,25 @@ class RulesView {
     if (!element) {
       this.store.dispatch(disableAllPseudoClasses());
       this.store.dispatch(updateRules([]));
       return;
     }
 
     this.elementStyle = new ElementStyle(element, this, {}, this.pageStyle,
       this.showUserAgentStyles);
+    this.elementStyle.onChanged = this.updateRules;
     await this.elementStyle.populate();
 
     this.store.dispatch(setPseudoClassLocks(this.elementStyle.element.pseudoClassLocks));
+    this.updateRules();
+  }
+
+  /**
+   * Updates the rules view by dispatching the current rules state. This is called from
+   * the update() function, and from the ElementStyle's onChange() handler.
+   */
+  updateRules() {
     this.store.dispatch(updateRules(this.elementStyle.rules));
   }
 }
 
 module.exports = RulesView;
--- a/devtools/client/inspector/rules/reducers/rules.js
+++ b/devtools/client/inspector/rules/reducers/rules.js
@@ -14,53 +14,55 @@ const INITIAL_RULES = {
 };
 
 /**
  * Given a rule's TextProperty, returns the properties that are needed to render a
  * CSS declaration.
  *
  * @param  {TextProperty} declaration
  *         A TextProperty of a rule.
- * @param  {Number} index
- *         The index of the CSS declaration within the declaration block.
+ * @param  {String} ruleId
+ *         The rule id that is associated with the given CSS declaration.
  * @return {Object} containing the properties needed to render a CSS declaration.
  */
-function getDeclarationState(declaration, index) {
+function getDeclarationState(declaration, ruleId) {
   return {
     // Array of the computed properties for a CSS declaration.
     computedProperties: declaration.computedProperties,
     // An unique CSS declaration id.
-    id: `${declaration.name}${declaration.value}${index}`,
+    id: declaration.id,
     // Whether or not the declaration is enabled.
     isEnabled: declaration.enabled,
     // Whether or not the declaration's property name is known.
     isKnownProperty: declaration.isKnownProperty,
     // Whether or not the the declaration is overridden.
     isOverridden: !!declaration.overridden,
     // The declaration's property name.
     name: declaration.name,
     // The declaration's priority (either "important" or an empty string).
     priority: declaration.priority,
+    // The CSS rule id that is associated with this CSS declaration.
+    ruleId,
     // The declaration's property value.
     value: declaration.value,
   };
 }
 
 /**
  * Given a Rule, returns the properties that are needed to render a CSS rule.
  *
  * @param  {Rule} rule
  *         A Rule object containing information about a CSS rule.
  * @return {Object} containing the properties needed to render a CSS rule.
  */
 function getRuleState(rule) {
   return {
     // Array of CSS declarations.
-    declarations: rule.declarations.map((declaration, i) =>
-      getDeclarationState(declaration, i)),
+    declarations: rule.declarations.map(declaration =>
+      getDeclarationState(declaration, rule.domRule.actorID)),
     // An unique CSS rule id.
     id: rule.domRule.actorID,
     // An object containing information about the CSS rule's inheritance.
     inheritance: rule.inheritance,
     // Whether or not the rule does not match the current selected element.
     isUnmatched: rule.isUnmatched,
     // Whether or not the rule is an user agent style.
     isUserAgentStyle: rule.isSystem,