-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Navigation block: Fix some off canvas appender accessibility issues #47047
Conversation
Size Change: +142 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Flaky tests detected in cc7e9ce. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3890046761
|
toggleProps={ { 'aria-describedby': descriptionId } } | ||
/> | ||
<div | ||
className="offcanvas-editor-appender__description" | ||
id={ descriptionId } | ||
> | ||
{ description } | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great idea ❤️
@@ -14,6 +14,10 @@ | |||
} | |||
} | |||
|
|||
.offcanvas-editor-appender__description { | |||
display: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: does not display: none
make the content in accessible to screen readers entirely? I guess the purpose here is that we don't want this text to be read out except in the context of being the description of the inserter? Am I right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think display: none
is fine for aria descriptions. For visually hidden text where a different approach needs to be taken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this with Voiceover and it behaved as expected.
One small concern about the display: none
but in my testing it did announce the description correctly so perhaps this isn't an Issue?
What?
Fixes #47024
See also #47011
Sets the correct
aria-posinset
andaria-setsize
html attributes for the off-canvas editor's appender.Also adds an aria description ('Append at position 3, Level 1') to give some context about where the block will be added.
Why?
The
aria-posinset
andaria-setsize
issue is very minor as screen readers don't announce them. But it's good to set it correctly just in case screenreaders do start announcing them after an update.The aria description is used to replace the fact that screen readers don't announce that information These descriptions could be improved to mention the parent block name (especially in the case of the appender). The trouble is there's no safe way to access the
rootClientId
at the moment. For now this is roughly inline with the other aria descriptions in List View.How?
Appender
implementation to theListViewBranch
component (where it has access to the block count and nesting level).aria-posinset
andaria-setsize
. Thearia-setsize
for blocks also needs to be incremented when there's an appender to account for the extra row.Testing Instructions for Keyboard
Screenshots or screencast