Bug 1499049 - (Part 5) Refactor Changes panel to render diff of nested CSS rule structures; r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 25 Oct 2018 13:42:05 +0000
changeset 443105 0b04c98133b3d22225d8aeb7c4df325ccaa0fea3
parent 443104 578d839f3c6c472b43b0fc9b4753549ebdb9ccb3
child 443106 f7565ed1376dc6cc9ffa6b9f054a9a4aeaa2f480
push id34939
push usernerli@mozilla.com
push dateFri, 26 Oct 2018 15:47:55 +0000
treeherdermozilla-central@760a16bf0d2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1499049
milestone65.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 1499049 - (Part 5) Refactor Changes panel to render diff of nested CSS rule structures; r=pbro Depends on D8721 - Externalize reusable CSSDeclaration React component; - Introduce the ability to render rules with unlimited levels of nesting, but cap indentation in the UI at a reasonable level; - Remove accordion behavior from rules, but keep for sources; - Cleanup CSS styles for Changes panel. Differential Revision: https://phabricator.services.mozilla.com/D8722
devtools/client/inspector/changes/components/CSSDeclaration.js
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/components/moz.build
devtools/client/themes/changes.css
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/components/CSSDeclaration.js
@@ -0,0 +1,38 @@
+/* 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 dom = require("devtools/client/shared/vendor/react-dom-factories");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
+class CSSDeclaration extends PureComponent {
+  static get propTypes() {
+    return {
+      property: PropTypes.string.isRequired,
+      value: PropTypes.string.isRequired,
+      className: PropTypes.string,
+    };
+  }
+
+  static get defaultProps() {
+    return {
+      className: "",
+    };
+  }
+
+  render() {
+    const { property, value, className } = this.props;
+
+    return dom.div({ className },
+      dom.span({ className: "declaration-name theme-fg-color5"}, property),
+      ":",
+      dom.span({ className: "declaration-value theme-fg-color1"}, value),
+      ";"
+    );
+  }
+}
+
+module.exports = CSSDeclaration;
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -1,78 +1,119 @@
 /* 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 { createFactory, 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 { connect } = require("devtools/client/shared/vendor/react-redux");
 
+const CSSDeclaration = createFactory(require("./CSSDeclaration"));
+
 class ChangesApp extends PureComponent {
   static get propTypes() {
     return {
+      // Redux state slice assigned to Track Changes feature; passed as prop by connect()
       changes: PropTypes.object.isRequired,
     };
   }
 
-  renderMutations(remove = {}, add = {}) {
-    const removals = Object.entries(remove).map(([prop, value]) => {
-      return dom.div(
-        { className: "line diff-remove"},
-        `${prop}: ${value};`
-      );
+  constructor(props) {
+    super(props);
+    // In the Redux store, all rules exist in a collection at the same level of nesting.
+    // Parent rules come before child rules. Parent/child dependencies are set
+    // via parameters in each rule pointing to the corresponding rule ids.
+    //
+    // To render rules, we traverse the descendant rule tree and render each child rule
+    // found. This means we get into situations where we can render the same rule multiple
+    // times: once as a child of its parent and once standalone.
+    //
+    // By keeping a log of rules previously rendered we prevent needless multi-rendering.
+    this.renderedRules = [];
+  }
+
+  renderDeclarations(remove = {}, add = {}) {
+    const removals = Object.entries(remove).map(([property, value]) => {
+      return CSSDeclaration({ className: "level diff-remove", property, value });
     });
 
-    const additions = Object.entries(add).map(([prop, value]) => {
-      return dom.div(
-        { className: "line diff-add"},
-        `${prop}: ${value};`
-      );
+    const additions = Object.entries(add).map(([property, value]) => {
+      return CSSDeclaration({ className: "level diff-add", property, value });
     });
 
     return [removals, additions];
   }
 
-  renderSelectors(selectors = {}) {
-    return Object.keys(selectors).map(sel => {
+  renderRule(ruleId, rule, rules) {
+    const selector = rule.selector;
+
+    if (this.renderedRules.includes(ruleId)) {
+      return null;
+    }
+
+    // Mark this rule as rendered so we don't render it again.
+    this.renderedRules.push(ruleId);
+
+    return dom.div(
+      {
+        className: "rule",
+      },
+      dom.div(
+        {
+          className: "level selector",
+          title: selector,
+        },
+        selector,
+        dom.span({ className: "bracket-open" }, "{")
+      ),
+      // Render any nested child rules if they are present.
+      rule.children.length > 0 && rule.children.map(childRuleId => {
+        return this.renderRule(childRuleId, rules[childRuleId], rules);
+      }),
+      // Render any changed CSS declarations.
+      this.renderDeclarations(rule.remove, rule.add),
+      dom.span({ className: "level bracket-close" }, "}")
+    );
+  }
+
+  renderDiff(changes = {}) {
+    // Render groups of style sources: stylesheets and element style attributes.
+    return Object.entries(changes).map(([sourceId, source]) => {
+      const href = source.href || `inline stylesheet #${source.index}`;
+      const rules = source.rules;
+
       return dom.details(
-        { className: "selector", open: true },
+        {
+          className: "source devtools-monospace",
+          open: true,
+        },
         dom.summary(
           {
-            title: sel,
-          },
-          sel),
-        this.renderMutations(selectors[sel].remove, selectors[sel].add)
-      );
-    });
-  }
-
-  renderDiff(diff = {}) {
-    // Render groups of style sources: stylesheets, embedded styles and inline styles
-    return Object.keys(diff).map(href => {
-      return dom.details(
-        { className: "source", open: true },
-        dom.summary(
-          {
+            className: "href",
             title: href,
           },
           href),
-        // Render groups of selectors
-        this.renderSelectors(diff[href])
+        // Render changed rules within this source.
+        Object.entries(rules).map(([ruleId, rule]) => {
+          return this.renderRule(ruleId, rule, rules);
+        })
       );
     });
   }
 
   render() {
+    // Reset log of rendered rules.
+    this.renderedRules = [];
+
     return dom.div(
       {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-changes",
       },
-      this.renderDiff(this.props.changes.diff)
+      this.renderDiff(this.props.changes)
     );
   }
 }
 
 module.exports = connect(state => state)(ChangesApp);
--- a/devtools/client/inspector/changes/components/moz.build
+++ b/devtools/client/inspector/changes/components/moz.build
@@ -1,9 +1,10 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 DevToolsModules(
     'ChangesApp.js',
+    'CSSDeclaration.js',
 )
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -3,78 +3,78 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
  /* CSS Variables specific to the Changes panel that aren't defined by the themes */
  :root {
    --diff-add-background-color: #f1feec;
    --diff-add-text-color: #54983f;
    --diff-remove-background-color: #fbf2f5;
    --diff-remove-text-color: #bf7173;
+   --diff-level-offset: 15px;
  }
 
 #sidebar-panel-changes {
   margin: 0;
+  padding: 0;
   width: 100%;
   height: 100%;
   overflow: auto;
 }
 
-details {
-  font-family: var(--monospace-font-family);
-  font-size: 12px;
-}
-
-summary {
-  outline: none;
-  padding: 5px 0;
-  -moz-user-select: none;
-  cursor: pointer;
-}
-
-details.source[open] {
+#sidebar-panel-changes .source[open] {
   padding-bottom: 10px;
 }
 
-details.source > summary {
+#sidebar-panel-changes .href {
   background: var(--theme-sidebar-background);
   border-top: 1px solid var(--theme-splitter-color);
   border-bottom: 1px solid var(--theme-splitter-color);
-  padding-left: 5px;
+  padding: 5px;
   white-space: nowrap;
   text-overflow: ellipsis;
   overflow: hidden;
+  cursor: pointer;
 }
 
-details.selector > summary {
-  padding-left: 10px;
+#sidebar-panel-changes .rule .level {
+  padding-top: 3px;
+  padding-right: 5px;
+  padding-bottom: 3px;
+  padding-left: var(--diff-level-offset);
+  position: relative;
 }
 
-details.selector summary::after{
-  content: "{…}";
-  display: inline-block;
-  padding-left: 5px;
+#sidebar-panel-changes .rule > .rule .level {
+  padding-left: calc(var(--diff-level-offset) * 2);
+}
+
+#sidebar-panel-changes .rule > .rule > .rule .level {
+  padding-left: calc(var(--diff-level-offset) * 3);
 }
 
-details.selector[open]{
-  margin-bottom: 5px;
+#sidebar-panel-changes .rule > .rule > .rule > .rule .level {
+  padding-left: calc(var(--diff-level-offset) * 4);
 }
 
-details.selector[open] summary::after{
-  content: "{";
+#sidebar-panel-changes .rule .selector,
+#sidebar-panel-changes .rule .bracket-close {
+  margin-left: calc(-1 * var(--diff-level-offset) + 5px);
 }
 
-details.selector[open]::after{
-  content: "}";
-  display: block;
-  padding-left: 10px;
+#sidebar-panel-changes .rule .bracket-open {
+  display: inline-block;
+  margin-left: 5px;
 }
 
-.line {
-  padding: 3px 5px 3px 15px;
-  position: relative;
+#sidebar-panel-changes .declaration-name {
+  margin-left: 10px;
+}
+
+#sidebar-panel-changes .declaration-value {
+  margin-left: 5px;
 }
 
 .diff-add::before,
 .diff-remove::before{
   position: absolute;
   left: 5px;
 }