Bug 1529606 - Add Copy Rule button to Changes panel. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 25 Feb 2019 22:20:53 +0000
changeset 460993 f5629bb3e671ad15ede908de3cc0b2b2be342728
parent 460992 987a62667969a5f88f8c3f4693299e097db60daf
child 460994 f0fb6764fdb7cd0bae97e21db7eb1cdab9cd9f9f
push id35613
push usernerli@mozilla.com
push dateTue, 26 Feb 2019 03:52:35 +0000
treeherdermozilla-central@faec87a80ed1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1529606
milestone67.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 1529606 - Add Copy Rule button to Changes panel. r=pbro Adds a button that shows up when hovering selectors in the Changes panel. When clicked, it invokes the same Copy Rule behavior implemented for the context menu: copies the full content of the rule with changes applied. The added/changed CSS class names use BEM notation. I intend to refactor the Changes panel stylesheet to BEM in a follow-up patch. Differential Revision: https://phabricator.services.mozilla.com/D20808
devtools/client/inspector/changes/ChangesView.js
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/test/browser_changes_rule_selector.js
devtools/client/locales/en-US/changes.properties
devtools/client/themes/changes.css
devtools/client/themes/variables.css
--- a/devtools/client/inspector/changes/ChangesView.js
+++ b/devtools/client/inspector/changes/ChangesView.js
@@ -35,16 +35,17 @@ class ChangesView {
     this.telemetry = this.inspector.telemetry;
     this.window = window;
 
     this.onAddChange = this.onAddChange.bind(this);
     this.onClearChanges = this.onClearChanges.bind(this);
     this.onChangesFront = this.onChangesFront.bind(this);
     this.onContextMenu = this.onContextMenu.bind(this);
     this.onCopy = this.onCopy.bind(this);
+    this.onCopyRule = this.copyRule.bind(this);
     this.destroy = this.destroy.bind(this);
 
     this.init();
   }
 
   get contextMenu() {
     if (!this._contextMenu) {
       this._contextMenu = new ChangesContextMenu(this);
@@ -52,16 +53,17 @@ class ChangesView {
 
     return this._contextMenu;
   }
 
   init() {
     const changesApp = ChangesApp({
       onContextMenu: this.onContextMenu,
       onCopy: this.onCopy,
+      onCopyRule: this.onCopyRule,
     });
 
     // listen to the front for initialization, add listeners
     // when it is ready
     this._getChangesFront();
 
     // Expose the provider to let inspector.js use it in setupSidebar.
     this.provider = createElement(Provider, {
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -18,23 +18,38 @@ class ChangesApp extends PureComponent {
   static get propTypes() {
     return {
       // Nested CSS rule tree structure of CSS changes grouped by source (stylesheet)
       changesTree: PropTypes.object.isRequired,
       // Event handler for "contextmenu" event
       onContextMenu: PropTypes.func.isRequired,
       // Event handler for "copy" event
       onCopy: PropTypes.func.isRequired,
+      // Event handler for click on "Copy Rule" button
+      onCopyRule: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
   }
 
+  renderCopyButton(ruleId) {
+    return dom.button(
+      {
+        className: "changes__copy-rule-button",
+        onClick: e => {
+          this.props.onCopyRule(ruleId);
+        },
+        title: getStr("changes.contextmenu.copyRuleDescription"),
+      },
+      getStr("changes.contextmenu.copyRule")
+    );
+  }
+
   renderDeclarations(remove = [], add = []) {
     const removals = remove
       // Sorting changed declarations in the order they appear in the Rules view.
       .sort((a, b) => a.index > b.index)
       .map(({property, value, index}) => {
         return CSSDeclaration({
           key: "remove-" + property + index,
           className: "level diff-remove",
@@ -59,23 +74,24 @@ class ChangesApp extends PureComponent {
 
     return [removals, additions];
   }
 
   renderRule(ruleId, rule, level = 0) {
     return dom.div(
       {
         key: ruleId,
-        className: "rule devtools-monospace",
+        className: "changes__rule devtools-monospace",
         "data-rule-id": ruleId,
         style: {
           "--diff-level": level,
         },
       },
       this.renderSelectors(rule.selectors),
+      this.renderCopyButton(ruleId),
       // Render any nested child rules if they exist.
       rule.children.map(childRule => {
         return this.renderRule(childRule.ruleId, childRule, level + 1);
       }),
       // Render any changed CSS declarations.
       this.renderDeclarations(rule.remove, rule.add),
       dom.div({ className: `level` }, "}")
     );
@@ -102,17 +118,17 @@ class ChangesApp extends PureComponent {
     }
 
     const elements = [];
 
     for (const [selector, diffClass] of selectorDiffClassMap) {
       elements.push(dom.div(
         {
           key: selector,
-          className: `level selector ${diffClass}`,
+          className: `level changes__selector ${diffClass}`,
           title: selector,
         },
         getDiffMarker(diffClass),
         selector,
         dom.span({}, " {")
       ));
     }
 
--- a/devtools/client/inspector/changes/test/browser_changes_rule_selector.js
+++ b/devtools/client/inspector/changes/test/browser_changes_rule_selector.js
@@ -35,20 +35,20 @@ add_task(async function() {
   const onRuleViewChanged = once(ruleView, "ruleview-changed");
   info("Pressing Enter key to commit the change");
   EventUtils.synthesizeKey("KEY_Enter");
   info("Waiting for rule view to update");
   await onRuleViewChanged;
   info("Wait for the change to be tracked");
   await onTrackChange;
 
-  const rules = panel.querySelectorAll(".rule");
+  const rules = panel.querySelectorAll(".changes__rule");
   is(rules.length, 1, "One rule was tracked as changed");
 
-  const selectors = rules.item(0).querySelectorAll(".selector");
+  const selectors = rules.item(0).querySelectorAll(".changes__selector");
   is(selectors.length, 2, "Two selectors were tracked as changed");
 
   const firstSelector = selectors.item(0);
   is(firstSelector.title, "div", "Old selector name was tracked.");
   ok(firstSelector.classList.contains("diff-remove"), "Old selector was removed.");
 
   const secondSelector = selectors.item(1);
   is(secondSelector.title, ".test", "New selector name was tracked.");
--- a/devtools/client/locales/en-US/changes.properties
+++ b/devtools/client/locales/en-US/changes.properties
@@ -41,15 +41,19 @@ changes.contextmenu.copyChanges=Copy Cha
 # option in Changes panel context menu which copies all changed CSS declarations from a
 # stylesheet
 changes.contextmenu.copyAllChanges=Copy All Changes
 
 # LOCALIZATION NOTE (changes.contextmenu.copyRule): Label for "Copy Rule" option in
 # Changes panel context menu which copies the complete contents of a CSS rule.
 changes.contextmenu.copyRule=Copy Rule
 
+# LOCALIZATION NOTE (changes.contextmenu.copyRuleDescription): Detailed explanation for
+# "Copy Rule" option in Changes panel. Used as title attribute on "Copy Rule" button.
+changes.contextmenu.copyRuleDescription=Copy contents of this CSS rule to clipboard.
+
 # LOCALIZATION NOTE (changes.contextmenu.selectAll): Label for "Select All" option in the
 # Changes panel context menu to select all text content.
 changes.contextmenu.selectAll=Select All
 
 # LOCALIZATION NOTE (changes.contextmenu.selectAll.accessKey): Access key for "Select All"
 # option in the Changes panel.
 changes.contextmenu.selectAll.accessKey=A
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -1,29 +1,33 @@
 /* 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/. */
 
  /* CSS Variables specific to the Changes panel that aren't defined by the themes */
  :root {
+   --changes-button-background-color: var(--grey-90-a10);
+   --changes-button-background-color-hover: var(--grey-90-a15);
+   --changes-button-background-color-active: var(--grey-90-a20);
    --diff-add-background-color: #f1feec;
    --diff-add-text-color: #54983f;
    --diff-remove-background-color: #fbf2f5;
    --diff-remove-text-color: #bf7173;
    --diff-source-background: var(--theme-toolbar-background);
    --diff-level: 0;
    --diff-level-offset: 10px;
    /*
     Minimum padding so content on the first level (zero) isn't touching the edge. Added
     and removed lines will re-declare this to add extra padding to clear the +/- icons.
    */
    --diff-level-min-offset: 5px;
  }
 
 :root.theme-dark {
+  --changes-button-background-color: #252526;
   --diff-add-background-color: rgba(18, 188, 0, 0.15);
   --diff-add-text-color: #12BC00;
   --diff-remove-background-color: rgba(255, 0, 57, 0.15);
   --diff-remove-text-color: #FF0039;
   --diff-source-background: #222225;
 }
 
 :root[dir="rtl"] {
@@ -72,20 +76,52 @@
 #sidebar-panel-changes .level {
   padding-top: 3px;
   padding-right: 5px;
   padding-bottom: 3px;
   padding-left: calc(var(--diff-level-min-offset) +
                      var(--diff-level-offset) * var(--diff-level));
 }
 
-#sidebar-panel-changes .selector {
+/* Hide the Copy Rule button by default. */
+.changes__copy-rule-button {
+  background-color: var(--changes-button-background-color);
+  border-radius: 5px;
+  border: none;
+  color: var(--theme-body-color);
+  display: none;
+  padding: 1px 5px;
+  position: absolute;
+  right: 5px;
+  top: 2px;
+}
+
+.changes__copy-rule-button:hover {
+  background-color: var(--changes-button-background-color-hover);
+}
+
+.changes__copy-rule-button:active {
+  background-color: var(--changes-button-background-color-active);
+}
+
+.changes__rule {
+  position: relative;
+}
+
+.changes__selector {
   word-wrap: break-word;
 }
 
+/* Show the Copy Rule button when hovering over the rule's selector elements */
+.changes__selector:hover + .changes__copy-rule-button,
+.changes__selector:hover + .changes__selector + .changes__copy-rule-button,
+.changes__copy-rule-button:hover {
+  display: block;
+}
+
 #sidebar-panel-changes .declaration-name {
   margin-left: 10px;
 }
 
 .diff-add,
 .diff-remove {
   --diff-level-min-offset: 15px;
   position: relative;
--- a/devtools/client/themes/variables.css
+++ b/devtools/client/themes/variables.css
@@ -250,9 +250,12 @@
   --grey-45: #939395;
   --grey-50: #737373;
   --grey-55: #5c5c5f;
   --grey-60: #4a4a4f;
   --grey-70: #38383d;
   --grey-80: #2a2a2e;
   --grey-85: #1b1b1d;
   --grey-90: #0c0c0d;
+  --grey-90-a10: rgba(12, 12, 13, 0.1);
+  --grey-90-a15: rgba(12, 12, 13, 0.15);
+  --grey-90-a20: rgba(12, 12, 13, 0.2);
 }