Skip to content

Commit

Permalink
DataViews: set proper semantics for dropdown items (#56676)
Browse files Browse the repository at this point in the history
  • Loading branch information
oandregal authored Dec 1, 2023
1 parent 08eb841 commit f987124
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 16 deletions.
3 changes: 2 additions & 1 deletion packages/edit-site/src/components/dataviews/add-filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -95,7 +97,6 @@ export default function AddFilter( { fields, view, onChangeView } ) {
],
} ) );
} }
role="menuitemcheckbox"
>
{ element.label }
</DropdownMenuItem>
Expand Down
12 changes: 7 additions & 5 deletions packages/edit-site/src/components/dataviews/view-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } />
Expand All @@ -66,8 +68,6 @@ function ViewTypeMenu( { view, onChangeView, supportedLayouts } ) {
type: availableView.type,
} );
} }
// TODO: check about role and a11y.
role="menuitemcheckbox"
>
{ availableView.label }
</DropdownMenuItem>
Expand Down Expand Up @@ -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 } />
}
Expand All @@ -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>
Expand Down Expand Up @@ -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 } />
Expand All @@ -158,7 +159,6 @@ function FieldsVisibilityMenu( { view, onChangeView, fields } ) {
: [ ...view.hiddenFields, field.id ],
} );
} }
role="menuitemcheckbox"
>
{ field.header }
</DropdownMenuItem>
Expand Down Expand Up @@ -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 } />
Expand Down
8 changes: 8 additions & 0 deletions packages/edit-site/src/components/dataviews/view-table.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 && (
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -172,6 +178,8 @@ function HeaderMenu( { dataView, header } ) {
return (
<DropdownMenuItem
key={ element.value }
role="menuitemradio"
aria-checked={ isActive }
suffix={
isActive && <Icon icon={ check } />
}
Expand Down
25 changes: 15 additions & 10 deletions test/e2e/specs/site-editor/new-templates-list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 } ) => {
Expand All @@ -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' );

Expand All @@ -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();
Expand Down

0 comments on commit f987124

Please sign in to comment.