Bug 1530341 - (Part 2) Add Copy All Changes button to Changes panel. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 25 Feb 2019 22:37:02 +0000
changeset 518878 ea28e78057d5a0492582727564e2220322a690a6
parent 518877 d744e4b8444357f5507c22ced0da7e6dac2fe441
child 518879 8136a17fbec93c9b9fa698a224b39928a829cd91
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1530341, 1524548
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 1530341 - (Part 2) Add Copy All Changes button to Changes panel. r=gl Depends on D21007 Adds a Copy All Changes button to the toolbar of the Changes panel. When pressed, this builds a stylesheet out of the changes for all sources tracked (stylesheets, element styles, etc) The output format is the same as the now defunct Bug 1524548 with the added code comment as separator between the sources. Differential Revision: https://phabricator.services.mozilla.com/D21008
devtools/client/inspector/changes/ChangesView.js
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/selectors/changes.js
devtools/client/inspector/changes/test/unit/test_changes_stylesheet.js
devtools/client/locales/en-US/changes.properties
devtools/client/themes/changes.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.onCopyAllChanges = this.copyAllChanges.bind(this);
     this.onCopyRule = this.copyRule.bind(this);
     this.destroy = this.destroy.bind(this);
 
     this.init();
   }
 
   get contextMenu() {
     if (!this._contextMenu) {
@@ -53,16 +54,17 @@ class ChangesView {
 
     return this._contextMenu;
   }
 
   init() {
     const changesApp = ChangesApp({
       onContextMenu: this.onContextMenu,
       onCopy: this.onCopy,
+      onCopyAllChanges: this.onCopyAllChanges,
       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.
@@ -102,16 +104,24 @@ class ChangesView {
       // The connection to the server may have been cut, for
       // example during test
       // teardown. Here we just catch the error and silently
       // ignore it.
     }
   }
 
   /**
+   * Handler for the "Copy All Changes" button. Simple wrapper that just calls
+   * |this.copyChanges()| with no filters in order to trigger default operation.
+   */
+  copyAllChanges() {
+    this.copyChanges();
+  }
+
+  /**
    * Handler for the "Copy Changes" option from the context menu.
    * Builds a CSS text with the aggregated changes and copies it to the clipboard.
    *
    * Optional rule and source ids can be used to filter the scope of the operation:
    * - if both a rule id and source id are provided, copy only the changes to the
    * matching rule within the matching source.
    * - if only a source id is provided, copy the changes to all rules within the
    * matching source.
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -18,25 +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 All Changes" button
+      onCopyAllChanges: PropTypes.func.isRequired,
       // Event handler for click on "Copy Rule" button
       onCopyRule: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
   }
 
+  renderCopyAllChangesButton() {
+    return dom.button(
+      {
+        className: "changes__copy-all-changes-button",
+        onClick: this.props.onCopyAllChanges,
+        title: getStr("changes.contextmenu.copyAllChangesDescription"),
+      },
+      getStr("changes.contextmenu.copyAllChanges")
+    );
+  }
+
   renderCopyButton(ruleId) {
     return dom.button(
       {
         className: "changes__copy-rule-button",
         onClick: e => {
           this.props.onCopyRule(ruleId);
         },
         title: getStr("changes.contextmenu.copyRuleDescription"),
@@ -185,16 +198,17 @@ class ChangesApp extends PureComponent {
     return dom.div(
       {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-changes",
         onContextMenu: this.props.onContextMenu,
         onCopy: this.props.onCopy,
       },
       !hasChanges && this.renderEmptyState(),
+      hasChanges && this.renderCopyAllChangesButton(),
       hasChanges && this.renderDiff(this.props.changesTree)
     );
   }
 }
 
 /**
  * Get a React element with text content of either a plus or minus sign according to
  * the given CSS class name used to mark added or removed lines in the changes diff view.
--- a/devtools/client/inspector/changes/selectors/changes.js
+++ b/devtools/client/inspector/changes/selectors/changes.js
@@ -220,20 +220,21 @@ function getChangesStylesheet(state, fil
 
     return removals + additions;
   }
 
   // Iterate through all sources in the change tree and build a CSS stylesheet string.
   return Object.entries(changeTree).reduce((stylesheetText, [sourceId, source]) => {
     const { href, rules } = source;
     // Write code comment with source origin
-    stylesheetText += `/* ${getSourceForDisplay(source)} | ${href} */\n`;
+    stylesheetText += `\n/* ${getSourceForDisplay(source)} | ${href} */\n`;
     // Write CSS rules
     stylesheetText += Object.entries(rules).reduce((str, [ruleId, rule]) => {
-      str += writeRule(ruleId, rule, 0);
+      // Add a new like only after top-level rules (level == 0)
+      str += writeRule(ruleId, rule, 0) + "\n";
       return str;
     }, "");
 
     return stylesheetText;
   }, "");
 }
 
 module.exports = {
--- a/devtools/client/inspector/changes/test/unit/test_changes_stylesheet.js
+++ b/devtools/client/inspector/changes/test/unit/test_changes_stylesheet.js
@@ -5,39 +5,39 @@
 
 // Check that getChangesStylesheet() serializes tracked changes from nested CSS rules
 // into the expected stylesheet format.
 
 const { getChangesStylesheet } = require("devtools/client/inspector/changes/selectors/changes");
 
 const { CHANGES_STATE } = require("resource://test/mocks");
 
-// Wrap multi-line string in backticks and trim to ensure exact string check in test.
+// Wrap multi-line string in backticks to ensure exact check in test, including new lines.
 const STYLESHEET_FOR_ANCESTOR = `
 /* Inline #0 | http://localhost:5000/at-rules-nested.html */
 
 @media (min-width: 50em) {
   @supports (display: grid) {
     body {
       /* background-color: royalblue; */
       background-color: red;
     }
   }
 }
-`.trim();
+`;
 
-// Wrap multi-line string in backticks and trim to ensure exact string check in test.
+// Wrap multi-line string in backticks to ensure exact check in test, including new lines.
 const STYLESHEET_FOR_DESCENDANT = `
 /* Inline #0 | http://localhost:5000/at-rules-nested.html */
 
 body {
   /* background-color: royalblue; */
   background-color: red;
 }
-`.trim();
+`;
 
 add_test(() => {
   info("Check stylesheet generated for the first ancestor in the CSS rule tree.");
   equal(getChangesStylesheet(CHANGES_STATE), STYLESHEET_FOR_ANCESTOR,
     "Stylesheet includes all ancestors.");
 
   info("Check stylesheet generated for the last descendant in the CSS rule tree.");
   const filter = { sourceIds: ["source1"], ruleIds: ["rule3"] };
--- a/devtools/client/locales/en-US/changes.properties
+++ b/devtools/client/locales/en-US/changes.properties
@@ -33,16 +33,21 @@ changes.contextmenu.copy=Copy
 # option in the Changes panel.
 changes.contextmenu.copy.accessKey=C
 
 # LOCALIZATION NOTE (changes.contextmenu.copyAllChanges): Label for "Copy All Changes"
 # 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.copyAllChangesDescription): Detailed explanation
+# for "Copy All Changes" option in Changes panel. Used as title attribute on "Copy All
+# Changes" button
+changes.contextmenu.copyAllChangesDescription=Copy a list of all CSS changes to clipboard.
+
 # 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.
 
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -76,16 +76,28 @@
 #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));
 }
 
+.changes__copy-all-changes-button {
+  -moz-context-properties: fill;
+  background: url(chrome://devtools/skin/images/copy.svg) no-repeat;
+  background-size: 16px;
+  border: none;
+  color: var(--theme-body-color);
+  fill: currentColor;
+  height: 16px;
+  margin: 7px 10px;
+  padding-left: 21px;
+}
+
 /* 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;