Skip to content

Commit

Permalink
Merge pull request #5153 from nextcloud-libraries/fix/nc-actions--key…
Browse files Browse the repository at this point in the history 
…board-navigation

fix(NcActions): keyboard navigation
  • Loading branch information
@susnux
susnux committed Jan 26, 2024
2 parents 0965f37 + f26764a commit c9cc3e7
Showing 1 changed file with 105 additions and 55 deletions.
160 changes: 105 additions & 55 deletions src/components/NcActions/NcActions.vue
View file
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 @@ -992,9 +1005,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 @@ -1006,6 +1020,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 @@ -1020,16 +1038,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 @@ -1088,8 +1115,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 @@ -1127,8 +1156,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 @@ -1222,6 +1253,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)
}
},
onClick(event) {
/**
Expand All @@ -1248,46 +1285,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 @@ -1393,10 +1448,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 @@ -1470,8 +1523,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 @@ -1490,9 +1543,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 @@ -1502,7 +1552,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

0 comments on commit c9cc3e7

Please sign in to comment.