Bug 1503924 - Trim source URLs to filename in Changes panel. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Fri, 23 Nov 2018 16:25:18 +0000
changeset 504764 e2d3dd273e41b208f434a19b48dc04e5efa64ad2
parent 504763 e45d7cae915a56013eaa2f6a8f09788fa0b032c7
child 504765 999546476a96355dd32bc1e3cdd230b8f58521c4
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1503924
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 1503924 - Trim source URLs to filename in Changes panel. r=gl Differential Revision: https://phabricator.services.mozilla.com/D12588
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/utils/changes-utils.js
devtools/client/inspector/changes/utils/l10n.js
devtools/client/locales/en-US/changes.properties
devtools/client/themes/changes.css
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -5,16 +5,17 @@
 "use strict";
 
 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"));
+const { getSourceForDisplay } = require("../utils/changes-utils");
 const { getStr } = require("../utils/l10n");
 
 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,
     };
@@ -77,17 +78,17 @@ class ChangesApp extends PureComponent {
       diffClass = "diff-add";
     } else if (rule.changeType === "rule-remove") {
       diffClass = "diff-remove";
     }
 
     return dom.div(
       {
         key: ruleId,
-        className: "rule",
+        className: "rule devtools-monospace",
       },
       dom.div(
         {
           className: `level selector ${diffClass}`,
           title: selector,
         },
         selector,
         dom.span({ className: "bracket-open" }, "{")
@@ -100,31 +101,31 @@ class ChangesApp extends PureComponent {
       this.renderDeclarations(rule.remove, rule.add),
       dom.div({ className: `level bracket-close ${diffClass}` }, "}")
     );
   }
 
   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;
+      const path = getSourceForDisplay(source);
+      const { href, rules } = source;
 
-      return dom.details(
+      return dom.div(
         {
           key: sourceId,
-          className: "source devtools-monospace",
-          open: true,
+          className: "source",
         },
-        dom.summary(
+        dom.div(
           {
             className: "href",
             title: href,
           },
-          href),
+          path
+        ),
         // Render changed rules within this source.
         Object.entries(rules).map(([ruleId, rule]) => {
           return this.renderRule(ruleId, rule, rules);
         })
       );
     });
   }
 
--- a/devtools/client/inspector/changes/utils/changes-utils.js
+++ b/devtools/client/inspector/changes/utils/changes-utils.js
@@ -1,14 +1,16 @@
 /* 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 { getFormatStr, getStr } = require("./l10n");
+
 /**
 * Generate a hash that uniquely identifies a stylesheet or element style attribute.
 *
 * @param {Object} source
 *        Information about a stylesheet or element style attribute:
 *        {
 *          type:  {String}
 *                 One of "stylesheet" or "element".
@@ -49,10 +51,44 @@ function getRuleHash(ruleData) {
   const atRules = ancestors.reduce((acc, rule) => {
     acc += `${rule.typeName} ${(rule.conditionText || rule.name || rule.keyText)}`;
     return acc;
   }, "");
 
   return `${atRules}${selector}${ruleIndex}`;
 }
 
+/**
+ * Get a human-friendly style source path to display in the Changes panel.
+ * For element inline styles, return a string indicating that.
+ * For inline stylesheets, return a string indicating that plus the stylesheet's index.
+ * For URLs, return just the stylesheet filename.
+ *
+ * @param {Object} source
+ *        Information about the style source. Contains:
+ *        - type: {String} One of "element" or "stylesheet"
+ *        - href: {String|null} Stylesheet URL or document URL for elmeent inline styles
+ *        - index: {Number} Position of the stylesheet in its document's stylesheet list.
+ * @return {String}
+ */
+function getSourceForDisplay(source) {
+  let href;
+
+  switch (source.type) {
+    case "element":
+      href = getStr("changes.elementStyleLabel");
+      break;
+    case "stylesheet":
+      if (source.href) {
+        const url = new URL(source.href);
+        href = url.pathname.substring(url.pathname.lastIndexOf("/") + 1);
+      } else {
+        href = getFormatStr("changes.inlineStyleSheetLabel", `#${source.index}`);
+      }
+      break;
+  }
+
+  return href;
+}
+
+module.exports.getSourceForDisplay = getSourceForDisplay;
 module.exports.getSourceHash = getSourceHash;
 module.exports.getRuleHash = getRuleHash;
--- a/devtools/client/inspector/changes/utils/l10n.js
+++ b/devtools/client/inspector/changes/utils/l10n.js
@@ -4,9 +4,10 @@
 
 "use strict";
 
 const { LocalizationHelper } = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper("devtools/client/locales/changes.properties");
 
 module.exports = {
   getStr: (...args) => L10N.getStr(...args),
+  getFormatStr: (...args) => L10N.getFormatStr(...args),
 };
--- a/devtools/client/locales/en-US/changes.properties
+++ b/devtools/client/locales/en-US/changes.properties
@@ -6,8 +6,17 @@
 # the Inspector sidebar.
 
 # LOCALIZATION NOTE (changes.noChanges): This text is shown when no changes are available.
 changes.noChanges=No changes found.
 
 # LOCALIZATION NOTE (changes.noChangesDescription): This text is shown when no changes are
 # available and provides additional context for the purpose of the Changes panel.
 changes.noChangesDescription=Changes to CSS in Inspector will appear here.
+
+# LOCALIZATION NOTE (changes.inlineStyleSheetLabel): This label appears in the Changes
+# panel above changes done to inline stylesheets. The variable will be replaced with the
+# index of the stylesheet within its document like so: Inline #1
+changes.inlineStyleSheetLabel=Inline %S
+
+# LOCALIZATION NOTE (changes.elementStyleLabel): This label appears in the Changes
+# panel above changes done to element styles.
+changes.elementStyleLabel=Element
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -14,29 +14,26 @@
 #sidebar-panel-changes {
   margin: 0;
   padding: 0;
   width: 100%;
   height: 100%;
   overflow: auto;
 }
 
-#sidebar-panel-changes .source[open] {
-  padding-bottom: 10px;
-}
-
 #sidebar-panel-changes .href {
-  background: var(--theme-sidebar-background);
+  color: var(--theme-toolbar-color);
+  background: var(--theme-toolbar-background);
   border-top: 1px solid var(--theme-splitter-color);
   border-bottom: 1px solid var(--theme-splitter-color);
-  padding: 5px;
+  padding: 4px;
+  font-size: 12px;
   white-space: nowrap;
   text-overflow: ellipsis;
   overflow: hidden;
-  cursor: pointer;
 }
 
 #sidebar-panel-changes .rule .level {
   padding-top: 3px;
   padding-right: 5px;
   padding-bottom: 3px;
   padding-left: var(--diff-level-offset);
   position: relative;