Bug 1533158 - Replace the orange highlight color with yellow when highlighting CSS properties. r=pbro
authorMicah Tigley <mtigley@mozilla.com>
Mon, 11 Mar 2019 15:09:04 +0000
changeset 524409 a52a29b099bb46766007651c4418435ae68aa963
parent 524408 3338bffc95e30eb488aa3da2789bd50ee80a0260
child 524410 bac8eff6ce0dc26f3f806489539c835eaa80e308
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1533158
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 1533158 - Replace the orange highlight color with yellow when highlighting CSS properties. r=pbro Differential Revision: https://phabricator.services.mozilla.com/D22418
devtools/client/inspector/markup/utils.js
devtools/client/inspector/markup/views/markup-container.js
devtools/client/inspector/rules/rules.js
devtools/client/themes/rules.css
--- a/devtools/client/inspector/markup/utils.js
+++ b/devtools/client/inspector/markup/utils.js
@@ -5,58 +5,70 @@
 "use strict";
 
 /**
  * Apply a 'flashed' background and foreground color to elements. Intended
  * to be used with flashElementOff as a way of drawing attention to an element.
  *
  * @param  {Node} backgroundElt
  *         The element to set the highlighted background color on.
- * @param  {Node} foregroundElt
- *         The element to set the matching foreground color on.
- *         Optional.  This will equal backgroundElt if not set.
+ * @param  {Object} options
+ * @param  {Node} options.foregroundElt
+ *         The element to set the matching foreground color on. This will equal
+ *         backgroundElt if not set.
+ * @param  {String} options.backgroundClass
+ *         The background highlight color class to set on the element.
  */
-function flashElementOn(backgroundElt, foregroundElt = backgroundElt) {
+function flashElementOn(backgroundElt, {
+  foregroundElt = backgroundElt,
+  backgroundClass = "theme-bg-contrast",
+} = {}) {
   if (!backgroundElt || !foregroundElt) {
     return;
   }
 
   // Make sure the animation class is not here
   backgroundElt.classList.remove("flash-out");
 
   // Change the background
-  backgroundElt.classList.add("theme-bg-contrast");
+  backgroundElt.classList.add(backgroundClass);
 
   foregroundElt.classList.add("theme-fg-contrast");
   [].forEach.call(
     foregroundElt.querySelectorAll("[class*=theme-fg-color]"),
     span => span.classList.add("theme-fg-contrast")
   );
 }
 
 /**
  * Remove a 'flashed' background and foreground color to elements.
  * See flashElementOn.
  *
  * @param  {Node} backgroundElt
  *         The element to remove the highlighted background color on.
- * @param  {Node} foregroundElt
- *         The element to remove the matching foreground color on.
- *         Optional.  This will equal backgroundElt if not set.
+ * @param  {Object} options
+ * @param  {Node} options.foregroundElt
+ *         The element to remove the matching foreground color on. This will equal
+ *         backgroundElt if not set.
+ * @param  {String} options.backgroundClass
+ *         The background highlight color class to remove on the element.
  */
-function flashElementOff(backgroundElt, foregroundElt = backgroundElt) {
+function flashElementOff(backgroundElt, {
+  foregroundElt = backgroundElt,
+  backgroundClass = "theme-bg-contrast",
+} = {}) {
   if (!backgroundElt || !foregroundElt) {
     return;
   }
 
   // Add the animation class to smoothly remove the background
   backgroundElt.classList.add("flash-out");
 
   // Remove the background
-  backgroundElt.classList.remove("theme-bg-contrast");
+  backgroundElt.classList.remove(backgroundClass);
 
   foregroundElt.classList.remove("theme-fg-contrast");
   [].forEach.call(
     foregroundElt.querySelectorAll("[class*=theme-fg-color]"),
     span => span.classList.remove("theme-fg-contrast")
   );
 }
 
--- a/devtools/client/inspector/markup/views/markup-container.js
+++ b/devtools/client/inspector/markup/views/markup-container.js
@@ -649,23 +649,23 @@ MarkupContainer.prototype = {
   },
 
   /**
    * Temporarily flash the container to attract attention.
    * Used for markup mutations.
    */
   flashMutation: function() {
     if (!this.selected) {
-      flashElementOn(this.tagState, this.editor.elt);
+      flashElementOn(this.tagState, { foregroundElt: this.editor.elt });
       if (this._flashMutationTimer) {
         clearTimeout(this._flashMutationTimer);
         this._flashMutationTimer = null;
       }
       this._flashMutationTimer = setTimeout(() => {
-        flashElementOff(this.tagState, this.editor.elt);
+        flashElementOff(this.tagState, { foregroundElt: this.editor.elt });
       }, this.markup.CONTAINER_FLASHING_DURATION);
     }
   },
 
   _hovered: false,
 
   /**
    * Highlight the currently hovered tag + its closing tag if necessary
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -38,17 +38,18 @@ loader.lazyRequireGetter(this, "StyleIns
 loader.lazyRequireGetter(this, "AutocompletePopup", "devtools/client/shared/autocomplete-popup");
 loader.lazyRequireGetter(this, "KeyShortcuts", "devtools/client/shared/key-shortcuts");
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 const PREF_UA_STYLES = "devtools.inspector.showUserAgentStyles";
 const PREF_DEFAULT_COLOR_UNIT = "devtools.defaultColorUnit";
 const FILTER_CHANGED_TIMEOUT = 150;
-const REMOVE_FLASH_ELEMENT_DURATION = 500;
+// Removes the flash-out class from an element after 1 second.
+const REMOVE_FLASH_OUT_CLASS_TIMER = 1000;
 
 // This is used to parse user input when filtering.
 const FILTER_PROP_RE = /\s*([^:\s]*)\s*:\s*(.*?)\s*;?$/;
 // This is used to parse the filter search value to see if the filter
 // should be strict or not
 const FILTER_STRICT_RE = /\s*`(.*?)`\s*$/;
 const INSET_POINT_TYPES = ["top", "right", "bottom", "left"];
 
@@ -1519,31 +1520,33 @@ CssRuleView.prototype = {
 
   /**
    * Temporarily flash the given element.
    *
    * @param  {Element} element
    *         The element.
    */
   _flashElement(element) {
-    flashElementOn(element);
-    flashElementOff(element);
+    flashElementOn(element, {
+      backgroundClass: "ruleview-property-highlight-background-color" });
+    flashElementOff(element, {
+      backgroundClass: "ruleview-property-highlight-background-color" });
 
     if (this._removeFlashOutTimer) {
       clearTimeout(this._removeFlashOutTimer);
       this._removeFlashOutTimer = null;
     }
 
     // Remove the flash-out class to prevent the element from re-flashing when the view
     // is resized.
     this._removeFlashOutTimer = setTimeout(() => {
       element.classList.remove("flash-out");
       // Emit "scrolled-to-property" for use by tests.
       this.emit("scrolled-to-element");
-    }, REMOVE_FLASH_ELEMENT_DURATION);
+    }, REMOVE_FLASH_OUT_CLASS_TIMER);
   },
 
   /**
    * Scrolls to the top of either the rule or declaration. The view will try to scroll to
    * the rule if both can fit in the viewport. If not, then scroll to the declaration.
    *
    * @param  {Element} rule
    *         The rule to scroll to.
--- a/devtools/client/themes/rules.css
+++ b/devtools/client/themes/rules.css
@@ -3,23 +3,29 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* CSS Variables specific to this panel that aren't defined by the themes */
 :root {
   --rule-highlight-background-color: var(--theme-highlight-yellow);
   --rule-overridden-item-border-color: var(--theme-content-color3);
   --rule-header-background-color: var(--theme-toolbar-background);
   --rule-pseudo-class-text-color: var(--yellow-70) ;
+  /* This should be --yellow-50 but since we need an opacity of 0.4, we hard-code the
+  resulting color here for now. */
+  --rule-property-highlight-background-color: #FFF697;
 }
 
 :root.theme-dark {
   --rule-highlight-background-color: #521C76;
   --rule-overridden-item-border-color: var(--theme-content-color1);
   --rule-header-background-color: #222225;
   --rule-pseudo-class-text-color: var(--yellow-50);
+  /* This should be --yellow-50 but since we need an opacity of 0.3, we hard-code the
+  resulting color here for now. */
+  --rule-property-highlight-background-color: #605913;
 }
 
 /* Rule View Tabpanel */
 
 #sidebar-panel-ruleview {
   margin: 0;
   display: flex;
   flex-direction: column;
@@ -143,16 +149,20 @@
 .ruleview-code {
   direction: ltr;
 }
 
 .ruleview-property:not(:hover) > .ruleview-enableproperty {
   pointer-events: none;
 }
 
+.ruleview-property-highlight-background-color {
+  background-color: var(--rule-property-highlight-background-color);
+}
+
 .ruleview-expandable-container {
   display: block;
 }
 
 .ruleview-namecontainer {
   cursor: text;
   margin-left: -35px;
 }
@@ -644,11 +654,11 @@
 }
 
 .flash-out {
   animation: flash-out 1s ease-out;
 }
 
 @keyframes flash-out {
   from {
-    background: var(--theme-contrast-background);
+    background: var(--rule-property-highlight-background-color);
   }
 }