From ccf28ed055f08d38d28b19025d10cb4e7424464b Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Tue, 19 Jan 2021 20:52:57 -0500 Subject: [PATCH] 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 --- ext/bg/js/settings/anki-controller.js | 6 +- .../js/settings/anki-templates-controller.js | 6 +- ext/bg/js/settings/audio-controller.js | 7 +- ext/bg/js/settings/dictionary-controller.js | 17 ++-- ext/bg/js/settings/popup-menu.js | 96 ++++++++++--------- ext/bg/js/settings/profile-conditions-ui.js | 13 +-- ext/bg/js/settings/profile-controller.js | 19 ++-- ext/bg/js/settings/scan-inputs-controller.js | 15 +-- .../keyboard-shortcuts-controller.js | 7 +- ...tence-termination-characters-controller.js | 7 +- .../settings2/settings-display-controller.js | 14 +-- ...ranslation-text-replacements-controller.js | 15 +-- 12 files changed, 109 insertions(+), 113 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; } - - this._eventListeners.removeAllEventListeners(); - if (this._container.parentNode !== null) { - this._container.parentNode.removeChild(this._container); - } - - this.trigger('closed', { - popupMenu: this, - container: this._container, - menu: this._menu, + 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._containerNode.parentNode !== null) { + this._containerNode.parentNode.removeChild(this._containerNode); + } + + 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;