Fix issue where enter key wasn’t setting focus correctly

This commit is contained in:
Sam Potts 2018-08-05 22:41:21 +10:00
parent 4ea458e1a3
commit 0bc6b1f1b3
3 changed files with 50 additions and 37 deletions

46
src/js/controls.js vendored
View File

@ -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

View File

@ -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

View File

@ -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