Merge pull request #5218 from nextcloud/fix/5216/remove-incorrect-roles

fix(menubar): menubar and table menus a11y

src/components/Editor/SessionList.vue

@@ -22,17 +22,20 @@
 
 <template>
 	<NcPopover class="session-list" placement="bottom">
-		<div slot="trigger">
-			<button :title="label"
-				:aria-label="label"
-				class="avatar-list">
-				<div class="avatardiv icon-group" />
-				<AvatarWrapper v-for="session in sessionsVisible"
-					:key="session.id"
-					:session="session"
-					:size="40" />
-			</button>
-		</div>
+		<template #trigger="{ attrs }">
+			<div>
+				<button :title="label"
+					:aria-label="label"
+					class="avatar-list"
+					v-bind="attrs">
+					<div class="avatardiv icon-group" />
+					<AvatarWrapper v-for="session in sessionsVisible"
+						:key="session.id"
+						:session="session"
+						:size="40" />
+				</button>
+			</div>
+		</template>
 		<template #default>
 			<div class="session-menu">
 				<slot name="lastSaved" />

src/components/InlineActionsContainer.vue

@@ -1,56 +0,0 @@
-<!--
-  - @copyright Copyright (c) 2022
-  -
-  - @license GNU AGPL version 3 or any later version
-  -
-  - This program is free software: you can redistribute it and/or modify
-  - it under the terms of the GNU Affero General Public License as
-  - published by the Free Software Foundation, either version 3 of the
-  - License, or (at your option) any later version.
-  -
-  - This program is distributed in the hope that it will be useful,
-  - but WITHOUT ANY WARRANTY; without even the implied warranty of
-  - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-  - GNU Affero General Public License for more details.
-  -
-  - You should have received a copy of the GNU Affero General Public License
-  - along with this program. If not, see <http://www.gnu.org/licenses/>.
-  -
-  -->
-<template>
-	<li class="inline-container-base">
-		<ul class="inline-container-content">
-			<slot />
-		</ul>
-	</li>
-</template>
-
-<script>
-import { defineComponent } from 'vue'
-
-/**
- * A wrapper for allowing inlining NcAction components
- * within the action menu (see TableHeaderView for a usage example)
- */
-export default defineComponent({
-	name: 'InlineActionsContainer',
-})
-</script>
-
-<style lang="scss">
-ul.inline-container-content {
-	display: flex;
-	justify-content: space-between;
-	li {
-		flex: 1 1;
-	}
-
-	.action-button {
-		// Fix action buttons beeing shifted to the left (right padding)
-		padding: 0 !important;
-		width: 100%;
-		display: flex;
-		justify-content: center;
-	}
-}
-</style>

src/components/Menu/ActionAttachmentUpload.vue

@@ -25,14 +25,11 @@
 		:name="actionEntry.label"
 		:title="actionEntry.label"
 		:aria-label="actionEntry.label"
-		:container="menuIDSelector"
-		role="menu"
-		aria-haspopup>
+		:container="menuIDSelector">
 		<template #icon>
 			<component :is="icon"
 				:name="actionEntry.label"
-				:aria-label="actionEntry.label"
-				aria-haspopup />
+				:aria-label="actionEntry.label" />
 		</template>
 		<NcActionButton v-if="$editorUpload"
 			close-after-click

src/components/Menu/ActionFormattingHelp.vue

@@ -20,25 +20,30 @@
   -
   -->
 <template>
-	<NcActionButton close-after-click
+	<NextcloudVueNcActionButton close-after-click
 		data-text-action-entry="formatting-help"
 		v-on="$listeners">
 		<template #icon>
 			<Help />
 		</template>
 		{{ t('text', 'Formatting help') }}
-	</NcActionButton>
+	</NextcloudVueNcActionButton>
 </template>
 
 <script>
 import { defineComponent } from 'vue'
-import { NcActionButton } from '@nextcloud/vue'
+import { NcActionButton as NextcloudVueNcActionButton } from '@nextcloud/vue'
 import { Help } from '../icons.js'
 
 export default defineComponent({
-	name: 'ActionFormattingHelp',
+	// This component is used as a direct child of NcActions.
+	// Even if it actually renders NcActionButton, NcActions cannot see it due to rendering limitations in Vue.
+	// Though it works in general, NcActions doesn't handle it correctly. See NcActions docs for details.
+	// Hotfix - rename the component to NcActionButton because it represents and renders it.
+	// eslint-disable-next-line vue/match-component-file-name
+	name: 'NcActionButton',
 	components: {
-		NcActionButton,
+		NextcloudVueNcActionButton,
 		Help,
 	},
 })

src/components/Menu/ActionInsertLink.vue

@@ -19,7 +19,6 @@
   -->
 <template>
 	<NcActions class="entry-action entry-action__insert-link"
-		aria-haspopup
 		:title="actionEntry.label"
 		:aria-label="actionEntry.label"
 		:class="activeClass"
@@ -31,8 +30,7 @@
 		<template #icon>
 			<component :is="icon"
 				:name="actionEntry.label"
-				:aria-label="actionEntry.label"
-				aria-haspopup />
+				:aria-label="actionEntry.label" />
 		</template>
 		<NcActionButton v-if="state.active"
 			:data-text-action-entry="`${actionEntry.key}-remove`"

src/components/Menu/ActionList.vue

@@ -2,6 +2,7 @@
   - @copyright Copyright (c) 2022 Vinicius Reis <vinicius@nextcloud.com>
   -
   - @author Vinicius Reis <vinicius@nextcloud.com>
+  - @author Grigorii K. Shartsev <me@shgk.me>
   -
   - @license GNU AGPL version 3 or any later version
   -
@@ -23,37 +24,35 @@
 <template>
 	<NcActions :title="tooltip"
 		class="entry-list-action entry-action"
-		role="menu"
 		v-bind="state"
 		:container="menuIDSelector"
-		:aria-label="actionEntry.label"
-		:aria-pressed="state.active"
+		:aria-label="labelWithSelected"
 		:type="state.active ? 'primary': 'tertiary'"
-		:aria-activedescendant="currentChild ? `${$menuID}-child-${currentChild.key}` : null"
 		:force-menu="true"
-		:name="actionEntry.label"
 		:data-text-action-entry="actionEntry.key"
 		:data-text-action-active="activeKey"
 		@update:open="onOpenChange">
 		<template #icon>
 			<component :is="icon" :key="iconKey" />
 		</template>
-		<ActionSingle v-for="child in children"
-			:id="`${$menuID}-child-${child.key}`"
-			:key="`child-${child.key}`"
-			:active="currentChild?.key === child.key"
-			is-item
-			:action-entry="child"
-			v-on="$listeners"
-			@trigged="onTrigger" />
+		<template v-for="child in children">
+			<NcActionSeparator v-if="child.isSeparator" :key="`child-${child.key}`" />
+			<ActionListItem v-else
+				:key="`child-${child.key}`"
+				:active="currentChild?.key === child.key"
+				is-item
+				:action-entry="child"
+				v-on="$listeners"
+				@trigged="onTrigger" />
+		</template>
 		<slot v-bind="{ visible }" name="lastAction" />
 	</NcActions>
 </template>
 
 <script>
-import { NcActions } from '@nextcloud/vue'
+import { NcActions, NcActionSeparator } from '@nextcloud/vue'
 import { BaseActionEntry } from './BaseActionEntry.js'
-import ActionSingle from './ActionSingle.vue'
+import ActionListItem from './ActionListItem.vue'
 import { getIsActive } from './utils.js'
 import { useOutlineStateMixin } from '../Editor/Wrapper.provider.js'
 import useStore from '../../mixins/store.js'
@@ -63,7 +62,8 @@ export default {
 	name: 'ActionList',
 	components: {
 		NcActions,
-		ActionSingle,
+		NcActionSeparator,
+		ActionListItem,
 	},
 	extends: BaseActionEntry,
 	mixins: [useStore, useOutlineStateMixin, useMenuIDMixin],
@@ -110,6 +110,17 @@ export default {
 					: visible
 			})
 		},
+		labelWithSelected() {
+			if (this.currentChild) {
+				// TRANSLATORS: examples - Headings, "Heading 1" is selected - Callouts, "Info" is selected
+				return t('text', '{menuItemName}, "{selectedSubMenuItemName}" is selected', {
+					menuItemName: this.actionEntry.label,
+					selectedSubMenuItemName: this.currentChild.label,
+				})
+			}
+
+			return this.actionEntry.label
+		},
 	},
 	methods: {
 		onOpenChange(val) {

src/components/Menu/ActionListItem.vue

@@ -0,0 +1,82 @@
+<!--
+  - @copyright Copyright (c) 2022 Vinicius Reis <vinicius@nextcloud.com>
+  -
+  - @author Vinicius Reis <vinicius@nextcloud.com>
+  - @author Grigorii K. Shartsev <me@shgk.me>
+  -
+  - @license GNU AGPL version 3 or any later version
+  -
+  - This program is free software: you can redistribute it and/or modify
+  - it under the terms of the GNU Affero General Public License as
+  - published by the Free Software Foundation, either version 3 of the
+  - License, or (at your option) any later version.
+  -
+  - This program is distributed in the hope that it will be useful,
+  - but WITHOUT ANY WARRANTY; without even the implied warranty of
+  - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+  - GNU Affero General Public License for more details.
+  -
+  - You should have received a copy of the GNU Affero General Public License
+  - along with this program. If not, see <http://www.gnu.org/licenses/>.
+  -
+  -->
+
+<template>
+	<NextcloudVueNcActionButton class="entry-single-action entry-action entry-action-item"
+		:class="state.class"
+		:disabled="state.disabled"
+		:aria-keyshortcuts="keyshortcuts || undefined"
+		:data-text-action-entry="actionEntry.key"
+		:type="state.type"
+		:model-value="state.type !== 'button' ? state.active : undefined"
+		close-after-click
+		v-on="$listeners"
+		@click="runAction">
+		<template #icon>
+			<component :is="icon" />
+		</template>
+		{{ label }}
+	</NextcloudVueNcActionButton>
+</template>
+
+<script>
+import { NcActionButton as NextcloudVueNcActionButton } from '@nextcloud/vue'
+import { BaseActionEntry } from './BaseActionEntry.js'
+
+export default {
+	// This component is used as a direct child of NcActions.
+	// Even if it actually renders NcActionButton, NcActions cannot see it due to rendering limitations in Vue.
+	// Though it works in general, NcActions doesn't handle it correctly. See NcActions docs for details.
+	// Hotfix - rename the component to NcActionButton because it represents and renders it.
+	// eslint-disable-next-line vue/match-component-file-name
+	name: 'NcActionButton',
+
+	components: {
+		NextcloudVueNcActionButton,
+	},
+
+	extends: BaseActionEntry,
+
+	mounted() {
+		this.$editor.on('transaction', () => this.updateState())
+	},
+
+	methods: {
+		runAction() {
+			const { actionEntry } = this
+
+			if (actionEntry.click) {
+				actionEntry.click(this)
+			} else {
+				// Some actions run themselves.
+				// others still need to have .run() called upon them.
+				actionEntry.action(this.$editor.chain().focus())?.run()
+			}
+
+			this.$nextTick(() => {
+				this.$emit('trigged', { ...actionEntry })
+			})
+		},
+	},
+}
+</script>

src/components/Menu/ActionSingle.vue

@@ -2,6 +2,7 @@
   - @copyright Copyright (c) 2022 Vinicius Reis <vinicius@nextcloud.com>
   -
   - @author Vinicius Reis <vinicius@nextcloud.com>
+  - @author Grigorii K. Shartsev <me@shgk.me>
   -
   - @license GNU AGPL version 3 or any later version
   -
@@ -20,57 +21,52 @@
   -
   -->
 
+<template>
+	<NcButton class="entry-single-action entry-action"
+		:class="state.class"
+		:disabled="state.disabled"
+		:aria-keyshortcuts="keyshortcuts || undefined"
+		:data-text-action-entry="actionEntry.key"
+		:aria-label="label"
+		:title="tooltip"
+		type="tertiary"
+		:pressed="state.type !== 'button' ? state.active : undefined"
+		v-on="$listeners"
+		@click="runAction">
+		<template #icon>
+			<component :is="icon" />
+		</template>
+
+		<template v-if="actionEntry.forceLabel" #default>
+			{{ label }}
+		</template>
+	</NcButton>
+</template>
+
 <script>
-import { NcButton, NcActionButton } from '@nextcloud/vue'
+import { NcButton } from '@nextcloud/vue'
 import { BaseActionEntry } from './BaseActionEntry.js'
 
 export default {
 	name: 'ActionSingle',
+
+	components: {
+		NcButton,
+	},
+
 	extends: BaseActionEntry,
+
 	props: {
 		isItem: {
 			type: Boolean,
 			default: false,
 		},
 	},
-	computed: {
-		component() {
-			// Button and NcActionButton shares styles and behaviours
-			// to keep it simple, this component handle the small differences
-			return this.isItem
-				? NcActionButton
-				: NcButton
-		},
-		bindState() {
-			const { keyshortcuts } = this
-
-			const state = {
-				...this.state,
-				ariaLabel: this.label,
-			}
-
-			state.class = {
-				...state.class,
-				// inject a extra class
-				'entry-action-item': this.isItem,
-			}
-
-			if (keyshortcuts) {
-				state['aria-keyshortcuts'] = keyshortcuts
-			}
-
-			// item list behaviour
-			if (this.isItem) {
-				state.closeAfterClick = true
-			}
-
-			return state
-		},
-	},
 
 	mounted() {
 		this.$editor.on('transaction', () => this.updateState())
 	},
+
 	methods: {
 		runAction() {
 			const { actionEntry } = this
@@ -88,50 +84,5 @@ export default {
 			})
 		},
 	},
-
-	render(h) {
-		const {
-			$listeners,
-			actionEntry,
-			bindState,
-			component,
-			icon,
-			isItem,
-			runAction,
-			tooltip,
-			label,
-		} = this
-
-		const { class: classes, ...attrs } = bindState
-
-		const children = [h(icon, { slot: 'icon' })]
-
-		// do not use title if is a item of action list
-		const title = isItem ? undefined : tooltip
-
-		if (isItem || actionEntry.forceLabel) {
-			// add label
-			children.push(label)
-		}
-
-		return h(component, {
-			staticClass: 'entry-single-action entry-action',
-			class: classes,
-			attrs: {
-				title,
-				type: attrs.active ? 'primary' : 'tertiary',
-				role: 'menuitem',
-				'data-text-action-entry': actionEntry.key,
-				...attrs,
-			},
-			props: isItem
-				? { pressed: attrs.active }
-				: {},
-			on: {
-				...$listeners,
-				click: runAction,
-			},
-		}, children)
-	},
 }
 </script>

src/components/Menu/EmojiPickerAction.vue

@@ -28,8 +28,7 @@
 			<NcButton class="entry-action__button"
 				role="menu"
 				:title="actionEntry.label"
-				:aria-label="actionEntry.label"
-				:aria-haspopup="true">
+				:aria-label="actionEntry.label">
 				<template #icon>
 					<component :is="icon" />
 				</template>

src/components/Menu/MenuBar.vue

@@ -3,6 +3,7 @@
   -
   - @author Vinicius Reis <vinicius@nextcloud.com>
   - @author Julius Härtl <jus@bitgrid.net>
+  - @author Grigorii K. Shartsev <me@shgk.me>
   -
   - @license AGPL-3.0-or-later
   -
@@ -161,12 +162,23 @@ export default {
 			return list
 		},
 		hiddenEntries() {
-			const entries = this.entries.filter(({ priority }) => {
+			const remainingEntries = this.entries.filter(({ priority }) => {
 				// reverse logic from visibleEntries
 				return priority !== undefined && priority > this.iconsLimit
-			}).reduce((acc, entry) => {
+			})
+			const entries = remainingEntries.reduce((acc, entry, index) => {
 				// If entry has children, merge them into list. Otherwise keep entry itself.
 				const children = entry.children ?? [entry]
+				// If this block has menu entries, it should be separated for better visibility and a11y (menu item radio grouping)
+				if (children.length > 1) {
+					const hasPreviousItem = acc.length && !acc.at(-1).isSeparator
+					const separatorBefore = hasPreviousItem ? [{ key: `separator-before-${entry.id}`, isSeparator: true }] : []
+
+					const hasNextItem = index !== remainingEntries.length - 1
+					const separatorAfter = hasNextItem ? [{ key: `separator-after-${entry.id}`, isSeparator: true }] : []
+
+					return [...acc, ...separatorBefore, ...children, ...separatorAfter]
+				}
 				return [...acc, ...children]
 			}, [])
 

src/components/Menu/entries.js

@@ -2,6 +2,7 @@
  * @copyright Copyright (c) 2019 Julius Härtl <jus@bitgrid.net>
  *
  * @author Julius Härtl <jus@bitgrid.net>
+ * @author Grigorii K. Shartsev <me@shgk.me>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -194,6 +195,13 @@ export default [
 					return command.toggleHeading({ level: 6 })
 				},
 			},
+			{
+				key: 'headings-separator',
+				isSeparator: true,
+				visible: ({ $outlineState }) => {
+					return $outlineState.enable
+				},
+			},
 			{
 				key: 'outline',
 				icon: FormatListBulleted,

src/components/Menu/utils.js

@@ -2,6 +2,7 @@
  * @copyright Copyright (c) 2022 Vinicius Reis <vinicius@nextcloud.com>
  *
  * @author Vinicius Reis <vinicius@nextcloud.com>
+ * @author Grigorii K. Shartsev <me@shgk.me>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -68,6 +69,24 @@ const getIsActive = ({ isActive }, $editor) => {
 	return $editor.isActive(...args)
 }
 
+const getType = (actionEntry) => {
+	// isActive stores the value changing on active state change (on click)
+
+	// If it is an array, the button is one of the list of alternative values for a specific option
+	// Like ['heading', { level: 1 }]
+	if (Array.isArray(actionEntry.isActive)) {
+		return 'radio'
+	}
+
+	// If it is a string, it toggles a specific option like a checkbox
+	if (typeof actionEntry.isActive === 'string') {
+		return 'checkbox'
+	}
+
+	// Otherwise it is just a button
+	return 'button'
+}
+
 const getActionState = (actionEntry, $editor) => {
 	const active = getIsActive(actionEntry, $editor)
 
@@ -75,7 +94,7 @@ const getActionState = (actionEntry, $editor) => {
 		disabled: isDisabled(actionEntry, $editor),
 		class: getEntryClasses(actionEntry, active),
 		active,
-		'aria-selected': active,
+		type: getType(actionEntry),
 	}
 }
 

src/nodes/Table/TableHeaderView.vue

@@ -2,6 +2,7 @@
   - @copyright Copyright (c) 2022 Max <max@nextcloud.com>
   -
   - @author Max <max@nextcloud.com>
+  - @author Grigorii K. Shartsev <me@shgk.me>
   -
   - @license GNU AGPL version 3 or any later version
   -
@@ -27,9 +28,12 @@
 			<NcActions v-if="editor.isEditable"
 				ref="menu"
 				data-text-table-actions="header">
-				<InlineActionsContainer>
+				<NcActionButtonGroup>
 					<NcActionButton data-text-table-action="align-column-left"
 						:aria-label="t('text', 'Left align column')"
+						type="radio"
+						value="left"
+						:model-value="node.attrs.textAlign"
 						@click="alignLeft">
 						<template #icon>
 							<AlignHorizontalLeft />
@@ -37,6 +41,9 @@
 					</NcActionButton>
 					<NcActionButton data-text-table-action="align-column-center"
 						:aria-label="t('text', 'Center align column')"
+						type="radio"
+						value="center"
+						:model-value="node.attrs.textAlign"
 						@click="alignCenter">
 						<template #icon>
 							<AlignHorizontalCenter />
@@ -44,12 +51,15 @@
 					</NcActionButton>
 					<NcActionButton data-text-table-action="align-column-right"
 						:aria-label="t('text', 'Right align column')"
+						type="radio"
+						value="right"
+						:model-value="node.attrs.textAlign"
 						@click="alignRight">
 						<template #icon>
 							<AlignHorizontalRight />
 						</template>
 					</NcActionButton>
-				</InlineActionsContainer>
+				</NcActionButtonGroup>
 				<NcActionButton data-text-table-action="add-column-before"
 					close-after-click
 					@click="addColumnBefore">
@@ -81,8 +91,7 @@
 
 <script>
 import { NodeViewWrapper, NodeViewContent } from '@tiptap/vue-2'
-import { NcActions, NcActionButton } from '@nextcloud/vue'
-import InlineActionsContainer from '../../components/InlineActionsContainer.vue'
+import { NcActions, NcActionButton, NcActionButtonGroup } from '@nextcloud/vue'
 import {
 	AlignHorizontalCenter,
 	AlignHorizontalLeft,
@@ -99,8 +108,8 @@ export default {
 		AlignHorizontalLeft,
 		AlignHorizontalRight,
 		Delete,
-		InlineActionsContainer,
 		NcActionButton,
+		NcActionButtonGroup,
 		NcActions,
 		NodeViewWrapper,
 		NodeViewContent,