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.

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

fix(NcActions): keyboard navigation #5153

Merged
merged 5 commits into from
Jan 26, 2024
Merged
Changes from all commits
Commits
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
160 changes: 105 additions & 55 deletions src/components/NcActions/NcActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- @author John Molakvoæ <skjnldsv@protonmail.com>
- @author Marco Ambrosini <marcoambrosini@icloud.com>
- @author Raimund Schlüßler <raimund.schluessler@mailbox.org>
- @author Grigorii K. Shartsev <me@shgk.me>
-
- @license GNU AGPL version 3 or any later version
-
Expand Down Expand Up @@ -697,8 +698,8 @@ export default {
</NcActions>
</p>

<h2>Popover</h2>
Has any elements, including text input element, or no buttons.
<h2>Dialog</h2>
Includes data input elements
<p>
<NcActions aria-label="Group management">
<NcActionInput trailing-button-label="Submit" label="Rename group">
Expand All @@ -714,6 +715,19 @@ export default {
</NcActionButton>
</NcActions>
</p>

<h2>Toolip</h2>
Has only text and not interactive elements
<p>
<NcActions aria-label="Contact" :inline="1">
<NcActionLink aria-label="View profile" href="/u/alice" icon="icon-user-white">
View profile
</NcActionLink>
<NcActionText icon="icon-timezone-white">
Local time: 10:12
</NcActionText>
</NcActions>
</p>
</div>
</template>

Expand Down Expand Up @@ -793,7 +807,6 @@ p {
}
</style>
```

</docs>

<script>
Expand Down Expand Up @@ -835,7 +848,7 @@ export default {
* Provide the role for NcAction* components in the NcActions content.
* @type {import('vue').ComputedRef<boolean>}
*/
'NcActions:isSemanticMenu': computed(() => this.isSemanticMenu),
'NcActions:isSemanticMenu': computed(() => this.actionsMenuSemanticType === 'menu'),
}
},

Expand Down Expand Up @@ -991,9 +1004,10 @@ export default {
opened: this.open,
focusIndex: 0,
randomId: `menu-${GenRandomId()}`,
isSemanticMenu: false,
isSemanticNavigation: false,
isSemanticPopoverLike: false,
/**
* @type {'menu'|'navigation'|'dialog'|'tooltip'|''}
*/
actionsMenuSemanticType: '',
}
},

Expand All @@ -1005,6 +1019,10 @@ export default {
// If it has a name, we use a secondary button
: this.menuName ? 'secondary' : 'tertiary')
},

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

watch: {
Expand All @@ -1019,16 +1037,25 @@ export default {
},

methods: {
/**
* Get the name of the action component
*
* @param {import('vue').VNode} action - a vnode with a NcAction* component instance
* @return {string} the name of the action component
*/
getActionName(action) {
return action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag
},

/**
* Do we have exactly one Action and
* is it allowed as a standalone element?
*
* @param {Array} action The action to check
* @param {import('vue').VNode} action The action to check
* @return {boolean}
*/
isValidSingleAction(action) {
const componentName = action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag
return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(componentName)
return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(this.getActionName(action))
},

/**
Expand Down Expand Up @@ -1087,8 +1114,10 @@ export default {
// close everything
this.focusIndex = 0

// focus back the menu button
this.$refs.menuButton.$el.focus()
if (returnFocus) {
// Focus back the menu button
this.$refs.menuButton.$el.focus()
}
},

onOpen(event) {
Expand Down Expand Up @@ -1126,8 +1155,10 @@ export default {
* @param {object} event The keydown event
*/
onKeydown(event) {
if (event.key === 'Tab' && !this.isSemanticPopoverLike) {
this.closeMenu(false)
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 === 'ArrowUp') {
Expand Down Expand Up @@ -1221,6 +1252,12 @@ export default {
},
onBlur(event) {
this.$emit('blur', event)

// When there is no focusable elements to handle Tab press from actions menu
// It requries manual closing
if (this.actionsMenuSemanticType === 'tooltip') {
this.closeMenu(false)
}
},
},

Expand All @@ -1236,46 +1273,64 @@ export default {
* This also ensure that we don't get 'text' elements, which would
* become problematic later on.
*/
const actions = (this.$slots.default || []).filter(
action => action?.componentOptions?.tag || action?.componentOptions?.Ctor?.extendOptions?.name,
)

const getActionName = (action) => action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag

const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
const textInputActions = ['NcActionInput', 'NcActionTextEditable']
const linkActions = ['NcActionLink', 'NcActionRouter']
const actions = (this.$slots.default || []).filter(action => this.getActionName(action))

const hasTextInputAction = actions.some(action => textInputActions.includes(getActionName(action)))
const hasMenuItemAction = actions.some(action => menuItemsActions.includes(getActionName(action)))
const hasLinkAction = actions.some(action => linkActions.includes(getActionName(action)))

// We consider the NcActions to have role="menu" if it consists some button-like action and not text inputs
this.isSemanticMenu = hasMenuItemAction && !hasTextInputAction
// We consider the NcActions to be navigation if it consists some link-like action
this.isSemanticNavigation = hasLinkAction && !hasMenuItemAction && !hasTextInputAction
// If it is not a menu and not a navigation, it is a popover with items: a form or just a text
this.isSemanticPopoverLike = !this.isSemanticMenu && !this.isSemanticNavigation
// Check that we have at least one action
if (actions.length === 0) {
return
}

const popupRole = this.isSemanticMenu
? 'menu'
: hasTextInputAction
? 'dialog'
: 'true'
/**
* Separate the actions into inline and menu actions
*/

/**
* Filter and list actions that are allowed to be displayed inline
* @type {import('vue').VNode[]}
*/
let inlineActions = actions.filter(this.isValidSingleAction)
if (this.forceMenu && inlineActions.length > 0 && this.inline > 0) {
let validInlineActions = actions.filter(this.isValidSingleAction)
if (this.forceMenu && validInlineActions.length > 0 && this.inline > 0) {
Vue.util.warn('Specifying forceMenu will ignore any inline actions rendering.')
inlineActions = []
validInlineActions = []
}
/**
* @type {import('vue').VNode[]}
*/
const inlineActions = validInlineActions.slice(0, this.inline)
/**
* @type {import('vue').VNode[]}
*/
const menuActions = actions.filter(action => !inlineActions.includes(action))

// Check that we have at least one action
if (actions.length === 0) {
return
/**
* Determine what kind of menu we have.
* It defines focus behavior and a11y.
*/

const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
const textInputActions = ['NcActionInput', 'NcActionTextEditable']
const linkActions = ['NcActionLink', 'NcActionRouter']

const hasTextInputAction = menuActions.some(action => textInputActions.includes(this.getActionName(action)))
const hasMenuItemAction = menuActions.some(action => menuItemsActions.includes(this.getActionName(action)))
const hasLinkAction = menuActions.some(action => linkActions.includes(this.getActionName(action)))

if (hasTextInputAction) {
this.actionsMenuSemanticType = 'dialog'
} else if (hasMenuItemAction) {
this.actionsMenuSemanticType = 'menu'
} else if (hasLinkAction) {
this.actionsMenuSemanticType = 'navigation'
} else {
this.actionsMenuSemanticType = 'tooltip'
}

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

/**
* Render the provided action
Expand Down Expand Up @@ -1381,10 +1436,8 @@ export default {
container: this.container,
popoverBaseClass: 'action-item__popper',
popupRole,
// Menu and navigation should not have focus trap
// Tab should close the menu and move focus to the next UI element
setReturnFocus: !this.isSemanticPopoverLike ? null : this.$refs.menuButton?.$el,
focusTrap: this.isSemanticPopoverLike,
setReturnFocus: this.withFocusTrap ? this.$refs.menuButton?.$el : null,
focusTrap: this.withFocusTrap,
},
// For some reason the popover component
// does not react to props given under the 'props' key,
Expand Down Expand Up @@ -1457,8 +1510,8 @@ export default {
* If we have a single action only and didn't force a menu,
* we render the action as a standalone button
*/
if (actions.length === 1 && inlineActions.length === 1 && !this.forceMenu) {
return renderInlineAction(inlineActions[0])
if (actions.length === 1 && validInlineActions.length === 1 && !this.forceMenu) {
return renderInlineAction(actions[0])
}

// If we completely re-render the children
Expand All @@ -1477,9 +1530,6 @@ export default {
* If we some inline actions to render, render them, then the menu
*/
if (inlineActions.length > 0 && this.inline > 0) {
const renderedInlineActions = inlineActions.slice(0, this.inline)
// Filter already rendered actions
const menuActions = actions.filter(action => !renderedInlineActions.includes(action))
return h('div',
{
class: [
Expand All @@ -1489,7 +1539,7 @@ export default {
},
[
// Render inline actions
...renderedInlineActions.map(renderInlineAction),
...inlineActions.map(renderInlineAction),
// render the rest within the popover menu
menuActions.length > 0
? h('div',
Expand Down