Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 10, 2024
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e64926
[a11y] Fix: Media button on the page view grid does not have an acces…
jorgefilipecosta Dec 6, 2024
1369e5f
feedback
jorgefilipecosta Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[a11y] Fix: Media button on the page view grid does not have an acces… 
…sible name.
  • Loading branch information
@jorgefilipecosta
jorgefilipecosta committed Dec 10, 2024
commit 7e649266e8aedd7d6c138f0821d382fa7e108a6c
25 changes: 23 additions & 2 deletions packages/dataviews/src/dataviews-layouts/grid/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
FlexItem,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useInstanceId } from '@wordpress/compose';

/**
* Internal dependencies
Expand Down Expand Up @@ -66,6 +67,7 @@ function GridItem< Item >( {
const { showTitle = true, showMedia = true, showDescription = true } = view;
const hasBulkAction = useHasAPossibleBulkAction( actions, item );
const id = getItemId( item );
const instanceId = useInstanceId( GridItem );
const isSelected = selection.includes( id );
const renderedMediaField = mediaField?.render ? (
<mediaField.render item={ item } />
Expand All @@ -89,6 +91,21 @@ function GridItem< Item >( {
className: 'dataviews-view-grid__title-field dataviews-title-field',
} );

let mediaA11yProps;
let titleA11yProps;
if ( isItemClickable( item ) && onClickItem ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @oandregal, I applied the suggestion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

mediaA11yProps = renderedTitleField
? {
'aria-labelledby': `dataviews-view-grid__title-field-${ instanceId }`,
}
: {
'aria-label': __( 'Navigate to item' ),
};
titleA11yProps = {
id: `dataviews-view-grid__title-field-${ instanceId }`,
};
}

return (
<VStack
spacing={ 0 }
Expand All @@ -112,7 +129,9 @@ function GridItem< Item >( {
} }
>
{ showMedia && renderedMediaField && (
<div { ...clickableMediaItemProps }>{ renderedMediaField }</div>
<div { ...clickableMediaItemProps } { ...mediaA11yProps }>
{ renderedMediaField }
</div>
) }
{ showMedia && renderedMediaField && (
<DataViewsSelectionCheckbox
Expand All @@ -128,7 +147,9 @@ function GridItem< Item >( {
justify="space-between"
className="dataviews-view-grid__title-actions"
>
<div { ...clickableTitleItemProps }>{ renderedTitleField }</div>
<div { ...clickableTitleItemProps } { ...titleA11yProps }>
{ renderedTitleField }
</div>
<ItemActions item={ item } actions={ actions } isCompact />
</HStack>
<VStack spacing={ 1 }>
Expand Down
Loading