💬 Add support for accelerator to button and menuitems

dothq · Feb 4, 2024 · faf7c1e · faf7c1e
1 parent 934130b
commit faf7c1e
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 59 deletions.
diff --git a/components/commands/Command.sys.mjs b/components/commands/Command.sys.mjs
@@ -107,6 +107,7 @@ export class Command extends ActionsMessenger {
 		this._inert = this.createEmptyMap();
 		this._mode = this.createEmptyMap();
 		this._checked = this.createEmptyMap();
+		this._accelerator = this.createEmptyMap();
 
 		this._setupEventListeners();
 	}
@@ -195,12 +196,15 @@ export class Command extends ActionsMessenger {
 	/** @type {Map<string, boolean>} */
 	_inert = null;
 
-	/** @type {Map<string, boolean>} */
+	/** @type {Map<string, string>} */
 	_mode = null;
 
 	/** @type {Map<string, boolean>} */
 	_checked = null;
 
+	/** @type {Map<string, string>} */
+	_accelerator = null;
+
 	/**
 	 * Updates an attribute on the command
 	 *
@@ -359,6 +363,20 @@ export class Command extends ActionsMessenger {
 		this.setAttribute("checked", newValue);
 	}
 
+	/**
+	 * The accelerator text for this command
+	 */
+	get accelerator() {
+		return this._accelerator.get(CommandAudiences.DEFAULT);
+	}
+
+	/**
+	 * Updates the accelerator text of this command
+	 */
+	set accelerator(newValue) {
+		this.setAttribute("accelerator", newValue);
+	}
+
 	/**
 	 * The context for this command
 	 */

diff --git a/components/commands/content/reload-tab.js b/components/commands/content/reload-tab.js
@@ -14,9 +14,6 @@ export class ReloadTabCommand extends TabCommand {
 	constructor(subscription, subscriber, area) {
 		super(subscription, subscriber, area);
 
-		this.label = "Reload";
-		this.icon = "reload";
-
 		this._maybeAlreadyLoading();
 	}
 
@@ -33,6 +30,7 @@ export class ReloadTabCommand extends TabCommand {
 				: "Reload this page"
 		};
 		this.icon = this.isLoading ? "close" : "reload";
+		this.accelerator = this.isLoading ? "Escape" : "Accel+R";
 	}
 
 	/**
@@ -61,7 +59,10 @@ export class ReloadTabCommand extends TabCommand {
 	 * Checks if our browser was loading as the command was initialised
 	 */
 	_maybeAlreadyLoading() {
-		if (this.isLoading !== undefined) return;
+		if (this.isLoading !== undefined) {
+			this._update();
+			return;
+		}
 
 		const { webProgress } = this.context.browser;
 

diff --git a/components/commands/widgets/browser-command-button.js b/components/commands/widgets/browser-command-button.js
@@ -27,6 +27,7 @@ class BrowserCommandButton extends BrowserCommandElementMixin(BrowserButton) {
 			case "inert":
 			case "mode":
 			case "checked":
+			case "accelerator":
 				this[attributeName] = value;
 				break;
 			default:

diff --git a/components/commands/widgets/browser-command-element.js b/components/commands/widgets/browser-command-element.js
@@ -24,6 +24,18 @@ var BrowserCommandElementMixin = (Base) => {
 			};
 		}
 
+		/**
+		 * The tooltip element for this command element
+		 */
+		get tooltipEl() {
+			return /** @type {BrowserTooltip} */ (
+				this.querySelector("tooltip") ||
+					document.createXULElement("tooltip", {
+						is: "browser-tooltip"
+					})
+			);
+		}
+
 		static get observedAttributes() {
 			// prettier-ignore
 			return (/** @type {any} */ (Base).observedAttributes || []).concat(["commandid"]);
@@ -51,6 +63,29 @@ var BrowserCommandElementMixin = (Base) => {
 			}
 		}
 
+		/**
+		 * Computes the tooltip text for this command element
+		 */
+		getTooltipText() {
+			let tooltipText = "";
+
+			const label = this.getAttribute("label");
+			const labelAuxiliary = this.getAttribute("labelauxiliary");
+			const accelerator = this.getAttribute("accelerator");
+
+			if (labelAuxiliary) {
+				tooltipText = labelAuxiliary;
+			} else if (label) {
+				tooltipText = label;
+			}
+
+			if (accelerator) {
+				tooltipText += ` (${accelerator})`;
+			}
+
+			return tooltipText;
+		}
+
 		/**
 		 * Performs the command attached to the element
 		 * @param {Event} [originalEvent]
@@ -163,6 +198,18 @@ var BrowserCommandElementMixin = (Base) => {
 			this.dispatchEvent(evt);
 		}
 
+		/**
+		 * Fired when a popup starts showing in the command element
+		 * @param {Event} event
+		 */
+		_onCommandElementPopupShowing(event) {
+			if (event.target === this.tooltipEl) {
+				if (this.tooltipEl.state == "closed") return;
+
+				this.tooltipEl.label = this.getTooltipText();
+			}
+		}
+
 		connectedCallback() {
 			if (super.connectedCallback) {
 				super.connectedCallback();
@@ -171,6 +218,14 @@ var BrowserCommandElementMixin = (Base) => {
 			if (this.commandId) {
 				this._initCommandSubscription();
 			}
+
+			this.setAttribute("tooltip", "_child");
+			this.prepend(this.tooltipEl);
+
+			this.addEventListener(
+				"popupshowing",
+				this._onCommandElementPopupShowing.bind(this)
+			);
 		}
 
 		attributeChangedCallback(attribute, oldValue, newValue) {
@@ -197,6 +252,11 @@ var BrowserCommandElementMixin = (Base) => {
 			}
 
 			this._destroyCommandSubscription();
+
+			this.removeEventListener(
+				"popupshowing",
+				this._onCommandElementPopupShowing.bind(this)
+			);
 		}
 	};
 

diff --git a/components/menus/content/xul-menu-base.js b/components/menus/content/xul-menu-base.js
@@ -109,6 +109,9 @@ var MozMenuItemBaseMixin = (Base) => {
 				case "icon":
 					this.image = ThemeIcons.getURI(value);
 					break;
+				case "accelerator":
+					this.acceltext = value;
+					break;
 				default:
 					this.setAttribute(attributeName, value);
 					break;
@@ -173,8 +176,6 @@ var MozMenuItemBaseMixin = (Base) => {
 		}
 
 		set label(newValue) {
-			if (this.label == newValue) return;
-
 			this.elements.label.textContent = newValue;
 			this.setAttribute("label", newValue);
 		}
@@ -187,8 +188,6 @@ var MozMenuItemBaseMixin = (Base) => {
 		}
 
 		set labelAuxiliary(newValue) {
-			if (this.labelAuxiliary == newValue) return;
-
 			this.setAttribute("labelauxiliary", newValue);
 		}
 
@@ -200,8 +199,6 @@ var MozMenuItemBaseMixin = (Base) => {
 		}
 
 		set image(newValue) {
-			if (this.image == newValue) return;
-
 			this.elements.imageLeft.src = newValue;
 			this.setAttribute("image", newValue);
 		}
@@ -239,6 +236,7 @@ var MozMenuItemBaseMixin = (Base) => {
 
 		set acceltext(newValue) {
 			this.setAttribute("acceltext", newValue);
+			this.setAttribute("accelerator", newValue);
 
 			this.elements.accelerator.textContent = newValue;
 		}

diff --git a/components/menus/content/xul-menugroup.js b/components/menus/content/xul-menugroup.js
@@ -49,8 +49,7 @@
 			if (this.groupTooltip.state == "closed") return;
 
 			if (this.activeItem) {
-				this.groupTooltip.label =
-					this.activeItem.labelAuxiliary || this.activeItem.label;
+				this.groupTooltip.label = this.activeItem.getTooltipText();
 			}
 		}
 

diff --git a/components/widgets/content/browser-button.js b/components/widgets/content/browser-button.js
@@ -29,7 +29,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 	 * @typedef {Object} BrowserButtonElements
 	 * @property {HTMLSpanElement} label - The buttons's label
 	 * @property {BrowserIcon} icon - The button's icon
-	 * @property {BrowserTooltip} tooltip - The button's tooltip
 	 *
 	 * @returns {BrowserButtonElements}
 	 */
@@ -49,12 +48,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 						class: "browser-button-icon",
 						name: this.getAttribute("icon") || ""
 					})
-			),
-			tooltip: /** @type {BrowserTooltip} */ (
-				this.querySelector("tooltip") ||
-					document.createXULElement("tooltip", {
-						is: "browser-tooltip"
-					})
 			)
 		};
 	}
@@ -107,8 +100,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 
 		this.setAttribute("label", newLabel);
 		this.elements.label.textContent = newLabel;
-
-		this._updateTooltipText();
 	}
 
 	/**
@@ -125,8 +116,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 		if (newLabelAuxiliary == this.labelAuxiliary) return;
 
 		this.setAttribute("labelauxiliary", newLabelAuxiliary);
-
-		this._updateTooltipText();
 	}
 
 	/**
@@ -162,6 +151,20 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 		this.toggleAttribute("checked", newChecked);
 	}
 
+	/**
+	 * The accelerator text of the browser button
+	 */
+	get accelerator() {
+		return this.getAttribute("accelerator");
+	}
+
+	/**
+	 * Updates the checked/toggled state of the browser button
+	 */
+	set accelerator(newAccelerator) {
+		this.setAttribute("accelerator", newAccelerator);
+	}
+
 	/**
 	 * Toggles a transitioning psuedo class on the button
 	 * @param {string} name
@@ -209,33 +212,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 		);
 	}
 
-	/**
-	 * Updates the button's tooltip text
-	 */
-	_updateTooltipText() {
-		if (!this.elements.tooltip.visible) return;
-
-		let tooltipText = "";
-
-		if (this.labelAuxiliary) {
-			tooltipText = this.labelAuxiliary;
-		} else if (this.label) {
-			tooltipText = this.label;
-		}
-
-		this.elements.tooltip.label = tooltipText;
-	}
-
-	/**
-	 * Fired when a popup starts showing in the button
-	 * @param {Event} event
-	 */
-	_onPopupShowing(event) {
-		if (event.target === this.elements.tooltip) {
-			this._updateTooltipText();
-		}
-	}
-
 	connectedCallback() {
 		this.classList.add("browser-button");
 		this.classList.toggle(
@@ -252,9 +228,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 			)
 		);
 
-		this.setAttribute("tooltip", "_child");
-		this.appendChild(this.elements.tooltip);
-
 		this.addEventListener(
 			"mouseleave",
 			this._handleBrowserButtonEvent.bind(this)
@@ -263,7 +236,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 			"focusout",
 			this._handleBrowserButtonEvent.bind(this)
 		);
-		this.addEventListener("popupshowing", this._onPopupShowing.bind(this));
 
 		this.resizeObserver.observe(this);
 	}
@@ -277,10 +249,6 @@ class BrowserButton extends BrowserContextualMixin(HTMLButtonElement) {
 			"focusout",
 			this._handleBrowserButtonEvent.bind(this)
 		);
-		this.removeEventListener(
-			"popupshowing",
-			this._onPopupShowing.bind(this)
-		);
 
 		this.resizeObserver.disconnect();
 	}