Skip to content

Commit

Permalink
fix(NcActions): set menu roles only in true menu
Browse files Browse the repository at this point in the history
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
  • Loading branch information
ShGKme committed Nov 16, 2023
1 parent 57878b6 commit 63bc999
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 32 deletions.
12 changes: 10 additions & 2 deletions src/components/NcActionButton/NcActionButton.vue
Expand Up @@ -173,12 +173,12 @@ export default {
</docs>

<template>
<li class="action" :class="{ 'action--disabled': disabled }">
<li class="action" :class="{ 'action--disabled': disabled }" :role="isInSemanticMenu && 'presentation'">
<button class="action-button"
:class="{ focusable: isFocusable }"
:aria-label="ariaLabel"
:title="title"
role="menuitem"
:role="isInSemanticMenu && 'menuitem'"
type="button"
@click="onClick">
<!-- @slot Manually provide icon -->
Expand Down Expand Up @@ -235,6 +235,13 @@ export default {
},
mixins: [ActionTextMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* disabled state of the action button
Expand Down Expand Up @@ -263,6 +270,7 @@ export default {
default: false,
},
},
computed: {
/**
* determines if the action is focusable
Expand Down
10 changes: 9 additions & 1 deletion src/components/NcActionButtonGroup/NcActionButtonGroup.vue
Expand Up @@ -81,7 +81,7 @@ export default {
</docs>

<template>
<li class="nc-button-group-base">
<li class="nc-button-group-base" :role="isInSemanticMenu && 'presentation'">
<div v-if="name" :id="labelId">
{{ name }}
</div>
Expand All @@ -100,6 +100,14 @@ import GenRandomId from '../../utils/GenRandomId.js'
*/
export default defineComponent({
name: 'NcActionButtonGroup',
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* Optional text shown below the button group
Expand Down
10 changes: 9 additions & 1 deletion src/components/NcActionCaption/NcActionCaption.vue
Expand Up @@ -30,14 +30,22 @@ This component is made to be used inside of the [NcActions](#NcActions) componen
</docs>

<template>
<li class="app-navigation-caption">
<li class="app-navigation-caption" :role="isInSemanticMenu && 'presentation'">
{{ name }}
</li>
</template>

<script>
export default {
name: 'NcActionCaption',
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* The caption's text
Expand Down
23 changes: 21 additions & 2 deletions src/components/NcActionCheckbox/NcActionCheckbox.vue
Expand Up @@ -35,8 +35,8 @@ This component is made to be used inside of the [NcActions](#NcActions) componen
</docs>

<template>
<li class="action" :class="{ 'action--disabled': disabled }">
<span class="action-checkbox">
<li class="action" :class="{ 'action--disabled': disabled }" :role="isInSemanticMenu && 'presentation'">
<span class="action-checkbox" :role="isInSemanticMenu && 'menuitemcheckbox'" :aria-checked="ariaChecked">
<input :id="id"
ref="checkbox"
:disabled="disabled"
Expand Down Expand Up @@ -64,6 +64,13 @@ export default {
mixins: [ActionGlobalMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* id attribute of the checkbox element
Expand Down Expand Up @@ -115,6 +122,18 @@ export default {
isFocusable() {
return !this.disabled
},
/**
* aria-checked attribute for role="menuitemcheckbox"
*
* @return {'true'|'false'|undefined} aria-checked value if needed
*/
ariaChecked() {
if (this.isInSemanticMenu) {
return this.checked ? 'true' : 'false'
}
return undefined
},
},
methods: {
Expand Down
11 changes: 9 additions & 2 deletions src/components/NcActionLink/NcActionLink.vue
Expand Up @@ -78,15 +78,15 @@ export default {
</docs>

<template>
<li class="action">
<li class="action" :role="isInSemanticMenu && 'presentation'">
<a :download="download"
:href="href"
:aria-label="ariaLabel"
:target="target"
:title="title"
class="action-link focusable"
rel="nofollow noreferrer noopener"
role="menuitem"
:role="isInSemanticMenu && 'menuitem'"
@click="onClick">

<!-- @slot Manually provide icon -->
Expand Down Expand Up @@ -133,6 +133,13 @@ export default {
mixins: [ActionTextMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* destionation to link to
Expand Down
23 changes: 21 additions & 2 deletions src/components/NcActionRadio/NcActionRadio.vue
Expand Up @@ -37,8 +37,8 @@ So that only one of each name set can be selected at the same time.
</docs>

<template>
<li class="action" :class="{ 'action--disabled': disabled }">
<span class="action-radio">
<li class="action" :class="{ 'action--disabled': disabled }" :role="isInSemanticMenu && 'presentation'">
<span class="action-radio" role="menuitemradio" :aria-checked="ariaChecked">
<input :id="id"
ref="radio"
:disabled="disabled"
Expand Down Expand Up @@ -67,6 +67,13 @@ export default {
mixins: [ActionGlobalMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* id attribute of the radio element
Expand Down Expand Up @@ -126,6 +133,18 @@ export default {
isFocusable() {
return !this.disabled
},
/**
* aria-checked attribute for role="menuitemcheckbox"
*
* @return {'true'|'false'|undefined} aria-checked value if needed
*/
ariaChecked() {
if (this.isInSemanticMenu) {
return this.checked ? 'true' : 'false'
}
return undefined
},
},
methods: {
Expand Down
14 changes: 11 additions & 3 deletions src/components/NcActionRouter/NcActionRouter.vue
Expand Up @@ -22,13 +22,14 @@
-->

<template>
<li class="action">
<router-link :to="to"
<li class="action" :role="isInSemanticMenu && 'presentation'">
<RouterLink :to="to"
:aria-label="ariaLabel"
:exact="exact"
:title="title"
class="action-router focusable"
rel="nofollow noreferrer noopener"
:role="isInSemanticMenu && 'menuitem'"
@click.native="onClick">
<!-- @slot Manually provide icon -->
<slot name="icon">
Expand Down Expand Up @@ -62,7 +63,7 @@

<!-- fake slot to gather inner text -->
<slot v-if="false" />
</router-link>
</RouterLink>
</li>
</template>

Expand All @@ -74,6 +75,13 @@ export default {
mixins: [ActionTextMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
props: {
/**
* router-link to prop [https://router.vuejs.org/api/#to](https://router.vuejs.org/api/#to)
Expand Down
8 changes: 7 additions & 1 deletion src/components/NcActionText/NcActionText.vue
Expand Up @@ -22,7 +22,7 @@
-->

<template>
<li class="action">
<li class="action" :role="isInSemanticMenu && 'presentation'">
<span class="action-text"
@click="onClick">
<!-- @slot Manually provide icon -->
Expand Down Expand Up @@ -70,6 +70,12 @@ export default {
mixins: [ActionTextMixin],
inject: {
isInSemanticMenu: {
from: 'NcActions:isSemanticMenu',
default: false,
},
},
}
</script>

Expand Down
51 changes: 33 additions & 18 deletions src/components/NcActions/NcActions.vue
Expand Up @@ -802,7 +802,7 @@ import NcPopover from '../NcPopover/index.js'
import GenRandomId from '../../utils/GenRandomId.js'
import { t } from '../../l10n.js'
import Vue from 'vue'
import Vue, { computed } from 'vue'
import DotsHorizontal from 'vue-material-design-icons/DotsHorizontal.vue'
const focusableSelector = '.focusable'
Expand All @@ -824,6 +824,21 @@ export default {
NcPopover,
},
provide() {
return {
/**
* NcActions can be used as:
* - Application menu (has menu role)
* - Navigation (has no specific role, should be used an element with navigation role)
* - Popover with plain text or text inputs (has no specific role)
* Depending on the usage (used items), the menu and its items should have different roles for a11y.
* Provide the role for NcAction* components in the NcActions content.
* @type {import('vue').ComputedRef<boolean>}
*/
'NcActions:isSemanticMenu': computed(() => this.isSemanticMenu),
}
},
props: {
/**
* Specify the open state of the popover menu
Expand Down Expand Up @@ -976,6 +991,7 @@ export default {
opened: this.open,
focusIndex: 0,
randomId: `menu-${GenRandomId()}`,
isSemanticMenu: false,
}
},
Expand Down Expand Up @@ -1209,17 +1225,18 @@ export default {
action => action?.componentOptions?.tag || action?.componentOptions?.Ctor?.extendOptions?.name,
)
const isNavLink = (action) => {
const componentName = action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag
const href = action?.componentOptions?.propsData?.href
return (
componentName === 'NcActionLink'
&& !href?.startsWith('#')
&& new URL(href, window.location.origin).origin === window.location.origin
)
}
// Automatically detect whether all actions are website navigation links
const isNav = actions.every(isNavLink)
const getActionName = (action) => action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag
const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
const textInputActions = ['NcActionInput', 'NcActionTextEditable']
// Link actions could be a part of menu, but are without buttons they are considered navigation
// const linkActions = ['NcActionLink', 'NcActionRouter']
const hasNoTextInputActions = actions.every(action => !textInputActions.includes(getActionName(action)))
const hasSomeMenuItemAction = actions.some(action => menuItemsActions.includes(getActionName(action)))
// We consider the NcActions to have role="menu" if it consists some button-like action and not text inputs
this.isSemanticMenu = hasSomeMenuItemAction && hasNoTextInputActions
/**
* Filter and list actions that are allowed to be displayed inline
Expand Down Expand Up @@ -1313,9 +1330,6 @@ export default {
},
})
)
const ariaExpandedForTrigger = () => {
return (isNav || this.opened) ? this.opened.toString() : null
}
return h('NcPopover',
{
ref: 'popover',
Expand Down Expand Up @@ -1357,10 +1371,11 @@ export default {
slot: 'trigger',
ref: 'menuButton',
attrs: {
'aria-haspopup': isNav ? null : 'menu',
'aria-haspopup': this.isSemanticMenu ? null : 'menu',
'aria-label': this.menuName ? null : this.ariaLabel,
'aria-controls': this.opened ? this.randomId : null,
'aria-expanded': ariaExpandedForTrigger(),
// Do not add aria-expanded="true" when it is closed
'aria-expanded': this.opened ? 'true' : undefined,
},
on: {
focus: this.onFocus,
Expand All @@ -1387,7 +1402,7 @@ export default {
attrs: {
id: this.randomId,
tabindex: '-1',
role: isNav ? null : 'menu',
role: this.isSemanticMenu ? 'menu' : undefined,
},
}, [
actions,
Expand Down

0 comments on commit 63bc999

Please sign in to comment.