From f987124f44e5d22d64a8e232759f5a213c329bc8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com>
Date: Fri, 1 Dec 2023 12:42:52 +0100
Subject: [PATCH] DataViews: set proper semantics for dropdown items (#56676)

---
 .../src/components/dataviews/add-filter.js    |  3 ++-
 .../src/components/dataviews/view-actions.js  | 12 +++++----
 .../src/components/dataviews/view-table.js    |  8 ++++++
 .../site-editor/new-templates-list.spec.js    | 25 +++++++++++--------
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/packages/edit-site/src/components/dataviews/add-filter.js b/packages/edit-site/src/components/dataviews/add-filter.js
index 7999ff413f96cd..4dbbf7dc0938d9 100644
--- a/packages/edit-site/src/components/dataviews/add-filter.js
+++ b/packages/edit-site/src/components/dataviews/add-filter.js
@@ -81,6 +81,8 @@ export default function AddFilter( { fields, view, onChangeView } ) {
 						{ filter.elements.map( ( element ) => (
 							<DropdownMenuItem
 								key={ element.value }
+								role="menuitemradio"
+								aria-checked={ false }
 								onSelect={ () => {
 									onChangeView( ( currentView ) => ( {
 										...currentView,
@@ -95,7 +97,6 @@ export default function AddFilter( { fields, view, onChangeView } ) {
 										],
 									} ) );
 								} }
-								role="menuitemcheckbox"
 							>
 								{ element.label }
 							</DropdownMenuItem>
diff --git a/packages/edit-site/src/components/dataviews/view-actions.js b/packages/edit-site/src/components/dataviews/view-actions.js
index 80d694691d505a..28acd2bdb882d6 100644
--- a/packages/edit-site/src/components/dataviews/view-actions.js
+++ b/packages/edit-site/src/components/dataviews/view-actions.js
@@ -53,6 +53,8 @@ function ViewTypeMenu( { view, onChangeView, supportedLayouts } ) {
 				return (
 					<DropdownMenuItem
 						key={ availableView.type }
+						role="menuitemradio"
+						aria-checked={ availableView.id === view.type }
 						prefix={
 							availableView.type === view.type && (
 								<Icon icon={ check } />
@@ -66,8 +68,6 @@ function ViewTypeMenu( { view, onChangeView, supportedLayouts } ) {
 								type: availableView.type,
 							} );
 						} }
-						// TODO: check about role and a11y.
-						role="menuitemcheckbox"
 					>
 						{ availableView.label }
 					</DropdownMenuItem>
@@ -99,6 +99,8 @@ function PageSizeMenu( { view, onChangeView } ) {
 				return (
 					<DropdownMenuItem
 						key={ size }
+						role="menuitemradio"
+						aria-checked={ view.perPage === size }
 						prefix={
 							view.perPage === size && <Icon icon={ check } />
 						}
@@ -107,8 +109,6 @@ function PageSizeMenu( { view, onChangeView } ) {
 							event.preventDefault();
 							onChangeView( { ...view, perPage: size, page: 1 } );
 						} }
-						// TODO: check about role and a11y.
-						role="menuitemcheckbox"
 					>
 						{ size }
 					</DropdownMenuItem>
@@ -140,6 +140,7 @@ function FieldsVisibilityMenu( { view, onChangeView, fields } ) {
 				return (
 					<DropdownMenuItem
 						key={ field.id }
+						role="menuitemcheckbox"
 						prefix={
 							! view.hiddenFields?.includes( field.id ) && (
 								<Icon icon={ check } />
@@ -158,7 +159,6 @@ function FieldsVisibilityMenu( { view, onChangeView, fields } ) {
 									: [ ...view.hiddenFields, field.id ],
 							} );
 						} }
-						role="menuitemcheckbox"
 					>
 						{ field.header }
 					</DropdownMenuItem>
@@ -221,6 +221,8 @@ function SortMenu( { fields, view, onChangeView } ) {
 								return (
 									<DropdownMenuItem
 										key={ direction }
+										role="menuitemradio"
+										aria-checked={ isActive }
 										prefix={ <Icon icon={ info.icon } /> }
 										suffix={
 											isActive && <Icon icon={ check } />
diff --git a/packages/edit-site/src/components/dataviews/view-table.js b/packages/edit-site/src/components/dataviews/view-table.js
index 58d995e22e1d2c..60b584b7f10261 100644
--- a/packages/edit-site/src/components/dataviews/view-table.js
+++ b/packages/edit-site/src/components/dataviews/view-table.js
@@ -98,6 +98,10 @@ function HeaderMenu( { dataView, header } ) {
 							( [ direction, info ] ) => (
 								<DropdownMenuItem
 									key={ direction }
+									role="menuitemradio"
+									aria-checked={
+										sortedDirection === direction
+									}
 									prefix={ <Icon icon={ info.icon } /> }
 									suffix={
 										sortedDirection === direction && (
@@ -126,6 +130,8 @@ function HeaderMenu( { dataView, header } ) {
 				) }
 				{ isHidable && (
 					<DropdownMenuItem
+						role="menuitemradio"
+						aria-checked={ ! header.column.getIsVisible() }
 						prefix={ <Icon icon={ unseen } /> }
 						onSelect={ ( event ) => {
 							event.preventDefault();
@@ -172,6 +178,8 @@ function HeaderMenu( { dataView, header } ) {
 								return (
 									<DropdownMenuItem
 										key={ element.value }
+										role="menuitemradio"
+										aria-checked={ isActive }
 										suffix={
 											isActive && <Icon icon={ check } />
 										}
diff --git a/test/e2e/specs/site-editor/new-templates-list.spec.js b/test/e2e/specs/site-editor/new-templates-list.spec.js
index be6008080200c8..b28362cd2c66d3 100644
--- a/test/e2e/specs/site-editor/new-templates-list.spec.js
+++ b/test/e2e/specs/site-editor/new-templates-list.spec.js
@@ -14,13 +14,20 @@ test.describe( 'Templates', () => {
 		await Promise.all( [
 			requestUtils.activateTheme( 'twentytwentyone' ),
 			requestUtils.deactivatePlugin( 'gutenberg-test-dataviews' ),
+			requestUtils.deleteAllTemplates( 'wp_template' ),
 		] );
 	} );
 	test( 'Sorting', async ( { admin, page } ) => {
 		await admin.visitSiteEditor( { path: '/wp_template/all' } );
 		// Descending by title.
-		await page.getByRole( 'button', { name: 'Template' } ).click();
-		await page.getByRole( 'menuitem', { name: 'Sort descending' } ).click();
+		await page
+			.getByRole( 'button', { name: 'Template', exact: true } )
+			.click();
+		await page
+			.getByRole( 'menuitemradio', {
+				name: 'Sort descending',
+			} )
+			.click();
 		const firstTitle = page
 			.getByRole( 'region', {
 				name: 'Template',
@@ -33,7 +40,9 @@ test.describe( 'Templates', () => {
 			.first();
 		await expect( firstTitle ).toHaveText( 'Tag Archives' );
 		// Ascending by title.
-		await page.getByRole( 'menuitem', { name: 'Sort ascending' } ).click();
+		await page
+			.getByRole( 'menuitemradio', { name: 'Sort ascending' } )
+			.click();
 		await expect( firstTitle ).toHaveText( 'Category Archives' );
 	} );
 	test( 'Filtering', async ( { requestUtils, admin, page } ) => {
@@ -57,7 +66,7 @@ test.describe( 'Templates', () => {
 		// Filter by author.
 		await page.getByRole( 'button', { name: 'Add filter' } ).click();
 		await page.getByRole( 'menuitem', { name: 'Author' } ).hover();
-		await page.getByRole( 'menuitemcheckbox', { name: 'admin' } ).click();
+		await page.getByRole( 'menuitemradio', { name: 'admin' } ).click();
 		await expect( titles ).toHaveCount( 1 );
 		await expect( titles.first() ).toHaveText( 'Date Archives' );
 
@@ -68,17 +77,13 @@ test.describe( 'Templates', () => {
 		await expect( titles ).toHaveCount( 3 );
 		await page.getByRole( 'button', { name: 'Add filter' } ).click();
 		await page.getByRole( 'menuitem', { name: 'Author' } ).hover();
-		await page
-			.getByRole( 'menuitemcheckbox', { name: 'Emptytheme' } )
-			.click();
+		await page.getByRole( 'menuitemradio', { name: 'Emptytheme' } ).click();
 		await expect( titles ).toHaveCount( 2 );
-
-		await requestUtils.deleteAllTemplates( 'wp_template' );
 	} );
 	test( 'Field visibility', async ( { admin, page } ) => {
 		await admin.visitSiteEditor( { path: '/wp_template/all' } );
 		await page.getByRole( 'button', { name: 'Description' } ).click();
-		await page.getByRole( 'menuitem', { name: 'Hide' } ).click();
+		await page.getByRole( 'menuitemradio', { name: 'Hide' } ).click();
 		await expect(
 			page.getByRole( 'button', { name: 'Description' } )
 		).toBeHidden();