[a11y][Dataviews] Fix: use span instead of heading for the template titles #56785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jorgefilipecosta
added
[Type] Enhancement
jorgefilipecosta
requested review from
ntsekouras and
afercia
and removed request for
ntsekouras
|
Size Change: +53 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 7d85b31. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7103775271
|
…itles in the table view.
jorgefilipecosta
force-pushed
the
fix/a11y-issue-heading-use-for-template-titles-in-table-view
branch
from
31fb62f to
7d85b31
Compare
jorgefilipecosta
deleted the
fix/a11y-issue-heading-use-for-template-titles-in-table-view
branch
ntsekouras
reviewed
Dec 7, 2023
| @@ -83,7 +83,7 @@ function TemplateTitle( { item } ) { | |||
| const { isCustomized } = useAddedBy( item.type, item.id ); | |||
| return ( | |||
| <VStack spacing={ 1 }> | |||
| <Heading as="h3" level={ 5 }> | |||
| <View as="h3"> | |||
There was a problem hiding this comment.
This still renders an h3 and additionally has different styles from before.
A couple of things here:
- can you provide some context about why this should change? I think it follows the headings flow(above is
h2). - If we make any changes like that, they should be reflected to both lists(pages and templates).
--cc @andrewhayward
There was a problem hiding this comment.
Hi @ntsekouras thank you for the review.
This still renders an h3 and additionally has different styles from before.
I missed to stage my changes and committed an in development try 😅 It was fixed at #56955.
can you provide some context about why this should change? I think it follows the headings flow(above is h2).
I don't think it makes sense to have headings inside a table cell. Headings are typically used as titles for a section, and inside a cell where the title is located, there is usually no other content. I've checked user interfaces like Google Drive and Github, and they don't use headings for document titles of document name in the list of documents or titles in a list of issues or pull requests. Additionally, the current PHP-based page list on WordPress also does not use headings for page titles. Therefore, it seems that using headings is not a common practice. We should keep the current approach unless we see that headings would bring any benefit.
If we make any changes like that, they should be reflected to both lists(pages and templates).
The PR for the lists is available at #56956.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes an issue referred by @afercia where the dataviews table view uses a h3 heading in the titles cells.
This PR fixes the issue for the template view
/wp-admin/site-editor.php?path=%2Fwp_template%2Fall.Testing
Verify the template UI looks the same as in the trunk.
Verify the template's title is now a span and not a heading.