Skip to content

DataViewsPicker Grid layout: Support hiding the title #71865

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

Merged
andrewserong merged 3 commits into from
Sep 25, 2025
Merged

DataViewsPicker Grid layout: Support hiding the title #71865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc952ff
DataViewsPicker Grid layout: Support hiding the title
andrewserong Sep 24, 2025
564916e
Add changelog entry
andrewserong Sep 24, 2025
de85bbf
Add aria-label to the composite item when a title field is provided
andrewserong Sep 25, 2025
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
1 change: 1 addition & 0 deletions packages/dataviews/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
### Bug Fixes

- DataViews: keep non-hideable fields out of the hidden-fields list when they’re already invisible. [#71729](https://github.com/WordPress/gutenberg/pull/71729/)
- DataViewsPicker: Hide the space reserved for the title when the title is hidden. [#71865](https://github.com/WordPress/gutenberg/pull/71865)

### Enhancements

Expand Down
23 changes: 15 additions & 8 deletions packages/dataviews/src/dataviews-layouts/picker-grid/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ function GridItem< Item >( {

return (
<Composite.Item
aria-label={
titleField
? titleField.getValue( { item } ) || __( '(no title)' )
: undefined
}
Comment on lines +92 to +96
Copy link
Contributor Author

@andrewserong andrewserong Sep 25, 2025

Choose a reason for hiding this comment

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

Just explaining the use of a ternary here and falling back to undefined. For usage where a titleField itself is not provided, falling back to undefined will allow voice over to read from text within the component.

For example, if someone uses the DataViews and does not provide a title field, but does provide a description field, then the description field will be read out.

So the approach here uses a best-effort to provide a title as the label if a title field is set, otherwise it defers to the screen reader utility to make do with what's available.

key={ id }
render={ ( { children, ...props } ) => (
<VStack spacing={ 0 } children={ children } { ...props } />
Expand Down Expand Up @@ -130,14 +135,16 @@ function GridItem< Item >( {
tabIndex={ -1 }
/>
) }
<HStack
justify="space-between"
className="dataviews-view-picker-grid__title-actions"
>
<div className="dataviews-view-picker-grid__title-field dataviews-title-field">
{ renderedTitleField }
</div>
</HStack>
{ showTitle && (
<HStack
justify="space-between"
className="dataviews-view-picker-grid__title-actions"
>
<div className="dataviews-view-picker-grid__title-field dataviews-title-field">
{ renderedTitleField }
Copy link
Contributor

@talldan talldan Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I approved too quickly. A problem is that the items are all now unlabelled when using a screenreader if the visible label is hidden.

Copy link
Contributor

@tellthemachines tellthemachines Sep 24, 2025

Choose a reason for hiding this comment

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

Are we handling that in the original grid view? If not it might be worth fixing for both?

Copy link
Contributor

@talldan talldan Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like the checkbox gets a proper label, but the click wrapper around the image is missing a label.

I think this is the relevant code:

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

Lines 117 to 130 in 9f20f50

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

Having said that, in the story the image click wrappers are missing labels even when there's a visible title, so not sure what's going on there. I guess that code is a bit buggy.

Copy link
Contributor Author

@andrewserong andrewserong Sep 24, 2025

Choose a reason for hiding this comment

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

Thanks for flagging! I think the picker might want a slightly different approach to the standard Grid... a natural place for the label to go in the picker could be to add an aria-label to the Composite.Item if the title is hidden. I'll have a play around with that.

Copy link
Contributor Author

@andrewserong andrewserong Sep 25, 2025

Choose a reason for hiding this comment

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

Updated in de85bbf, seems to be working pretty well in my testing using macOS voiceover 👍

</div>
</HStack>
) }
<VStack spacing={ 1 }>
{ showDescription && descriptionField?.render && (
<descriptionField.render
Expand Down
Loading