Update hotkey settings design (#1564)

* Update PopupMenu event prevention

* Use vars for button padding

* Add button-inner-label style

* Add input-button button

* Update display of scope selection

* Add hidden argument text input field

* Remove unnecessary calls

* Display a strike through the enabled button when no scopes are selected

5 files changed, 190 insertions, 59 deletions

diff --git a/ext/css/material.css b/ext/css/material.css
index a5fcee29..efa5a730 100644
--- a/ext/css/material.css
+++ b/ext/css/material.css
@@ -798,10 +798,13 @@ button,
     --button-current-background-color: var(--button-background-color);
     --button-current-shadow: var(--button-shadow);
 
+    --button-padding-vertical: 0.5em;
+    --button-padding-horizontal: 1em;
+
     border-width: var(--thin-border-size);
     border-style: solid;
     border-radius: 0.3em;
-    padding: 0.5em 1em;
+    padding: var(--button-padding-vertical) var(--button-padding-horizontal);
     font-weight: bold;
     font-size: inherit;
     font-family: inherit;
@@ -940,6 +943,35 @@ button.low-emphasis.danger {
     --button-disabled-background-color: var(--danger-color-transparent0);
 }
 
+/* Input button */
+button.input-button {
+    --button-content-color: var(--button-default-icon-color);
+    --button-border-color: var(--input-background-color);
+    --button-background-color: var(--input-background-color);
+    --button-hover-background-color: var(--input-background-color-dark);
+    --button-active-background-color: var(--input-background-color-darker);
+
+    --button-padding-vertical: 0;
+    --button-padding-horizontal: 0.5em;
+
+    text-align: left;
+    font-weight: normal;
+    border-style: none;
+    border-width: 0;
+    width: var(--input-width-large);
+    height: var(--input-height);
+    line-height: var(--input-height);
+    box-sizing: border-box;
+    position: relative;
+}
+button.input-button.input-with-suffix-button {
+    flex: 1 1 auto;
+    border-top-right-radius: 0;
+    border-bottom-right-radius: 0;
+    border-right-style: none;
+    z-index: 1;
+}
+
 /* Input suffix button */
 button.input-suffix-button {
     --button-content-color: var(--button-default-icon-color);
@@ -985,6 +1017,16 @@ input[type=number][data-invalid=true]+button.input-suffix-button {
 }
 
 
+/* Button inner label */
+.button-inner-label {
+    margin-top: calc(-1 * var(--button-padding-vertical));
+    margin-bottom: calc(-1 * var(--button-padding-vertical));
+    margin-left: calc(-1 * var(--button-padding-horizontal));
+    margin-right: calc(-1 * var(--button-padding-horizontal));
+    padding: var(--button-padding-vertical) var(--button-padding-horizontal);
+}
+
+
 /* Material design icon button */
 button.icon-button {
     --button-content-color: var(--button-default-icon-color);
@@ -1140,7 +1182,9 @@ button.popup-menu-item {
     --button-disabled-background-color: transparent;
     --button-disabled-shadow: none;
 
-    padding: 0.625em 1.5em;
+    --button-padding-vertical: 0.625em;
+    --button-padding-horizontal: 1.5em;
+
     flex: 1 1 auto;
     border-radius: 0;
     border-style: none;
@@ -1174,7 +1218,9 @@ button.popup-menu-item:not([hidden]) {
 }
 :root[data-page-type=popup] .popup-menu.popup-menu-auto-size button.popup-menu-item,
 .popup-menu.popup-menu-small button.popup-menu-item {
-    padding: 0.5em 0.75em;
+    --button-padding-vertical: 0.5em;
+    --button-padding-horizontal: 0.75em;
+
     font-size: var(--font-size-small);
 }
 .popup-menu-item-group {
diff --git a/ext/css/settings.css b/ext/css/settings.css
index cd90b7d3..6bc06bf7 100644
--- a/ext/css/settings.css
+++ b/ext/css/settings.css
@@ -2047,9 +2047,34 @@ input.sentence-termination-character-input2 {
 .hotkey-list-item-action {
     flex: 1 1 auto;
 }
-.hotkey-list-item-enabled-label {
-    align-self: center;
-    margin-left: 1em;
+.hotkey-list-item-enabled-button-container {
+    display: flex;
+    flex-flow: row nowrap;
+    align-items: stretch;
+    margin-left: 0.375em;
+}
+button.hotkey-list-item-enabled-button {
+    display: flex;
+    flex-flow: row nowrap;
+    align-items: stretch;
+    width: auto;
+}
+button.hotkey-list-item-enabled-button[data-scope-count='0'] {
+    text-decoration: line-through;
+}
+.hotkey-list-item-enabled-button-label {
+    flex: 1 1 auto;
+    display: flex;
+    flex-flow: row nowrap;
+    align-items: center;
+}
+.hotkey-list-item-enabled-button-label>.checkbox {
+    margin-right: 0.5em;
+}
+.hotkey-list-item-action-argument {
+    margin-left: 0.375em;
+    flex: 1 1 auto;
+    visibility: hidden;
 }
 .hotkey-list-item-flex-row {
     display: flex;
@@ -2059,19 +2084,15 @@ input.sentence-termination-character-input2 {
 .hotkey-list-item-flex-row-label {
     margin: 0 0.5em 0 1em;
 }
-.hotkey-scope-checkbox-container {
+
+.hotkey-scope-popup-menu-item-label {
+    flex: 1 1 auto;
+    display: flex;
     flex-flow: row nowrap;
     align-items: center;
-    cursor: pointer;
-}
-.hotkey-scope-checkbox-container:not([hidden]) {
-    display: flex;
 }
-.hotkey-scope-checkbox-container:not(:last-child) {
-    margin-right: 0.75em;
-}
-.hotkey-scope-checkbox-container>span {
-    padding-left: 0.375em;
+.hotkey-scope-popup-menu-item-label>.checkbox {
+    margin-right: 0.5em;
 }
 
 .inline-icon {
diff --git a/ext/js/dom/popup-menu.js b/ext/js/dom/popup-menu.js
index af076baa..d8c75869 100644
--- a/ext/js/dom/popup-menu.js
+++ b/ext/js/dom/popup-menu.js
@@ -86,17 +86,19 @@ class PopupMenu extends EventDispatcher {
 
     _onMenuContainerClick(e) {
         if (e.currentTarget !== e.target) { return; }
-        e.stopPropagation();
-        e.preventDefault();
-        this._close(null, 'outside', true, e);
+        if (this._close(null, 'outside', true, e)) {
+            e.stopPropagation();
+            e.preventDefault();
+        }
     }
 
     _onMenuItemClick(e) {
         const item = e.currentTarget;
         if (item.disabled) { return; }
-        e.stopPropagation();
-        e.preventDefault();
-        this._close(item, 'item', true, e);
+        if (this._close(item, 'item', true, e)) {
+            e.stopPropagation();
+            e.preventDefault();
+        }
     }
 
     _onWindowResize() {
diff --git a/ext/js/pages/settings/keyboard-shortcuts-controller.js b/ext/js/pages/settings/keyboard-shortcuts-controller.js
index 99b16f06..514928d7 100644
--- a/ext/js/pages/settings/keyboard-shortcuts-controller.js
+++ b/ext/js/pages/settings/keyboard-shortcuts-controller.js
@@ -164,10 +164,11 @@ class KeyboardShortcutHotkeyEntry {
         this._eventListeners = new EventListenerCollection();
         this._inputField = null;
         this._actionSelect = null;
-        this._scopeCheckboxes = null;
-        this._scopeCheckboxContainers = null;
         this._basePath = `inputs.hotkeys[${this._index}]`;
         this._stringComparer = stringComparer;
+        this._enabledButton = null;
+        this._scopeMenu = null;
+        this._scopeMenuEventListeners = new EventListenerCollection();
     }
 
     prepare() {
@@ -176,13 +177,12 @@ class KeyboardShortcutHotkeyEntry {
         const menuButton = node.querySelector('.hotkey-list-item-button');
         const input = node.querySelector('.hotkey-list-item-input');
         const action = node.querySelector('.hotkey-list-item-action');
-        const scopeCheckboxes = node.querySelectorAll('.hotkey-scope-checkbox');
-        const scopeCheckboxContainers = node.querySelectorAll('.hotkey-scope-checkbox-container');
         const enabledToggle = node.querySelector('.hotkey-list-item-enabled');
+        const scopesButton = node.querySelector('.hotkey-list-item-scopes-button');
+        const enabledButton = node.querySelector('.hotkey-list-item-enabled-button');
 
         this._actionSelect = action;
-        this._scopeCheckboxes = scopeCheckboxes;
-        this._scopeCheckboxContainers = scopeCheckboxContainers;
+        this._enabledButton = enabledButton;
 
         this._inputField = new KeyboardMouseInputField(input, null, this._os);
         this._inputField.prepare(this._data.key, this._data.modifiers, false, true);
@@ -192,12 +192,10 @@ class KeyboardShortcutHotkeyEntry {
         enabledToggle.checked = this._data.enabled;
         enabledToggle.dataset.setting = `${this._basePath}.enabled`;
 
-        this._updateCheckboxVisibility();
-        this._updateCheckboxStates();
+        this._updateScopesButton();
 
-        for (const scopeCheckbox of scopeCheckboxes) {
-            this._eventListeners.addEventListener(scopeCheckbox, 'change', this._onScopeCheckboxChange.bind(this), false);
-        }
+        this._eventListeners.addEventListener(scopesButton, 'menuOpen', this._onScopesMenuOpen.bind(this));
+        this._eventListeners.addEventListener(scopesButton, 'menuClose', this._onScopesMenuClose.bind(this));
         this._eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this), false);
         this._eventListeners.addEventListener(this._actionSelect, 'change', this._onActionSelectChange.bind(this), false);
         this._eventListeners.on(this._inputField, 'change', this._onInputFieldChange.bind(this));
@@ -206,6 +204,7 @@ class KeyboardShortcutHotkeyEntry {
     cleanup() {
         this._eventListeners.removeAllEventListeners();
         this._inputField.cleanup();
+        this._clearScopeMenu();
         if (this._node.parentNode !== null) {
             this._node.parentNode.removeChild(this._node);
         }
@@ -227,6 +226,24 @@ class KeyboardShortcutHotkeyEntry {
         }
     }
 
+    _onScopesMenuOpen(e) {
+        const {menu} = e.detail;
+        this._scopeMenu = menu;
+        this._updateScopeMenuItems(menu);
+        this._updateDisplay(menu.containerNode); // Fix a animation issue due to changing checkbox values
+    }
+
+    _onScopesMenuClose(e) {
+        const {menu, action} = e.detail;
+        if (action === 'toggleScope') {
+            e.preventDefault();
+            return;
+        }
+        if (this._scopeMenu === menu) {
+            this._clearScopeMenu();
+        }
+    }
+
     _onInputFieldChange({key, modifiers}) {
         this._setKeyAndModifiers(key, modifiers);
     }
@@ -277,6 +294,8 @@ class KeyboardShortcutHotkeyEntry {
             scopes.splice(index, 1);
         }
 
+        this._updateScopesButton();
+
         await this._modifyProfileSettings([{
             action: 'set',
             path: `${this._basePath}.scopes`,
@@ -346,17 +365,13 @@ class KeyboardShortcutHotkeyEntry {
     }
 
     _updateCheckboxStates() {
-        const scopes = this._data.scopes;
-        for (const scopeCheckbox of this._scopeCheckboxes) {
-            scopeCheckbox.checked = scopes.includes(scopeCheckbox.dataset.scope);
-        }
+        if (this._scopeMenu === null) { return; }
+        this._updateScopeMenuItems(this._scopeMenu);
     }
 
     _updateCheckboxVisibility() {
-        const validScopes = this._getValidScopesForAction(this._data.action);
-        for (const node of this._scopeCheckboxContainers) {
-            node.hidden = !(validScopes === null || validScopes.has(node.dataset.scope));
-        }
+        if (this._scopeMenu === null) { return; }
+        this._updateScopeMenuItems(this._scopeMenu);
     }
 
     _getValidScopesForAction(action) {
@@ -364,4 +379,44 @@ class KeyboardShortcutHotkeyEntry {
         const scopesString = (optionNode !== null ? optionNode.dataset.scopes : void 0);
         return (typeof scopesString === 'string' ? new Set(scopesString.split(' ')) : null);
     }
+
+    _updateScopeMenuItems(menu) {
+        this._scopeMenuEventListeners.removeAllEventListeners();
+
+        const scopes = this._data.scopes;
+        const validScopes = this._getValidScopesForAction(this._data.action);
+
+        const bodyNode = menu.bodyNode;
+        const menuItems = bodyNode.querySelectorAll('.popup-menu-item');
+        for (const menuItem of menuItems) {
+            if (menuItem.dataset.menuAction !== 'toggleScope') { continue; }
+
+            const {scope} = menuItem.dataset;
+            menuItem.hidden = !(validScopes === null || validScopes.has(scope));
+
+            const checkbox = menuItem.querySelector('.hotkey-scope-checkbox');
+            if (checkbox !== null) {
+                checkbox.checked = scopes.includes(scope);
+                this._scopeMenuEventListeners.addEventListener(checkbox, 'change', this._onScopeCheckboxChange.bind(this), false);
+            }
+        }
+    }
+
+    _clearScopeMenu() {
+        this._scopeMenuEventListeners.removeAllEventListeners();
+        this._scopeMenu = null;
+    }
+
+    _updateScopesButton() {
+        const {scopes} = this._data;
+        this._enabledButton.dataset.scopeCount = `${scopes.length}`;
+    }
+
+    _updateDisplay(node) {
+        const {style} = node;
+        const {display} = style;
+        style.display = 'none';
+        getComputedStyle(node).getPropertyValue('display');
+        style.display = display;
+    }
 }
diff --git a/ext/settings.html b/ext/settings.html
index 2f8c012c..b99cf0c3 100644
--- a/ext/settings.html
+++ b/ext/settings.html
@@ -3128,7 +3128,15 @@
     <div class="hotkey-list-item-input-label-cell">Input:</div>
     <div class="hotkey-list-item-input-cell">
         <input type="text" class="hotkey-list-item-input" autocomplete="off" spellcheck="false" placeholder="No input" data-property="include">
-        <label class="toggle hotkey-list-item-enabled-label"><input type="checkbox" class="hotkey-list-item-enabled"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+        <div class="hotkey-list-item-enabled-button-container">
+            <button class="hotkey-list-item-enabled-button input-button input-with-suffix-button">
+                <label class="hotkey-list-item-enabled-button-label button-inner-label">
+                    <span class="checkbox"><input type="checkbox" class="hotkey-list-item-enabled"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></span>
+                    <span>Enabled</span>
+                </label>
+            </button>
+            <button class="hotkey-list-item-scopes-button input-suffix-button input-suffix-icon-button light-icon" data-menu="hotkey-list-item-scopes-menu" data-menu-position="below left"><span class="icon" data-icon="material-down-arrow"></span></button>
+        </div>
     </div>
 
     <div class="hotkey-list-item-action-label-cell">Action:</div>
@@ -3155,23 +3163,7 @@
             <option value="copyHostSelection" data-scopes="popup">Copy host window selection</option>
             <option value="scanSelectedText" data-scopes="web">Scan selected text</option>
         </select>
-        <div class="hotkey-list-item-flex-row">
-            <div class="hotkey-list-item-flex-row-label">Scopes:</div>
-            <div class="hotkey-list-item-flex-row">
-                <label class="hotkey-scope-checkbox-container" data-scope="web">
-                    <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="web"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
-                    <span>Web</span>
-                </label>
-                <label class="hotkey-scope-checkbox-container" data-scope="popup">
-                    <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="popup"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
-                    <span>Popup</span>
-                </label>
-                <label class="hotkey-scope-checkbox-container" data-scope="search">
-                    <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="search"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
-                    <span>Search</span>
-                </label>
-            </div>
-        </div>
+        <input type="text" class="hotkey-list-item-action-argument">
     </div>
 </div></div></template>
 
@@ -3193,6 +3185,21 @@
     <button class="popup-menu-item" data-menu-action="delete">Delete</button>
 </div></div></div></template>
 
+<template id="hotkey-list-item-scopes-menu-template"><div class="popup-menu-container" tabindex="-1" role="dialog"><div class="popup-menu"><div class="popup-menu-body">
+    <button class="popup-menu-item hotkey-scope-popup-menu-item" data-menu-action="toggleScope" data-scope="web"><label class="hotkey-scope-popup-menu-item-label button-inner-label">
+        <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="web"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
+        <span>Web</span>
+    </label></button>
+    <button class="popup-menu-item hotkey-scope-popup-menu-item" data-menu-action="toggleScope" data-scope="popup"><label class="hotkey-scope-popup-menu-item-label button-inner-label">
+        <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="popup"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
+        <span>Popup</span>
+    </label></button>
+    <button class="popup-menu-item hotkey-scope-popup-menu-item" data-menu-action="toggleScope" data-scope="search"><label class="hotkey-scope-popup-menu-item-label button-inner-label">
+        <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-scope="search"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></span></label>
+        <span>Search</span>
+    </label></button>
+</div></div></div></template>
+
 <template id="extension-hotkey-list-item-menu-template"><div class="popup-menu-container" tabindex="-1" role="dialog"><div class="popup-menu"><div class="popup-menu-body">
     <button class="popup-menu-item" data-menu-action="clearInput">Clear input</button>
     <button class="popup-menu-item" data-menu-action="resetInput">Reset input</button>