Bug 1518512 - (Part 2) Ensure all text in Changes panel can be copied. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Fri, 25 Jan 2019 15:50:35 +0000
changeset 515469 99655666ac5a6a7d387de42f5d5cf3d47f144fc1
parent 515468 21af538eb319b60bd10bf7cb542e51667af29a98
child 515470 b1df68082c2f195b02dd71d86d31878b7910b572
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)
reviewersgl
bugs1518512
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 1518512 - (Part 2) Ensure all text in Changes panel can be copied. r=gl Depends on D17255 Replaces CSS pseudo-elements for added/removed line markers with text elements that can be copied with the rest of the content. Differential Revision: https://phabricator.services.mozilla.com/D17256
devtools/client/inspector/changes/components/CSSDeclaration.js
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/themes/changes.css
--- a/devtools/client/inspector/changes/components/CSSDeclaration.js
+++ b/devtools/client/inspector/changes/components/CSSDeclaration.js
@@ -6,33 +6,36 @@
 
 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 {
+      className: PropTypes.string,
+      marker: PropTypes.object,
       property: PropTypes.string.isRequired,
       value: PropTypes.string.isRequired,
-      className: PropTypes.string,
     };
   }
 
   static get defaultProps() {
     return {
       className: "",
+      marker: null,
     };
   }
 
   render() {
-    const { property, value, className } = this.props;
+    const { className, marker, property, value } = this.props;
 
     return dom.div({ className: `declaration ${className}` },
+      marker,
       dom.span({ className: "declaration-name theme-fg-color3"}, 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
@@ -31,28 +31,30 @@ class ChangesApp extends PureComponent {
   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",
+          marker: getDiffMarker("diff-remove"),
           property,
           value,
         });
       });
 
     const additions = add
       // 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: "add-" + property + index,
           className: "level diff-add",
+          marker: getDiffMarker("diff-add"),
           property,
           value,
         });
       });
 
     return [removals, additions];
   }
 
@@ -74,26 +76,30 @@ class ChangesApp extends PureComponent {
           "--diff-level": level,
         },
       },
       dom.div(
         {
           className: `level selector ${diffClass}`,
           title: selector,
         },
+        getDiffMarker(diffClass),
         selector,
-        dom.span({ className: "bracket-open" }, "{")
+        dom.span({ className: "bracket-open" }, " {")
       ),
       // 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 bracket-close ${diffClass}` }, "}")
+      dom.div({ className: `level bracket-close ${diffClass}` },
+        getDiffMarker(diffClass),
+        "}"
+      )
     );
   }
 
   renderDiff(changes = {}) {
     // Render groups of style sources: stylesheets and element style attributes.
     return Object.entries(changes).map(([sourceId, source]) => {
       const path = getSourceForDisplay(source);
       const { href, rules, isFramed } = source;
@@ -145,15 +151,39 @@ class ChangesApp extends PureComponent {
         onContextMenu: this.props.onContextMenu,
       },
       !hasChanges && this.renderEmptyState(),
       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.
+ * This is used as a diff line maker that can be copied over as text with the rest of the
+ * content. CSS pseudo-elements are not part of the document flow and cannot be copied.
+ *
+ * @param  {String} className
+ *         One of "diff-add" or "diff-remove"
+ * @return {Component|null}
+ *         Returns null if the given className isn't recognized. React handles it.
+ */
+function getDiffMarker(className) {
+  let marker = null;
+  switch (className) {
+    case "diff-add":
+      marker = dom.span({ className: "diff-marker" }, "+ ");
+      break;
+    case "diff-remove":
+      marker = dom.span({ className: "diff-marker" }, "- ");
+      break;
+  }
+  return marker;
+}
+
 const mapStateToProps = state => {
   return {
     changesTree: getChangesTree(state.changes),
   };
 };
 
 module.exports = connect(mapStateToProps)(ChangesApp);
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -70,17 +70,16 @@
 }
 
 #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));
-  position: relative;
 }
 
 #sidebar-panel-changes .selector {
   word-wrap: break-word;
 }
 
 #sidebar-panel-changes .rule .bracket-open {
   display: inline-block;
@@ -93,37 +92,35 @@
 
 #sidebar-panel-changes .declaration-value {
   margin-left: 5px;
 }
 
 .diff-add,
 .diff-remove {
   --diff-level-min-offset: 15px;
+  position: relative;
 }
 
-.diff-add::before,
-.diff-remove::before {
+.diff-marker {
   position: absolute;
   left: 5px;
 }
 
 .diff-add {
   background-color: var(--diff-add-background-color);
 }
 
-.diff-add::before {
-  content: "+";
+.diff-add .diff-marker {
   color: var(--diff-add-text-color);
 }
 
 .diff-remove {
   background-color: var(--diff-remove-background-color);
 }
 
-.diff-remove::before{
-  content: "-";
+.diff-remove .diff-marker{
   color: var(--diff-remove-text-color);
 }
 
 #sidebar-panel-changes .devtools-sidepanel-no-result :not(:first-child) {
   font-style: normal;
 }