diff --git a/src/js/controls.js b/src/js/controls.js index acc9d011..6265f2aa 100644 --- a/src/js/controls.js +++ b/src/js/controls.js @@ -381,7 +381,7 @@ const controls = { // We have to bind to keyup otherwise Firefox triggers a click when a keydown event handler shifts focus // https://bugzilla.mozilla.org/show_bug.cgi?id=1220143 bindMenuItemShortcuts(menuItem, type) { - // Handle space or -> to open menu + // Navigate through menus via arrow keys and space on( menuItem, 'keydown keyup', @@ -429,6 +429,16 @@ const controls = { }, false, ); + + // Enter will fire a `click` event but we still need to manage focus + // So we bind to keyup which fires after and set focus here + on(menuItem, 'keyup', event => { + if (event.which !== 13) { + return; + } + + controls.focusFirstMenuItem.call(this, null, true); + }); }, // Create a settings menu item @@ -1074,6 +1084,23 @@ const controls = { toggleHidden(this.elements.settings.menu, !visible); }, + // Focus the first menu item in a given (or visible) menu + focusFirstMenuItem(pane, tabFocus = false) { + if (this.elements.settings.popup.hidden) { + return; + } + + let target = pane; + + if (!is.element(target)) { + target = Object.values(this.elements.settings.panels).find(pane => !pane.hidden); + } + + const firstItem = target.querySelector('[role^="menuitem"]'); + + setFocus.call(this, firstItem, tabFocus); + }, + // Show/hide menu toggleMenu(input) { const { popup } = this.elements.settings; @@ -1085,7 +1112,7 @@ const controls = { } // True toggle by default - const hidden = popup.hasAttribute('hidden'); + const { hidden } = popup; let show = hidden; if (is.boolean(input)) { @@ -1094,19 +1121,13 @@ const controls = { show = false; } else if (is.event(input)) { const isMenuItem = popup.contains(input.target); - const isButton = input.target === button; // If the click was inside the menu or if the click // wasn't the button or menu item and we're trying to // show the menu (a doc click shouldn't show the menu) - if (isMenuItem || (!isMenuItem && !isButton && show)) { + if (isMenuItem || (!isMenuItem && input.target !== button && show)) { return; } - - // Prevent the toggle being caught by the doc listener - if (isButton) { - input.stopPropagation(); - } } // Set button attributes @@ -1120,9 +1141,7 @@ const controls = { // Focus the first item if key interaction if (show && is.keyboardEvent(input)) { - const pane = Object.values(this.elements.settings.panels).find(pane => !pane.hidden); - const firstItem = pane.querySelector('[role^="menuitem"]'); - setFocus.call(this, firstItem, true); + controls.focusFirstMenuItem.call(this, null, true); } // If closing, re-focus the button else if (!show && !hidden) { @@ -1205,8 +1224,7 @@ const controls = { toggleHidden(target, false); // Focus the first item - const firstItem = target.querySelector('[role^="menuitem"]'); - setFocus.call(this, firstItem, tabFocus); + controls.focusFirstMenuItem.call(this, target, tabFocus); }, // Build the default HTML diff --git a/src/js/listeners.js b/src/js/listeners.js index 8176e9a3..c2b772b2 100644 --- a/src/js/listeners.js +++ b/src/js/listeners.js @@ -553,6 +553,9 @@ class Listeners { // Settings menu - click toggle this.bind(elements.buttons.settings, 'click', event => { + // Prevent the document click listener closing the menu + event.stopPropagation(); + controls.toggleMenu.call(player, event); }); @@ -563,8 +566,16 @@ class Listeners { elements.buttons.settings, 'keyup', event => { + const code = event.which; + // We only care about space and return - if (event.which !== 32 && event.which !== 13) { + if (![13, 32].includes(code)) { + return; + } + + // Because return triggers a click anyway, all we need to do is set focus + if (code === 13) { + controls.focusFirstMenuItem.call(player, null, true); return; } @@ -572,15 +583,13 @@ class Listeners { event.preventDefault(); // Prevent playing video (Firefox) - if (event.which === 32) { - event.stopPropagation(); - } + event.stopPropagation(); // Toggle menu controls.toggleMenu.call(player, event); }, null, - false, + false, // Can't be passive as we're preventing default ); // Escape closes menu diff --git a/src/js/utils/elements.js b/src/js/utils/elements.js index 3a3dfcfd..a6722da2 100644 --- a/src/js/utils/elements.js +++ b/src/js/utils/elements.js @@ -116,11 +116,7 @@ export function emptyElement(element) { // Replace element export function replaceElement(newChild, oldChild) { - if ( - !is.element(oldChild) || - !is.element(oldChild.parentNode) || - !is.element(newChild) - ) { + if (!is.element(oldChild) || !is.element(oldChild.parentNode) || !is.element(newChild)) { return null; } @@ -195,7 +191,7 @@ export function toggleHidden(element, hidden) { let hide = hidden; if (!is.boolean(hide)) { - hide = !element.hasAttribute('hidden'); + hide = !element.hidden; } if (hide) { @@ -263,10 +259,7 @@ export function trapFocus(element = null, toggle = false) { return; } - const focusable = getElements.call( - this, - 'button:not(:disabled), input:not(:disabled), [tabindex]', - ); + const focusable = getElements.call(this, 'button:not(:disabled), input:not(:disabled), [tabindex]'); const first = focusable[0]; const last = focusable[focusable.length - 1]; @@ -290,14 +283,7 @@ export function trapFocus(element = null, toggle = false) { } }; - toggleListener.call( - this, - this.elements.container, - 'keydown', - trap, - toggle, - false, - ); + toggleListener.call(this, this.elements.container, 'keydown', trap, toggle, false); } // Set focus and tab focus class