[a11y] Fix: Media button on the page view grid does not have an accessible name. #67690

jorgefilipecosta · 2024-12-06T11:50:15Z

All elements with an aria role of a button, such as the media item on the page's grid, should have a label, either nested inner text, an aria-label, or an aria-labeled by.
This PR fixes the issue by labeling the media field with the title field.

Testing

Navigate to ´wp-admin/site-editor.php?p=%2Fpage&layout=grid´.
With the developer tools inspect the media field (element with class dataviews-view-grid__media--clickable)
Go to the element a11y tab and verify the computed name for the element is correct.

youknowriad · 2024-12-06T13:26:00Z

packages/dataviews/src/dataviews-layouts/utils/get-clickable-item-props.ts

@@ -31,5 +35,7 @@ export default function getClickableItemProps< Item >( {
 				onClickItem( item );
 			}
 		},
+		...( id ? { id } : {} ),
+		...( ariaLabelledby ? { 'aria-labelledby': ariaLabelledby } : {} ),


Why are we adding props and just returning them instead of just adding these props directly to the elements?

Hi @youknowriad, because of a bad night of sleep, I guess 😅 I added the props directly to the elements.

github-actions · 2024-12-06T13:26:13Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

ntsekouras · 2024-12-09T09:27:39Z

packages/dataviews/src/dataviews-layouts/grid/index.tsx

-				<div { ...clickableMediaItemProps }>{ renderedMediaField }</div>
+				<div
+					{ ...clickableMediaItemProps }
+					aria-labelledby={ `dataviews-view-grid__title-field-${ instanceId }` }


Should we add those if the element is not clickable and therefore interactive? 🤔

Yes, as long as we have a role of button we should have a label. In cases where it is not interactive I think we should not have a role of button. But as far as I know in these cases the media button is always clickable, no?

So we probably need to add a label in getClickableItemProps or just make a check here?

I'm also curious why you chose aria-labelledby instead of just label.

I like using the title as a label, as it's the relevant information here. However, the titleField is not mandatory and the consumer doesn't need to provide it (comment this line). In that case, what would be the backup?

Hi @ntsekouras,

So we probably need to add a label in getClickableItemProps or just make a check here?

getClickableItemProps already prevented adding the role of button if the item was not clickable. However, we still added the aria-labelledby attribute even when the item was not clickable, and I have fixed that.

I'm also curious why you chose aria-labelledby instead of just label.

According to accessibility (a11y) guidelines, buttons should ideally be labeled using a visible label from their nested elements. If that's not possible, the next best solution is to use a visible label from an adjacent element and reference it. Only if neither option is available should we consider using an aria-label as a fallback. In this case, we can reference a visible adjacent element, so I opted for that approach.

This method was also technically simpler since the GridItem component does not have a way to compute the title as a simple string. Previously, we had a function called getItemTitle, but it seems that is no longer the case. By referencing the title field directly, I could simplify the implementation and avoid the need for additional props.

Hi @oandregal,

I like using the title as a label, as it's the relevant information here. However, the titleField is not mandatory and the consumer doesn't need to provide it (comment this line). In that case, what would be the backup?

Good point I added a fallback for this case.

oandregal · 2024-12-10T09:44:00Z

Related to this is that the media field doesn't communicate visually that is focused:

Screen.Recording.2024-12-10.at.10.45.21.mov

oandregal · 2024-12-10T09:48:00Z

Another thing is that, unlike other buttons, pressing SPACE when the item is focused doesn't trigger a click even (it does work when pressing ENTER):

Screen.Recording.2024-12-10.at.10.46.54.mov

oandregal · 2024-12-10T09:57:33Z

Before	After

oandregal · 2024-12-10T11:43:25Z

packages/dataviews/src/dataviews-layouts/grid/index.tsx

@@ -89,6 +91,17 @@ function GridItem< Item >( {
 		className: 'dataviews-view-grid__title-field dataviews-title-field',
 	} );

+	let mediaA11yProps = {};
+	if ( isItemClickable( item ) && onClickItem ) {


Can we extend this pattern to also calculate titleA11yProps in this place rather than in the field below? That way things that are connected (aria-labelleby in media + id in title) are more obvious.

Hi @oandregal, I applied the suggestion.

We don't need id in titleA11yProps when the renderedTitleField is null.

jorgefilipecosta · 2024-12-10T12:08:06Z

Related to this is that the media field doesn't communicate visually that is focused:

Screen.Recording.2024-12-10.at.10.45.21.mov

Hi @oandregal I fixed this issue at #67789.

jorgefilipecosta · 2024-12-10T12:34:22Z

Another thing is that, unlike other buttons, pressing SPACE when the item is focused doesn't trigger a click even (it does work when pressing ENTER):

Screen.Recording.2024-12-10.at.10.46.54.mov

Hi @oandregal, I fixed the issue at #67791.

jorgefilipecosta · 2024-12-10T12:34:55Z

Before After

It seems to indicate the label is correctly computed 👍

…sible name.

oandregal

I've left some little comment to address but this is ready, IMO.

github-actions · 2024-12-10T13:42:31Z

Flaky tests detected in 1369e5f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12256984439
📝 Reported issues:

[Flaky Test] Navigates the items list via UP/DOWN arrow keys #64334 in /test/e2e/specs/site-editor/dataviews-list-layout-keyboard.spec.js

…sible name. (WordPress#67690) Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>

jorgefilipecosta added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Dec 6, 2024

jorgefilipecosta requested review from youknowriad, oandregal and ntsekouras December 6, 2024 11:50

youknowriad reviewed Dec 6, 2024

View reviewed changes

jorgefilipecosta force-pushed the fix/media-button-on-pages-grid-does-not-have-an-accessible-name branch from 1b5d556 to f5b2ddd Compare December 6, 2024 14:39

ntsekouras reviewed Dec 9, 2024

View reviewed changes

oandregal added the [Type] Bug An existing feature does not function as intended label Dec 10, 2024

jorgefilipecosta force-pushed the fix/media-button-on-pages-grid-does-not-have-an-accessible-name branch from f5b2ddd to 3b29077 Compare December 10, 2024 10:53

oandregal reviewed Dec 10, 2024

View reviewed changes

jorgefilipecosta mentioned this pull request Dec 10, 2024

[Dataviews] Fix: Media item focus style is not visible on Grid. #67789

Merged

jorgefilipecosta mentioned this pull request Dec 10, 2024

[Dataviews] Fix: Space does not triggers the media button on grid view. #67791

Merged

jorgefilipecosta force-pushed the fix/media-button-on-pages-grid-does-not-have-an-accessible-name branch from 3b29077 to 7e64926 Compare December 10, 2024 12:46

[a11y] Fix: Media button on the page view grid does not have an acces…

7e64926

…sible name.

oandregal approved these changes Dec 10, 2024

View reviewed changes

feedback

1369e5f

jorgefilipecosta merged commit 601ede4 into trunk Dec 10, 2024
60 of 62 checks passed

jorgefilipecosta deleted the fix/media-button-on-pages-grid-does-not-have-an-accessible-name branch December 10, 2024 14:56

github-actions bot added this to the Gutenberg 19.9 milestone Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[a11y] Fix: Media button on the page view grid does not have an accessible name. #67690

[a11y] Fix: Media button on the page view grid does not have an accessible name. #67690

jorgefilipecosta commented Dec 6, 2024 •

edited

Loading

youknowriad Dec 6, 2024

jorgefilipecosta Dec 6, 2024

github-actions bot commented Dec 6, 2024 •

edited

Loading

ntsekouras Dec 9, 2024

jorgefilipecosta Dec 9, 2024

ntsekouras Dec 10, 2024 •

edited

Loading

ntsekouras Dec 10, 2024

oandregal Dec 10, 2024

jorgefilipecosta Dec 10, 2024

jorgefilipecosta Dec 10, 2024

oandregal commented Dec 10, 2024 •

edited

Loading

oandregal commented Dec 10, 2024

oandregal commented Dec 10, 2024

oandregal Dec 10, 2024

jorgefilipecosta Dec 10, 2024

oandregal Dec 10, 2024

jorgefilipecosta commented Dec 10, 2024

jorgefilipecosta commented Dec 10, 2024

jorgefilipecosta commented Dec 10, 2024

oandregal left a comment

github-actions bot commented Dec 10, 2024

[a11y] Fix: Media button on the page view grid does not have an accessible name. #67690

[a11y] Fix: Media button on the page view grid does not have an accessible name. #67690

Conversation

jorgefilipecosta commented Dec 6, 2024 • edited Loading

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntsekouras Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Dec 10, 2024 • edited Loading

oandregal commented Dec 10, 2024

oandregal commented Dec 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgefilipecosta commented Dec 10, 2024

jorgefilipecosta commented Dec 10, 2024

jorgefilipecosta commented Dec 10, 2024

oandregal left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 10, 2024

jorgefilipecosta commented Dec 6, 2024 •

edited

Loading

github-actions bot commented Dec 6, 2024 •

edited

Loading

ntsekouras Dec 10, 2024 •

edited

Loading

oandregal commented Dec 10, 2024 •

edited

Loading