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

feat(NcActions): Allow to manually specify the semantic menu type #5336

Merged
merged 2 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 98 additions & 33 deletions src/components/NcActions/NcActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@
`<NcActions>` is supposed to be used with direct `<NcAction*>` children.
Although it works when actions are not direct children but wrapped in custom components, it has limitations:
- No `inline` prop property, including a single action display;
- Accessibility issues, including changed keyboard behavior;
- Accessibility issues, including changed keyboard behavior (see below);
- Invalid HTML.

```
Expand Down Expand Up @@ -932,6 +932,40 @@
}
</style>
```

#### Manually providing semantic menu information
Due to limitations of Vue, when using a custom wrapper for action components, you have to provide the semantic menu type yourself.
This is used for keyboard navigation and accessibility.
In this example a `NcActionInput` component is used within a custom wrapper, so `NcActions` is not able to detect the semantic menu type of 'dialog' automatically,
meaning it must be provided manually:

```vue
<template>
<NcActions menu-semantic-type="dialog">
<MyWrapper />
</NcActions>
</template>
<script>
import Pencil from 'vue-material-design-icons/Pencil.vue'

export default {
components: {
MyWrapper: {
template: `
<NcActionInput trailing-button-label="Submit" label="Rename group">
<template #icon>
<Pencil :size="20" />
</template>
</NcActionInput>`,
},
components: {
Pencil,
},
},
}
</script>
```

</docs>

<script>
Expand Down Expand Up @@ -968,7 +1002,7 @@
/**
* NcActions can be used as:
* - Application menu (has menu role)
* - Navigation (has no specific role, should be used an element with navigation role)
* - Expanded block (has no specific role, should be used an element with expanded 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.
Expand Down Expand Up @@ -1022,6 +1056,34 @@
default: null,
},

/**
* NcActions can be used as:
*
* - Application menu (has menu role)
* - Navigation (has no specific role, should be used an element with expanded role)
* - Popover with plain text or text inputs (has no specific role)
*
* By default the used type is automatically detected by components used in the default slot.#
*
* With Vue this is limited to direct children of the NcActions component.
* So if you use a wrapper, you have to provide the semantic type yourself (see Example)
*
* Choose:
*
* - 'dialog' if you use any of these components: NcActionInput', 'NcActionTextEditable'
* - 'menu' if you use any of these components: 'NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio'
* - 'expanded' if using one of these: 'NcActionLink', 'NcActionRouter'. This represents an expanded block.
* - 'tooltip' only to be used when a text without any interactive elements is used.
* - Leave this property unset otherwise
*/
forceSemanticType: {
type: String,
default: null,
validator(value) {
return ['dialog', 'menu', 'expanded', 'tooltip'].includes(value)
},
},

/**
* Apply primary styling for this menu
*/
Expand Down Expand Up @@ -1139,7 +1201,7 @@
opened: this.open,
focusIndex: 0,
/**
* @type {'menu'|'navigation'|'dialog'|'tooltip'|'unknown'}
* @type {'menu'|'expanded'|'dialog'|'tooltip'|'unknown'}
*/
actionsMenuSemanticType: 'unknown',
externalFocusTrapStack: [],
Expand All @@ -1166,7 +1228,7 @@
*
* "aria-controls" should only present together with a valid aria-haspopup
*
* There is no valid popup role for navigation and tooltip in `aria-haspopup`.
* There is no valid popup role for expanded and tooltip in `aria-haspopup`.
* aria-haspopup="true" is equivalent to aria-haspopup="menu".
* They must not be treated as menus.
*
Expand Down Expand Up @@ -1196,7 +1258,7 @@
role: 'menu',
},
},
navigation: {
expanded: {
popupRole: undefined,
withArrowNavigation: false,
withTabNavigation: true,
Expand Down Expand Up @@ -1393,7 +1455,7 @@
return this.$refs.menu.querySelector('li.active')
},
/**
* @return {NodeListOf<HTMLElement>}

Check warning on line 1458 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)
Expand Down Expand Up @@ -1640,36 +1702,39 @@
* Determine what kind of menu we have.
* It defines keyboard navigation 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'
if (this.forceSemanticType) {
this.actionsMenuSemanticType = this.forceSemanticType
} else {
// (!) Hotfix (!)
// In Vue 2 it is not easy to search for NcAction* in sub-component of a slot.
// When a menu is rendered, children are not mounted yet.
// If we have NcActions > MyActionsList > NcActionButton, only MyActionsList's vnode is available.
// So when NcActions has actions as non-direct children, here then we don't know about them.
// Like this menu has no buttons/links/inputs.
// It makes the menu incorrectly considered a tooltip.
const ncActions = actions.filter((action) => this.getActionName(action).startsWith('NcAction'))
if (ncActions.length === actions.length) {
// True tooltip
this.actionsMenuSemanticType = 'tooltip'
const textInputActions = ['NcActionInput', 'NcActionTextEditable']
const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
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 = 'expanded'
} else {
// Custom components are passed to the NcActions
this.actionsMenuSemanticType = 'unknown'
// (!) Hotfix (!)
// In Vue 2 it is not easy to search for NcAction* in sub-component of a slot.
// When a menu is rendered, children are not mounted yet.
// If we have NcActions > MyActionsList > NcActionButton, only MyActionsList's vnode is available.
// So when NcActions has actions as non-direct children, here then we don't know about them.
// Like this menu has no buttons/links/inputs.
// It makes the menu incorrectly considered a tooltip.
const ncActions = actions.filter((action) => this.getActionName(action).startsWith('NcAction'))
if (ncActions.length === actions.length) {
// True tooltip
this.actionsMenuSemanticType = 'tooltip'
} else {
// Custom components are passed to the NcActions
this.actionsMenuSemanticType = 'unknown'
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion styleguide.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ module.exports = async () => {
},
resolve: {
alias: {
vue: 'vue/dist/vue.js',
vue$: 'vue/dist/vue.js',
},
},
}),
Expand Down
48 changes: 46 additions & 2 deletions tests/unit/components/NcActions/NcActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,57 @@
*/

import { mount } from '@vue/test-utils'
import { Fragment } from 'vue-frag'

import NcActions from '../../../../src/components/NcActions/NcActions.vue'
import NcActionButton from '../../../../src/components/NcActionButton/NcActionButton.vue'
import NcActionInput from '../../../../src/components/NcActionInput/NcActionInput.vue'
import TestCompositionApi from './TestCompositionApi.vue'
import { defineComponent } from 'vue'

describe('NcActions.vue', () => {
'use strict'

describe('semantic menu type', () => {
const MyWrapper = defineComponent({
template: '<Fragment><NcActionInput /></Fragment>',
components: { Fragment, NcActionInput },
})

// This currently fails due to limitations of Vue 2
it.failing('Can auto detect semantic menu type in wrappers', () => {
const wrapper = mount(NcActions, {
slots: {
default: [
'<MyWrapper />',
],
},
stubs: {
MyWrapper,
},
})

expect(wrapper.vm.$data.actionsMenuSemanticType).toBe('dialog')
})

it('Can set the type manually', () => {
const wrapper = mount(NcActions, {
propsData: {
forceSemanticType: 'dialog',
},
slots: {
default: [
'<MyWrapper />',
],
},
stubs: {
MyWrapper,
},
})

expect(wrapper.vm.$data.actionsMenuSemanticType).toBe('dialog')
})
})

describe('when using the component with', () => {
it('no actions elements', () => {
const wrapper = mount(NcActions, {
Expand Down Expand Up @@ -178,7 +222,7 @@ describe('NcActions.vue', () => {
inline: 1,
},
})

expect(wrapper.find('img[src="http://example.com/image.png"').exists()).toBe(true)
})
})
Expand Down
Loading