Update keyboard shortcut controller (#1258)

* Change attribute

* Update hidden style

* Hide scope checkboxes when they are not supported for that action

3 files changed, 102 insertions, 31 deletions

diff --git a/ext/bg/css/settings2.css b/ext/bg/css/settings2.css
index e89634f3..7f7a6763 100644
--- a/ext/bg/css/settings2.css
+++ b/ext/bg/css/settings2.css
@@ -2007,11 +2007,13 @@ input.sentence-termination-character-input2 {
     margin: 0 0.5em 0 1em;
 }
 .hotkey-scope-checkbox-container {
-    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;
 }
diff --git a/ext/bg/js/settings2/keyboard-shortcuts-controller.js b/ext/bg/js/settings2/keyboard-shortcuts-controller.js
index 42b36e86..43dd738e 100644
--- a/ext/bg/js/settings2/keyboard-shortcuts-controller.js
+++ b/ext/bg/js/settings2/keyboard-shortcuts-controller.js
@@ -164,6 +164,9 @@ class KeyboardShortcutHotkeyEntry {
         this._os = os;
         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;
     }
@@ -175,24 +178,29 @@ class KeyboardShortcutHotkeyEntry {
         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');
 
+        this._actionSelect = action;
+        this._scopeCheckboxes = scopeCheckboxes;
+        this._scopeCheckboxContainers = scopeCheckboxContainers;
+
         this._inputField = new KeyboardMouseInputField(input, null, this._os);
         this._inputField.prepare(this._data.key, this._data.modifiers, false, true);
 
         action.value = this._data.action;
-        action.dataset.setting = `${this._basePath}.action`;
 
         enabledToggle.checked = this._data.enabled;
         enabledToggle.dataset.setting = `${this._basePath}.enabled`;
 
-        const scopes = this._data.scopes;
+        this._updateCheckboxVisibility();
+        this._updateCheckboxStates();
+
         for (const scopeCheckbox of scopeCheckboxes) {
-            scopeCheckbox.checked = scopes.includes(scopeCheckbox.dataset.type);
             this._eventListeners.addEventListener(scopeCheckbox, 'change', this._onScopeCheckboxChange.bind(this), false);
         }
-
         this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+        this._eventListeners.addEventListener(this._actionSelect, 'change', this._onActionSelectChange.bind(this), false);
         this._eventListeners.on(this._inputField, 'change', this._onInputFieldChange.bind(this));
     }
 
@@ -227,9 +235,14 @@ class KeyboardShortcutHotkeyEntry {
 
     _onScopeCheckboxChange(e) {
         const node = e.currentTarget;
-        const {type} = node.dataset;
-        if (typeof type !== 'string') { return; }
-        this._setScopeEnabled(type, node.checked);
+        const {scope} = node.dataset;
+        if (typeof scope !== 'string') { return; }
+        this._setScopeEnabled(scope, node.checked);
+    }
+
+    _onActionSelectChange(e) {
+        const value = e.currentTarget.value;
+        this._setAction(value);
     }
 
     async _delete() {
@@ -294,4 +307,60 @@ class KeyboardShortcutHotkeyEntry {
         }
         return null;
     }
+
+    async _setAction(value) {
+        const targets = [{
+            action: 'set',
+            path: `${this._basePath}.action`,
+            value
+        }];
+
+        this._data.action = value;
+
+        const scopes = this._data.scopes;
+        const validScopes = this._getValidScopesForAction(value);
+        if (validScopes !== null) {
+            let changed = false;
+            for (let i = 0, ii = scopes.length; i < ii; ++i) {
+                if (!validScopes.has(scopes[i])) {
+                    scopes.splice(i, 1);
+                    --i;
+                    --ii;
+                    changed = true;
+                }
+            }
+            if (changed) {
+                targets.push({
+                    action: 'set',
+                    path: `${this._basePath}.scopes`,
+                    value: scopes
+                });
+                this._updateCheckboxStates();
+            }
+        }
+
+        await this._modifyProfileSettings(targets);
+
+        this._updateCheckboxVisibility();
+    }
+
+    _updateCheckboxStates() {
+        const scopes = this._data.scopes;
+        for (const scopeCheckbox of this._scopeCheckboxes) {
+            scopeCheckbox.checked = scopes.includes(scopeCheckbox.dataset.scope);
+        }
+    }
+
+    _updateCheckboxVisibility() {
+        const validScopes = this._getValidScopesForAction(this._data.action);
+        for (const node of this._scopeCheckboxContainers) {
+            node.hidden = !(validScopes === null || validScopes.has(node.dataset.scope));
+        }
+    }
+
+    _getValidScopesForAction(action) {
+        const optionNode = this._actionSelect.querySelector(`option[value="${action}"]`);
+        const scopesString = (optionNode !== null ? optionNode.dataset.scopes : void 0);
+        return (typeof scopesString === 'string' ? new Set(scopesString.split(' ')) : null);
+    }
 }
diff --git a/ext/bg/settings2.html b/ext/bg/settings2.html
index 738120bb..0fd3feb0 100644
--- a/ext/bg/settings2.html
+++ b/ext/bg/settings2.html
@@ -2995,35 +2995,35 @@
     <div class="hotkey-list-item-action-label-cell">Action:</div>
     <div class="hotkey-list-item-action-cell">
         <select class="hotkey-list-item-action">
-            <option value="">None</option>
-            <option value="close">Close</option>
-            <option value="focusSearchBox">Focus search box</option>
-            <option value="nextEntry">Go to next entry</option>
-            <option value="nextEntry3">Go to next entry (x3)</option>
-            <option value="previousEntry">Go to previous entry</option>
-            <option value="previousEntry3">Go to previous entry (x3)</option>
-            <option value="lastEntry">Go to last entry</option>
-            <option value="firstEntry">Go to first entry</option>
-            <option value="nextEntryDifferentDictionary">Go to next dictionary</option>
-            <option value="previousEntryDifferentDictionary">Go to previous dictionary</option>
-            <option value="historyBackward">Navigate backward in history</option>
-            <option value="historyForward">Navigate forward in history</option>
-            <option value="addNoteKanji">Add kanji note</option>
-            <option value="addNoteTermKanji">Add term note</option>
-            <option value="addNoteTermKana">Add term note (reading)</option>
-            <option value="viewNote">View note</option>
-            <option value="playAudio">Play audio</option>
-            <option value="copyHostSelection">Copy selection</option>
+            <option value="" data-scopes="popup search">None</option>
+            <option value="close" data-scopes="popup search">Close</option>
+            <option value="focusSearchBox" data-scopes="search">Focus search box</option>
+            <option value="nextEntry" data-scopes="popup search">Go to next entry</option>
+            <option value="nextEntry3" data-scopes="popup search">Go to next entry (x3)</option>
+            <option value="previousEntry" data-scopes="popup search">Go to previous entry</option>
+            <option value="previousEntry3" data-scopes="popup search">Go to previous entry (x3)</option>
+            <option value="lastEntry" data-scopes="popup search">Go to last entry</option>
+            <option value="firstEntry" data-scopes="popup search">Go to first entry</option>
+            <option value="nextEntryDifferentDictionary" data-scopes="popup search">Go to next dictionary</option>
+            <option value="previousEntryDifferentDictionary" data-scopes="popup search">Go to previous dictionary</option>
+            <option value="historyBackward" data-scopes="popup search">Navigate backward in history</option>
+            <option value="historyForward" data-scopes="popup search">Navigate forward in history</option>
+            <option value="addNoteKanji" data-scopes="popup search">Add kanji note</option>
+            <option value="addNoteTermKanji" data-scopes="popup search">Add term note</option>
+            <option value="addNoteTermKana" data-scopes="popup search">Add term note (reading)</option>
+            <option value="viewNote" data-scopes="popup search">View note</option>
+            <option value="playAudio" data-scopes="popup search">Play audio</option>
+            <option value="copyHostSelection" data-scopes="popup search">Copy selection</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">
-                    <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-type="popup"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></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">
-                    <label class="checkbox"><input type="checkbox" class="hotkey-scope-checkbox" data-type="search"><span class="checkbox-body"><span class="checkbox-fill"></span><span class="checkbox-border"></span><span class="checkbox-check"></span></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>