Skip to content

Commit

Permalink
Merge pull request #5336 from nextcloud-libraries/fix/actions-discove…
Browse files Browse the repository at this point in the history
…r-children

feat(NcActions): Allow to manually specify the semantic menu type
  • Loading branch information
susnux authored Mar 11, 2024
2 parents ff6664b + 8c72eb4 commit ba95036
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 36 deletions.
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 @@ p {
`<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 @@ export default {
}
</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 @@ export default {
/**
* 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 @@ export default {
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 @@ export default {
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 @@ export default {
*
* "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 @@ export default {
role: 'menu',
},
},
navigation: {
expanded: {
popupRole: undefined,
withArrowNavigation: false,
withTabNavigation: true,
Expand Down Expand Up @@ -1640,36 +1702,39 @@ export default {
* 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

0 comments on commit ba95036

Please sign in to comment.