Bug 1553448 - Improve appearance of card menu at about:addons r=mstriemer,jaws
authorRob Wu <rob@robwu.nl>
Tue, 28 May 2019 18:16:47 +0000
changeset 475952 596487d6126ac07f27c9aa61d768e5d402ad4245
parent 475951 1978cee5e9631a66984bc3421261c0a5c0b03f4b
child 475953 79f67b95b9b26a2c377c92636bddd6aea9da25a9
push id86561
push userrob@robwu.nl
push dateTue, 28 May 2019 20:53:45 +0000
treeherderautoland@596487d6126a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstriemer, jaws
bugs1553448
milestone69.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 1553448 - Improve appearance of card menu at about:addons r=mstriemer,jaws - Render "hidden" appearance via opacity. This fixes the reported regression, and also results in a desired fading of the icon (if any). - Set cursor to "default" avoid "pointer" cursor in menu items. - Let the "checked" icon adapt the color of the style sheet (e.g. to account for dark themes) instead of using the default black color. - Prevent clicks on non-button elements in the menu from expanding the card, e.g. when the user clicks on the triangle or separator. - Render menu item icon at the right in RTL locales. Differential Revision: https://phabricator.services.mozilla.com/D32145
toolkit/mozapps/extensions/content/aboutaddons.js
toolkit/mozapps/extensions/content/panel-item.css
toolkit/mozapps/extensions/content/panel-list.css
--- a/toolkit/mozapps/extensions/content/aboutaddons.js
+++ b/toolkit/mozapps/extensions/content/aboutaddons.js
@@ -344,21 +344,17 @@ class PanelList extends HTMLElement {
     }
   }
 
   get open() {
     return this.hasAttribute("open");
   }
 
   set open(val) {
-    if (val) {
-      this.setAttribute("open", "true");
-    } else {
-      this.removeAttribute("open");
-    }
+    this.toggleAttribute("open", val);
   }
 
   show(triggeringEvent) {
     this.triggeringEvent = triggeringEvent;
     this.open = true;
   }
 
   hide(triggeringEvent) {
@@ -454,16 +450,20 @@ class PanelList extends HTMLElement {
       case "resize":
       case "scroll":
       case "blur":
         this.hide();
         break;
       case "click":
         if (e.target.tagName == "PANEL-ITEM") {
           this.hide();
+        } else {
+          // Avoid falling through to the default click handler of the
+          // add-on card, which would expand the add-on card.
+          e.stopPropagation();
         }
         break;
       case "mousedown":
       case "focusin":
         // There will be a focusin after the mousedown that opens the panel
         // using the mouse. Ignore the first focusin event if it's on the
         // triggering target.
         if (this.triggeringEvent &&
@@ -505,33 +505,25 @@ class PanelItem extends HTMLElement {
     this.button = this.shadowRoot.querySelector("button");
   }
 
   get disabled() {
     return this.button.hasAttribute("disabled");
   }
 
   set disabled(val) {
-    if (val) {
-      this.button.setAttribute("disabled", "");
-    } else {
-      this.button.removeAttribute("disabled");
-    }
+    this.button.toggleAttribute("disabled", val);
   }
 
   get checked() {
     return this.hasAttribute("checked");
   }
 
   set checked(val) {
-    if (val) {
-      this.setAttribute("checked", "");
-    } else {
-      this.removeAttribute("checked");
-    }
+    this.toggleAttribute("checked", val);
   }
 }
 customElements.define("panel-item", PanelItem);
 
 class AddonOptions extends HTMLElement {
   connectedCallback() {
     if (this.children.length == 0) {
       this.render();
@@ -1159,18 +1151,17 @@ class AddonCard extends HTMLElement {
           }
           break;
         case "report":
           this.panel.hide();
           openAbuseReport({addonId: addon.id, reportEntryPoint: "menu"});
           break;
         default:
           // Handle a click on the card itself.
-          // Don't expand if expanded or a button was clicked.
-          if (!this.expanded && e.target.localName != "button") {
+          if (!this.expanded) {
             loadViewFn("detail", this.addon.id);
           }
           break;
       }
     } else if (e.type == "change") {
       let {name} = e.target;
       if (name == "autoupdate") {
         addon.applyBackgroundUpdates = e.target.value;
--- a/toolkit/mozapps/extensions/content/panel-item.css
+++ b/toolkit/mozapps/extensions/content/panel-item.css
@@ -1,8 +1,14 @@
+:host([checked]) {
+  --icon: url("chrome://global/skin/icons/check.svg");
+  -moz-context-properties: fill;
+  fill: currentColor;
+}
+
 button {
   background-color: transparent;
   color: inherit;
   background-image: var(--icon);
   background-position: 16px center;
   background-repeat: no-repeat;
   background-size: 16px;
   border: none;
@@ -10,32 +16,36 @@ button {
   display: block;
   font-size: inherit;
   padding: 4px 40px;
   padding-inline-end: 12px;
   text-align: start;
   width: 100%;
 }
 
+button:dir(rtl) {
+  background-position: right 16px center;
+}
+
 :host([badged]) button::after {
   content: "";
   display: block;
   width: 5px;
   height: 5px;
   border-radius: 50%;
   background: var(--blue-50);
   position: absolute;
   top: 4px;
   left: 28px;
 }
 
-:host([checked]) {
-  --icon: url("chrome://global/skin/icons/check.svg");
-}
-
 button:focus,
-button:not([disabled]):hover {
+button:enabled:hover {
   background-color: var(--in-content-button-background);
 }
 
-button:not([disabled]):hover:active {
+button:enabled:hover:active {
   background-color: var(--in-content-button-background-hover);
 }
+
+button:disabled {
+  opacity: 0.4;
+}
--- a/toolkit/mozapps/extensions/content/panel-list.css
+++ b/toolkit/mozapps/extensions/content/panel-list.css
@@ -12,16 +12,17 @@
   border: 1px solid var(--in-content-box-border-color);
   border-radius: var(--panel-border-radius);
   padding: 6px 0;
   margin-bottom: 16px;
   box-shadow: var(--shadow-30);
   min-width: 12em;
   z-index: 1;
   white-space: nowrap;
+  cursor: default;
 }
 
 :host([valign="top"]) {
   bottom: 30px;
 }
 
 :host([valign="bottom"]) {
   top: 30px;