Menu refactor (#1277)

* Rename menuOpened event to menuOpen

* Rename menuClosed event to menuClose

* Rename close event

* Assign _isClosed

* Reuse event detail

* Expose PopupMenu.openMenus

* Rename and expose properties

* Add cancelable argument to close

* Update menuOpen detail

* Update menuClose detail

12 files changed, 108 insertions, 112 deletions

diff --git a/ext/bg/js/settings/anki-controller.js b/ext/bg/js/settings/anki-controller.js
index 49bdea60..b816a47b 100644
--- a/ext/bg/js/settings/anki-controller.js
+++ b/ext/bg/js/settings/anki-controller.js
@@ -445,10 +445,10 @@ class AnkiCardController {
         this._validateField(node, index);
     }
 
-    _onFieldMenuClosed({currentTarget: node, detail: {action, item}}) {
+    _onFieldMenuClose({currentTarget: button, detail: {action, item}}) {
         switch (action) {
             case 'setFieldMarker':
-                this._setFieldMarker(node, item.dataset.marker);
+                this._setFieldMarker(button, item.dataset.marker);
                 break;
         }
     }
@@ -547,7 +547,7 @@ class AnkiCardController {
                 } else {
                     delete menuButton.dataset.menu;
                 }
-                this._fieldEventListeners.addEventListener(menuButton, 'menuClosed', this._onFieldMenuClosed.bind(this), false);
+                this._fieldEventListeners.addEventListener(menuButton, 'menuClose', this._onFieldMenuClose.bind(this), false);
             }
 
             totalFragment.appendChild(content);
diff --git a/ext/bg/js/settings/anki-templates-controller.js b/ext/bg/js/settings/anki-templates-controller.js
index 6bd1bad7..125d8e16 100644
--- a/ext/bg/js/settings/anki-templates-controller.js
+++ b/ext/bg/js/settings/anki-templates-controller.js
@@ -69,7 +69,7 @@ class AnkiTemplatesController {
         resetButton.addEventListener('click', this._onReset.bind(this), false);
         resetConfirmButton.addEventListener('click', this._onResetConfirm.bind(this), false);
         if (menuButton !== null) {
-            menuButton.addEventListener('menuClosed', this._onFieldMenuClosed.bind(this), false);
+            menuButton.addEventListener('menuClose', this._onFieldMenuClose.bind(this), false);
         }
 
         this._settingsController.on('optionsChanged', this._onOptionsChanged.bind(this));
@@ -138,10 +138,10 @@ class AnkiTemplatesController {
         this._validate(infoNode, field, 'term-kanji', true, false);
     }
 
-    _onFieldMenuClosed({currentTarget: node, detail: {action, item}}) {
+    _onFieldMenuClose({currentTarget: button, detail: {action, item}}) {
         switch (action) {
             case 'setFieldMarker':
-                this._setFieldMarker(node, item.dataset.marker);
+                this._setFieldMarker(button, item.dataset.marker);
                 break;
         }
     }
diff --git a/ext/bg/js/settings/audio-controller.js b/ext/bg/js/settings/audio-controller.js
index 7c9e77f6..71f908d0 100644
--- a/ext/bg/js/settings/audio-controller.js
+++ b/ext/bg/js/settings/audio-controller.js
@@ -168,7 +168,7 @@ class AudioController {
             eventListeners.addEventListener(removeButton, 'click', this._onAudioSourceRemoveClicked.bind(this, entry), false);
         }
         if (menuButton !== null) {
-            eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this, entry), false);
+            eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this, entry), false);
         }
 
         this._audioSourceContainer.appendChild(container);
@@ -224,9 +224,8 @@ class AudioController {
         this._removeAudioSourceEntry(entry);
     }
 
-    _onMenuClosed(entry, e) {
-        const {detail: {action}} = e;
-        switch (action) {
+    _onMenuClose(entry, e) {
+        switch (e.detail.action) {
             case 'remove':
                 this._removeAudioSourceEntry(entry);
                 break;
diff --git a/ext/bg/js/settings/dictionary-controller.js b/ext/bg/js/settings/dictionary-controller.js
index e32e4d6f..941b2039 100644
--- a/ext/bg/js/settings/dictionary-controller.js
+++ b/ext/bg/js/settings/dictionary-controller.js
@@ -86,8 +86,8 @@ class DictionaryEntry {
             this._eventListeners.addEventListener(deleteButton, 'click', this._onDeleteButtonClicked.bind(this), false);
         }
         if (menuButton !== null) {
-            this._eventListeners.addEventListener(menuButton, 'menuOpened', this._onMenuOpened.bind(this), false);
-            this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+            this._eventListeners.addEventListener(menuButton, 'menuOpen', this._onMenuOpen.bind(this), false);
+            this._eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this), false);
         }
         if (detailsToggleLink !== null && this._detailsContainer !== null) {
             this._eventListeners.addEventListener(detailsToggleLink, 'click', this._onDetailsToggleLinkClicked.bind(this), false);
@@ -116,10 +116,10 @@ class DictionaryEntry {
         this._delete();
     }
 
-    _onMenuOpened(e) {
-        const {detail: {menu}} = e;
-        const showDetails = menu.querySelector('.popup-menu-item[data-menu-action="showDetails"]');
-        const hideDetails = menu.querySelector('.popup-menu-item[data-menu-action="hideDetails"]');
+    _onMenuOpen(e) {
+        const node = e.detail.menu.node;
+        const showDetails = node.querySelector('.popup-menu-item[data-menu-action="showDetails"]');
+        const hideDetails = node.querySelector('.popup-menu-item[data-menu-action="hideDetails"]');
         const hasDetails = (this._detailsContainer !== null);
         const detailsVisible = (hasDetails && !this._detailsContainer.hidden);
         if (showDetails !== null) {
@@ -132,9 +132,8 @@ class DictionaryEntry {
         }
     }
 
-    _onMenuClosed(e) {
-        const {detail: {action}} = e;
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'delete':
                 this._delete();
                 break;
diff --git a/ext/bg/js/settings/popup-menu.js b/ext/bg/js/settings/popup-menu.js
index 63c257ad..5ae7435c 100644
--- a/ext/bg/js/settings/popup-menu.js
+++ b/ext/bg/js/settings/popup-menu.js
@@ -16,62 +16,72 @@
  */
 
 class PopupMenu extends EventDispatcher {
-    constructor(sourceElement, container) {
+    constructor(sourceElement, containerNode) {
         super();
         this._sourceElement = sourceElement;
-        this._container = container;
-        this._menu = container.querySelector('.popup-menu');
+        this._containerNode = containerNode;
+        this._node = containerNode.querySelector('.popup-menu');
         this._isClosed = false;
         this._eventListeners = new EventListenerCollection();
     }
 
+    get sourceElement() {
+        return this._sourceElement;
+    }
+
+    get containerNode() {
+        return this._containerNode;
+    }
+
+    get node() {
+        return this._node;
+    }
+
     get isClosed() {
         return this._isClosed;
     }
 
     prepare() {
-        const items = this._menu.querySelectorAll('.popup-menu-item');
+        const items = this._node.querySelectorAll('.popup-menu-item');
         this._setPosition(items);
-        this._container.focus();
+        this._containerNode.focus();
 
         this._eventListeners.addEventListener(window, 'resize', this._onWindowResize.bind(this), false);
-        this._eventListeners.addEventListener(this._container, 'click', this._onMenuContainerClick.bind(this), false);
+        this._eventListeners.addEventListener(this._containerNode, 'click', this._onMenuContainerClick.bind(this), false);
 
         const onMenuItemClick = this._onMenuItemClick.bind(this);
         for (const item of items) {
             this._eventListeners.addEventListener(item, 'click', onMenuItemClick, false);
         }
 
-        this._sourceElement.dispatchEvent(new CustomEvent('menuOpened', {
+        PopupMenu.openMenus.add(this);
+
+        this._sourceElement.dispatchEvent(new CustomEvent('menuOpen', {
             bubbles: false,
             cancelable: false,
-            detail: {
-                popupMenu: this,
-                container: this._container,
-                menu: this._menu
-            }
+            detail: {menu: this}
         }));
     }
 
-    close() {
-        return this._close(null, 'close');
+    close(cancelable=true) {
+        return this._close(null, 'close', cancelable);
     }
 
     // Private
 
     _onMenuContainerClick(e) {
         if (e.currentTarget !== e.target) { return; }
-        this._close(null, 'outside');
+        this._close(null, 'outside', true);
     }
 
     _onMenuItemClick(e) {
         const item = e.currentTarget;
         if (item.disabled) { return; }
-        this._close(item, 'item');
+        this._close(item, 'item', true);
     }
 
     _onWindowResize() {
-        this._close(null, 'resize');
+        this._close(null, 'resize', true);
     }
 
     _setPosition(items) {
@@ -122,8 +132,8 @@ class PopupMenu extends EventDispatcher {
         }
 
         // Position
-        const menu = this._menu;
-        const fullRect = this._container.getBoundingClientRect();
+        const menu = this._node;
+        const fullRect = this._containerNode.getBoundingClientRect();
         const sourceRect = this._sourceElement.getBoundingClientRect();
         const menuRect = menu.getBoundingClientRect();
         let top = menuRect.top;
@@ -154,37 +164,35 @@ class PopupMenu extends EventDispatcher {
         menu.style.top = `${y}px`;
     }
 
-    _close(item, cause) {
+    _close(item, cause, cancelable) {
         if (this._isClosed) { return true; }
         const action = (item !== null ? item.dataset.menuAction : null);
 
-        const result = this._sourceElement.dispatchEvent(new CustomEvent('menuClosed', {
-            bubbles: false,
-            cancelable: true,
-            detail: {
-                popupMenu: this,
-                container: this._container,
-                menu: this._menu,
-                item,
-                action,
-                cause
-            }
-        }));
-        if (!result) { return false; }
+        const detail = {
+            menu: this,
+            item,
+            action,
+            cause
+        };
+        const result = this._sourceElement.dispatchEvent(new CustomEvent('menuClose', {bubbles: false, cancelable, detail}));
+        if (cancelable && !result) { return false; }
+
+        PopupMenu.openMenus.delete(this);
 
+        this._isClosed = true;
         this._eventListeners.removeAllEventListeners();
-        if (this._container.parentNode !== null) {
-            this._container.parentNode.removeChild(this._container);
+        if (this._containerNode.parentNode !== null) {
+            this._containerNode.parentNode.removeChild(this._containerNode);
         }
 
-        this.trigger('closed', {
-            popupMenu: this,
-            container: this._container,
-            menu: this._menu,
-            item,
-            action,
-            cause
-        });
+        this.trigger('close', detail);
         return true;
     }
 }
+
+Object.defineProperty(PopupMenu, 'openMenus', {
+    configurable: false,
+    enumerable: true,
+    writable: false,
+    value: new Set()
+});
diff --git a/ext/bg/js/settings/profile-conditions-ui.js b/ext/bg/js/settings/profile-conditions-ui.js
index 9cedd1ac..1ab46b04 100644
--- a/ext/bg/js/settings/profile-conditions-ui.js
+++ b/ext/bg/js/settings/profile-conditions-ui.js
@@ -478,8 +478,8 @@ class ProfileConditionUI {
         this._eventListeners.addEventListener(this._operatorInput, 'change', this._onOperatorChange.bind(this), false);
         if (this._removeButton !== null) { this._eventListeners.addEventListener(this._removeButton, 'click', this._onRemoveButtonClick.bind(this), false); }
         if (this._menuButton !== null) {
-            this._eventListeners.addEventListener(this._menuButton, 'menuOpened', this._onMenuOpened.bind(this), false);
-            this._eventListeners.addEventListener(this._menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+            this._eventListeners.addEventListener(this._menuButton, 'menuOpen', this._onMenuOpen.bind(this), false);
+            this._eventListeners.addEventListener(this._menuButton, 'menuClose', this._onMenuClose.bind(this), false);
         }
     }
 
@@ -545,15 +545,16 @@ class ProfileConditionUI {
         this._removeSelf();
     }
 
-    _onMenuOpened({detail: {menu}}) {
-        const deleteGroup = menu.querySelector('.popup-menu-item[data-menu-action="deleteGroup"]');
+    _onMenuOpen(e) {
+        const node = e.detail.menu.node;
+        const deleteGroup = node.querySelector('.popup-menu-item[data-menu-action="deleteGroup"]');
         if (deleteGroup !== null) {
             deleteGroup.hidden = (this._parent.childCount <= 1);
         }
     }
 
-    _onMenuClosed({detail: {action}}) {
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'delete':
                 this._removeSelf();
                 break;
diff --git a/ext/bg/js/settings/profile-controller.js b/ext/bg/js/settings/profile-controller.js
index f40818c7..924e965e 100644
--- a/ext/bg/js/settings/profile-controller.js
+++ b/ext/bg/js/settings/profile-controller.js
@@ -612,8 +612,8 @@ class ProfileEntry {
         this._eventListeners.addEventListener(this._isDefaultRadio, 'change', this._onIsDefaultRadioChange.bind(this), false);
         this._eventListeners.addEventListener(this._nameInput, 'input', this._onNameInputInput.bind(this), false);
         this._eventListeners.addEventListener(this._countLink, 'click', this._onConditionsCountLinkClick.bind(this), false);
-        this._eventListeners.addEventListener(this._menuButton, 'menuOpened', this._onMenuOpened.bind(this), false);
-        this._eventListeners.addEventListener(this._menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+        this._eventListeners.addEventListener(this._menuButton, 'menuOpen', this._onMenuOpen.bind(this), false);
+        this._eventListeners.addEventListener(this._menuButton, 'menuClose', this._onMenuClose.bind(this), false);
     }
 
     cleanup() {
@@ -658,16 +658,17 @@ class ProfileEntry {
         this._profileController.openProfileConditionsModal(this._index);
     }
 
-    _onMenuOpened({detail: {menu}}) {
+    _onMenuOpen(e) {
+        const node = e.detail.menu.node;
         const count = this._profileController.profileCount;
-        this._setMenuActionEnabled(menu, 'moveUp', this._index > 0);
-        this._setMenuActionEnabled(menu, 'moveDown', this._index < count - 1);
-        this._setMenuActionEnabled(menu, 'copyFrom', count > 1);
-        this._setMenuActionEnabled(menu, 'delete', count > 1);
+        this._setMenuActionEnabled(node, 'moveUp', this._index > 0);
+        this._setMenuActionEnabled(node, 'moveDown', this._index < count - 1);
+        this._setMenuActionEnabled(node, 'copyFrom', count > 1);
+        this._setMenuActionEnabled(node, 'delete', count > 1);
     }
 
-    _onMenuClosed({detail: {action}}) {
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'moveUp':
                 this._profileController.moveProfile(this._index, -1);
                 break;
diff --git a/ext/bg/js/settings/scan-inputs-controller.js b/ext/bg/js/settings/scan-inputs-controller.js
index b8b3f710..a810a2fb 100644
--- a/ext/bg/js/settings/scan-inputs-controller.js
+++ b/ext/bg/js/settings/scan-inputs-controller.js
@@ -208,8 +208,8 @@ class ScanInputField {
             this._eventListeners.addEventListener(removeButton, 'click', this._onRemoveClick.bind(this));
         }
         if (menuButton !== null) {
-            this._eventListeners.addEventListener(menuButton, 'menuOpened', this._onMenuOpened.bind(this));
-            this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this));
+            this._eventListeners.addEventListener(menuButton, 'menuOpen', this._onMenuOpen.bind(this));
+            this._eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this));
         }
 
         this._updateDataSettingTargets();
@@ -245,9 +245,10 @@ class ScanInputField {
         this._removeSelf();
     }
 
-    _onMenuOpened({detail: {menu}}) {
-        const showAdvanced = menu.querySelector('.popup-menu-item[data-menu-action="showAdvanced"]');
-        const hideAdvanced = menu.querySelector('.popup-menu-item[data-menu-action="hideAdvanced"]');
+    _onMenuOpen(e) {
+        const node = e.detail.menu.node;
+        const showAdvanced = node.querySelector('.popup-menu-item[data-menu-action="showAdvanced"]');
+        const hideAdvanced = node.querySelector('.popup-menu-item[data-menu-action="hideAdvanced"]');
         const advancedVisible = (this._node.dataset.showAdvanced === 'true');
         if (showAdvanced !== null) {
             showAdvanced.hidden = advancedVisible;
@@ -257,8 +258,8 @@ class ScanInputField {
         }
     }
 
-    _onMenuClosed({detail: {action}}) {
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'remove':
                 this._removeSelf();
                 break;
diff --git a/ext/bg/js/settings2/keyboard-shortcuts-controller.js b/ext/bg/js/settings2/keyboard-shortcuts-controller.js
index ea6bd81c..30846a62 100644
--- a/ext/bg/js/settings2/keyboard-shortcuts-controller.js
+++ b/ext/bg/js/settings2/keyboard-shortcuts-controller.js
@@ -199,7 +199,7 @@ class KeyboardShortcutHotkeyEntry {
         for (const scopeCheckbox of scopeCheckboxes) {
             this._eventListeners.addEventListener(scopeCheckbox, 'change', this._onScopeCheckboxChange.bind(this), false);
         }
-        this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+        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));
     }
@@ -214,9 +214,8 @@ class KeyboardShortcutHotkeyEntry {
 
     // Private
 
-    _onMenuClosed(e) {
-        const {detail: {action}} = e;
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'delete':
                 this._delete();
                 break;
diff --git a/ext/bg/js/settings2/sentence-termination-characters-controller.js b/ext/bg/js/settings2/sentence-termination-characters-controller.js
index 1e055f40..d62771ec 100644
--- a/ext/bg/js/settings2/sentence-termination-characters-controller.js
+++ b/ext/bg/js/settings2/sentence-termination-characters-controller.js
@@ -182,7 +182,7 @@ class SentenceTerminationCharacterEntry {
         this._eventListeners.addEventListener(typeSelect, 'change', this._onTypeSelectChange.bind(this), false);
         this._eventListeners.addEventListener(character1Input, 'change', this._onCharacterChange.bind(this, 1), false);
         this._eventListeners.addEventListener(character2Input, 'change', this._onCharacterChange.bind(this, 2), false);
-        this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+        this._eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this), false);
     }
 
     cleanup() {
@@ -208,9 +208,8 @@ class SentenceTerminationCharacterEntry {
         this._setCharacterValue(node, characterNumber, value);
     }
 
-    _onMenuClosed(e) {
-        const {detail: {action}} = e;
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'delete':
                 this._delete();
                 break;
diff --git a/ext/bg/js/settings2/settings-display-controller.js b/ext/bg/js/settings2/settings-display-controller.js
index c5661b13..6f0a8276 100644
--- a/ext/bg/js/settings2/settings-display-controller.js
+++ b/ext/bg/js/settings2/settings-display-controller.js
@@ -26,7 +26,6 @@ class SettingsDisplayController {
         this._modalController = modalController;
         this._contentNode = null;
         this._menuContainer = null;
-        this._openPopupMenus = new Set();
         this._onMoreToggleClickBind = null;
         this._onMenuButtonClickBind = null;
     }
@@ -198,11 +197,6 @@ class SettingsDisplayController {
         return false;
     }
 
-    _onClosePopupMenu({popupMenu, onClose}) {
-        this._openPopupMenus.delete(popupMenu);
-        popupMenu.off('closed', onClose);
-    }
-
     _onInputTabActionKeyDown(e) {
         if (e.key !== 'Tab' || e.ctrlKey) { return; }
 
@@ -248,7 +242,7 @@ class SettingsDisplayController {
     }
 
     _closeTopMenuOrModal() {
-        for (const popupMenu of this._openPopupMenus) {
+        for (const popupMenu of PopupMenu.openMenus) {
             popupMenu.close();
             return;
         }
@@ -266,12 +260,6 @@ class SettingsDisplayController {
         this._menuContainer.appendChild(menu);
 
         const popupMenu = new PopupMenu(element, menu);
-        this._openPopupMenus.add(popupMenu);
-
-        const data = {popupMenu, onClose: null};
-        data.onClose = this._onClosePopupMenu.bind(this, data);
-
-        popupMenu.on('closed', data.onClose);
         popupMenu.prepare();
     }
 
diff --git a/ext/bg/js/settings2/translation-text-replacements-controller.js b/ext/bg/js/settings2/translation-text-replacements-controller.js
index 41ee8e3f..864b279e 100644
--- a/ext/bg/js/settings2/translation-text-replacements-controller.js
+++ b/ext/bg/js/settings2/translation-text-replacements-controller.js
@@ -158,8 +158,8 @@ class TranslationTextReplacementsEntry {
         replacementInput.dataset.setting = `${pathBase}.replacement`;
         ignoreCaseToggle.dataset.setting = `${pathBase}.ignoreCase`;
 
-        this._eventListeners.addEventListener(menuButton, 'menuOpened', this._onMenuOpened.bind(this), false);
-        this._eventListeners.addEventListener(menuButton, 'menuClosed', this._onMenuClosed.bind(this), false);
+        this._eventListeners.addEventListener(menuButton, 'menuOpen', this._onMenuOpen.bind(this), false);
+        this._eventListeners.addEventListener(menuButton, 'menuClose', this._onMenuClose.bind(this), false);
         this._eventListeners.addEventListener(patternInput, 'settingChanged', this._onPatternChanged.bind(this), false);
         this._eventListeners.addEventListener(ignoreCaseToggle, 'settingChanged', this._updateTestInput.bind(this), false);
         this._eventListeners.addEventListener(replacementInput, 'settingChanged', this._updateTestInput.bind(this), false);
@@ -175,14 +175,15 @@ class TranslationTextReplacementsEntry {
 
     // Private
 
-    _onMenuOpened({detail: {menu}}) {
+    _onMenuOpen(e) {
+        const node = e.detail.menu.node;
         const testVisible = this._isTestVisible();
-        menu.querySelector('[data-menu-action=showTest]').hidden = testVisible;
-        menu.querySelector('[data-menu-action=hideTest]').hidden = !testVisible;
+        node.querySelector('[data-menu-action=showTest]').hidden = testVisible;
+        node.querySelector('[data-menu-action=hideTest]').hidden = !testVisible;
     }
 
-    _onMenuClosed({detail: {action}}) {
-        switch (action) {
+    _onMenuClose(e) {
+        switch (e.detail.action) {
             case 'remove':
                 this._parent.deleteGroup(this._index);
                 break;