DataViews: Custom empty elements are no longer wrapped in <p> tags to improve accessibility
#71561
Conversation
The custom empty element can be anything arbitrary, including divs and headings. These elements are not semantically allowed to be children of paragraphs. The spinner continues to be wrapped in a paragraph. This is allowed semantically (the spinner is just an <svg> element), it keeps the vertical margins of the spinner consistent with the default empty message, and it is consistent with the spinners which show when the dataviews are loading more data in infinite scroll mode.
p-jackson
force-pushed
the
fix/a11y-dataviews-empty-message
branch
from
f5893be to
8fec89d
Compare
p-jackson
added
[Type] Bug
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
p-jackson
requested review from
andrewserong,
arthur791004,
simison and
youknowriad
|
Flaky tests detected in 8fec89d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17579295497
|
| > | ||
| <p>{ isLoading ? <Spinner /> : empty }</p> | ||
| { isLoading ? ( | ||
| <p> |
There was a problem hiding this comment.
Do we need "p" around the spinner?
There was a problem hiding this comment.
Without the <p> it can cause a jump in the dataview content when the view flips from the loading state to the empty state, because the vertical margin from the <p> appears all of a sudden.
I'm being super extra paranoid about this particular case because the default spinner and empty message elements are what appear in the Gutenberg editor. And I definitely do not want to introduce visual changes to the Gutenberg editor while working on this standalone component.
There was a problem hiding this comment.
And I definitely do not want to introduce visual changes to the Gutenberg editor while working on this standalone component.
I like this reasoning 👍
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
Not sure about needing the "p" around spinners but other than that. This LGTM
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for the ping, LGTM too!
What?
Fixes an accessibility error when providing a custom empty element.
Why?
The custom empty element can be anything arbitrary, including divs and headings. These elements are not semantically allowed to be children of paragraphs.
Raised here #70867 (comment)
How?
If a dataviews user wants to have a wrapping paragraph for their custom message, they now need to provide their own. The margin provided by the paragraph element doesn't usually do anything, since the message is usually centred in a full height dataview in Gutenberg.
The spinner continues to be wrapped in a paragraph. This is allowed semantically (the spinner is just an element), it keeps the vertical margins of the spinner consistent with the default empty message, and it is consistent with the spinners which show when the dataviews are loading more data in infinite scroll mode.
Testing Instructions
There should be no visual changes.
You can check the custom messages in storybook. First run
npm run storybook:dev. Then check out the custom empty message stories.DataViews : Custom Empty
DataViews : Free Composition
Testing Instructions for Keyboard
No keyboard changes.