Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(NcActions): put in order tab and arrow navigation #5305

Merged
merged 1 commit into from
Feb 26, 2024
Merged

fix(NcActions): put in order tab and arrow navigation #5305

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
214 changes: 160 additions & 54 deletions src/components/NcActions/NcActions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,8 @@
<template>
<div>
<h2>Application menu</h2>
Uses buttons, button groups, links and router links, separators, text. May have checkboxes and radio buttons.
<p>Has buttons, button groups, links and router links, separators, texts. May have checkboxes and radio buttons. Separator can be used to make groups of radio buttons as well.</p>
<p><kbd>Arrows</kbd> are used to navigate between items, <kbd>Tab</kbd> is used to navigate to the next UI element on the page.</p>
<p>
<NcActions aria-label="Email menu" type="tertiary">
<NcActionButtonGroup>
Expand Down Expand Up @@ -624,9 +625,6 @@
</NcActionLink>
</NcActions>
</p>

<h2>Menu in menubar</h2>
Same as application menu. Separator can be used to make groups of radio buttons as well.
<p>
<NcActions aria-label="Text settings" type="tertiary">
<template #icon>
Expand Down Expand Up @@ -677,7 +675,8 @@
</p>

<h2>Navigation</h2>
Uses links or router links. May use text elements, captions and separators.
<p>Has links or router links. May use text elements, captions and separators.</p>
<p>Uses classic <kbd>Tab</kbd> navigation.</p>
<p>
<NcActions aria-label="Applications navigation" :inline="2" type="tertiary">
<NcActionLink href="/apps/dashboard" icon="icon-category-dashboard-white">
Expand All @@ -692,14 +691,15 @@
<NcActionLink href="/apps/contacts" icon="icon-contacts-white">
Contacts
</NcActionLink>
<NcActionLink href="/apps/spreed" icon="icon-circles-white">
<NcActionLink href="/apps/circles" icon="icon-circles-white">
Circles
</NcActionLink>
</NcActions>
</p>

<h2>Dialog</h2>
Includes data input elements
<p>Includes data input elements, forms.</p>
<p>Uses <kbd>Tab</kbd> navigation with a focus trap.</p>
<p>
<NcActions aria-label="Group management">
<NcActionInput trailing-button-label="Submit" label="Rename group">
Expand All @@ -717,7 +717,7 @@
</p>

<h2>Toolip</h2>
Has only text and not interactive elements
<p>Has only text and no interactive elements.</p>
<p>
<NcActions aria-label="Contact" :inline="1">
<NcActionLink aria-label="View profile" href="/u/alice" icon="icon-user-white">
Expand Down Expand Up @@ -1132,9 +1132,9 @@
focusIndex: 0,
randomId: `menu-${GenRandomId()}`,
/**
* @type {'menu'|'navigation'|'dialog'|'tooltip'|''}
* @type {'menu'|'navigation'|'dialog'|'tooltip'|'unknown'}
*/
actionsMenuSemanticType: '',
actionsMenuSemanticType: 'unknown',
externalFocusTrapStack: [],
}
},
Expand All @@ -1148,8 +1148,63 @@
: this.menuName ? 'secondary' : 'tertiary')
},

/**
* A11y roles and keyboard navigation configuration depending on the semantic type
*/
config() {
/**
* Accessibility notes:
*
* There is no valid popup role for navigation and tooltip in `aria-haspopup`.
* aria-haspopup="true" is equivalent to aria-haspopup="menu".
* They must not be treated as menus.
*
* Having both arrow and tab navigation is not allowed for a11y.
* Either menu is an atomic UI element, and arrows select menu items, Tab moves to the next UI element.
* Or the menu is an expanded list of UI elements.
*
* Navigation type is just an "expanded" block, similar to native <details> element.
*/
const configs = {
menu: {
popupRole: 'menu',
withArrowNavigation: true,
withTabNavigation: false,
withFocusTrap: false,
},
navigation: {
popupRole: undefined,
withArrowNavigation: false,
withTabNavigation: true,
withFocusTrap: false,
},
dialog: {
popupRole: 'dialog',
withArrowNavigation: false,
withTabNavigation: true,
withFocusTrap: true,
},
tooltip: {
popupRole: undefined,
withArrowNavigation: false,
withTabNavigation: false,
withFocusTrap: false,
},
// Due to Vue limitations, we sometimes cannot determine the true type
// As a fallback use both arrow navigation and focus trap
unknown: {
popupRole: undefined,
role: undefined,
withArrowNavigation: true,
withTabNavigation: false,
withFocusTrap: true,
},
}
return configs[this.actionsMenuSemanticType]
},

withFocusTrap() {
return this.actionsMenuSemanticType === 'dialog'
return this.config.withFocusTrap
},
},

Expand Down Expand Up @@ -1189,7 +1244,7 @@
* We need to pause all the focus traps for opening popover and then unpause them back after closing.
*/
intersectIntoCurrentFocusTrapStack() {
if (this.withFocusTrap) {
if (this.config.withFocusTrap) {
return
}

Expand Down Expand Up @@ -1291,10 +1346,25 @@
},

// MENU KEYS & FOCUS MANAGEMENT
// focus nearest focusable item on mouse move
// DO NOT change the focus if the target is already focused
// this will prevent issues with input being unfocused
// on mouse move
/**
* @return {HTMLElement|null}
*/
getCurrentActiveMenuItemElement() {
return this.$refs.menu.querySelector('li.active')
},
/**
* @return {NodeListOf<HTMLElement>}

Check warning on line 1356 in src/components/NcActions/NcActions.vue

View workflow job for this annotation

GitHub Actions / NPM lint 

The type 'NodeListOf' is undefined
*/
getFocusableMenuItemElements() {
return this.$refs.menu.querySelectorAll(focusableSelector)
},
/**
* Focus nearest focusable item on mouse move.
* DO NOT change the focus if the target is already focused
* this will prevent issues with input being unfocused
* on mouse move
* @param {PointerEvent} event - The mouse move event
*/
onMouseFocusAction(event) {
if (document.activeElement === event.target) {
return
Expand All @@ -1304,7 +1374,7 @@
if (menuItem && this.$refs.menu.contains(menuItem)) {
const focusableItem = menuItem.querySelector(focusableSelector)
if (focusableItem) {
const focusList = this.$refs.menu.querySelectorAll(focusableSelector)
const focusList = this.getFocusableMenuItemElements()
const focusIndex = [...focusList].indexOf(focusableItem)
if (focusIndex > -1) {
this.focusIndex = focusIndex
Expand All @@ -1319,33 +1389,77 @@
* @param {object} event The keydown event
*/
onKeydown(event) {
if (event.key === 'Tab' && !this.withFocusTrap) {
// Return focus to restore Tab sequence
// So browser will correctly move focus to the next element
this.closeMenu(true)
}
if (event.key === 'Tab') {
if (this.config.withFocusTrap) {
// Focus is managed by focus-trap
return
}

if (event.key === 'ArrowUp') {
this.focusPreviousAction(event)
}
if (!this.config.withTabNavigation) {
// Tab is not used for navigation - close the menu
// Return focus to restore Tab sequence
// So browser will correctly move focus to the next element
this.closeMenu(true)
return
}

if (event.key === 'ArrowDown') {
this.focusNextAction(event)
}
// Tab is used for classic navigation but we implement it manually

if (event.key === 'PageUp') {
this.focusFirstAction(event)
event.preventDefault()

const focusList = this.getFocusableMenuItemElements()
const focusIndex = [...focusList].indexOf(document.activeElement)
if (focusIndex === -1) {
// This is not supposed to happen
// But if it does - do nothing and keep native behavior
return
}
const newFocusIndex = event.shiftKey ? focusIndex - 1 : focusIndex + 1

// There is no focus-trap, so going out of the menu closes it
if (newFocusIndex < 0 || newFocusIndex === focusList.length) {
this.closeMenu(true)
}

// Update current focused element
this.focusIndex = newFocusIndex
this.focusAction()
return
}

if (event.key === 'PageDown') {
this.focusLastAction(event)
if (this.config.withArrowNavigation) {
if (event.key === 'ArrowUp') {
this.focusPreviousAction(event)
}

if (event.key === 'ArrowDown') {
this.focusNextAction(event)
}

if (event.key === 'PageUp') {
this.focusFirstAction(event)
}

if (event.key === 'PageDown') {
this.focusLastAction(event)
}
}

if (event.key === 'Escape') {
this.closeMenu()
event.preventDefault()
}
},
onTriggerKeydown(event) {
if (event.key === 'Escape') {
// Tooltip has no focusable elements and the focus remains on the trigger button.
// So keydown event on the menu cannot be handled to close Tooltip on Escape.
// Handle on the trigger.
if (this.actionsMenuSemanticType === 'tooltip') {
this.closeMenu()
}
}
},
removeCurrentActive() {
const currentActiveElement = this.$refs.menu.querySelector('li.active')
if (currentActiveElement) {
Expand All @@ -1354,7 +1468,7 @@
},
focusAction() {
// TODO: have a global disabled state for non input elements
const focusElement = this.$refs.menu.querySelectorAll(focusableSelector)[this.focusIndex]
const focusElement = this.getFocusableMenuItemElements()[this.focusIndex]
if (focusElement) {
this.removeCurrentActive()
const liMenuParent = focusElement.closest('li.action')
Expand All @@ -1377,7 +1491,7 @@
},
focusNextAction(event) {
if (this.opened) {
const indexLength = this.$refs.menu.querySelectorAll(focusableSelector).length - 1
const indexLength = this.getFocusableMenuItemElements().length - 1
if (this.focusIndex === indexLength) {
this.focusFirstAction(event)
} else {
Expand All @@ -1391,7 +1505,7 @@
if (this.opened) {
this.preventIfEvent(event)
// In case a button is considered aria-selected we will use this one as a initial focus
const firstSelectedIndex = [...this.$refs.menu.querySelectorAll(focusableSelector)].findIndex((button) => {
const firstSelectedIndex = [...this.getFocusableMenuItemElements()].findIndex((button) => {
return button.parentElement.getAttribute('aria-selected')
})
this.focusIndex = firstSelectedIndex > -1 ? firstSelectedIndex : 0
Expand All @@ -1401,7 +1515,7 @@
focusLastAction(event) {
if (this.opened) {
this.preventIfEvent(event)
this.focusIndex = this.$refs.menu.querySelectorAll(focusableSelector).length - 1
this.focusIndex = this.getFocusableMenuItemElements().length - 1
this.focusAction()
}
},
Expand All @@ -1422,7 +1536,7 @@
if (this.actionsMenuSemanticType === 'tooltip') {
// Tooltip is supposed to have no focusable element.
// However, if there is a custom focusable element, it will be auto-focused and cause the menu to be closed on open.
if (this.$refs.menu && this.$refs.menu.querySelectorAll(focusableSelector).length === 0) {
if (this.$refs.menu && this.getFocusableMenuItemElements().length === 0) {
this.closeMenu(false)
}
}
Expand Down Expand Up @@ -1482,7 +1596,7 @@

/**
* Determine what kind of menu we have.
* It defines focus behavior and a11y.
* It defines keyboard navigation and a11y.
*/

const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
Expand Down Expand Up @@ -1513,19 +1627,10 @@
this.actionsMenuSemanticType = 'tooltip'
} else {
// Custom components are passed to the NcActions
// dialog is a universal fallback option
this.actionsMenuSemanticType = 'dialog'
this.actionsMenuSemanticType = 'unknown'
}
}

const actionsRoleToHtmlPopupRole = {
dialog: 'dialog',
menu: 'menu',
navigation: undefined,
tooltip: undefined,
}
const popupRole = actionsRoleToHtmlPopupRole[this.actionsMenuSemanticType]

/**
* Render the provided action
*
Expand Down Expand Up @@ -1629,9 +1734,9 @@
boundary: this.boundariesElement,
container: this.container,
popoverBaseClass: 'action-item__popper',
popupRole,
setReturnFocus: this.withFocusTrap ? this.$refs.menuButton?.$el : null,
focusTrap: this.withFocusTrap,
popupRole: this.config.popupRole,
setReturnFocus: this.config.withFocusTrap ? this.$refs.menuButton?.$el : null,
focusTrap: this.config.withFocusTrap,
},
// For some reason the popover component
// does not react to props given under the 'props' key,
Expand Down Expand Up @@ -1662,13 +1767,14 @@
ref: 'menuButton',
attrs: {
'aria-label': this.menuName ? null : this.ariaLabel,
// 'aria-controls' is not needed for navigation menu
'aria-controls': this.opened && popupRole ? this.randomId : null,
// 'aria-controls' should only present together with a valid aria-haspopup
'aria-controls': this.opened && this.config.popupRole ? this.randomId : null,
},
on: {
focus: this.onFocus,
blur: this.onBlur,
click: this.onClick,
keydown: this.onTriggerKeydown,
},
}, [
h('template', { slot: 'icon' }, [triggerIcon]),
Expand All @@ -1691,7 +1797,7 @@
attrs: {
id: this.randomId,
tabindex: '-1',
role: popupRole,
role: this.config.popupRole,
// TODO: allow to provide dialog aria-label
},
}, [
Expand Down