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
Add announcement on formatting change for screen readers #35896
Add announcement on formatting change for screen readers #35896
Conversation
I did a before and after comparison with NVDA and Chrome on Windows and the PR is working as described. I am not the target user for this feature as I am not a screen reader user, but I agree it is an improvement since there were no announcements before this PR. I did not do a code review. |
Tested with NVDA/Chrome. This is pretty slick; I didn't encounter any issues testing, and definitely agree that this is an improvement. |
Thanks @joedolson and @carolinan for testing. This is ready for code review. @noisysocks do you think this can land before Friday so it can go in to 11.9? If not, this should be perfect for following release after WordPress 5.9. Thanks. |
I don't think it's a "must have" for 5.9 so I won't add it to the board but I'm confident we can get it merged in time anyway.
@noisysocks Where exactly should I define the title for the announcement? I agree with you about the positional args situation but not sure where else to define this. The other nice thing about the toggleFormat() function is that I can tell if the format is being added or removed. I just don't know how I can get the title of the formatting button any other way than passing it directly as an arg. I know I can get say a list of blocks easily as I can access the block-editor store, but didn't look so simple for formatting buttons. |
@noisysocks Sorry about that, ignore my question. You gave me an example and I looked over it. Let me get it updated. That makes sense. |
@noisysocks Alright, fixes made. If you spot anything else, I'll likely be back on this tomorrow. |
I have to step out so can't test right now but the code LGTM |
* Add announcement message for Bold toggle for screen readers. * Speak update on toggleFormat(). * Try to fix tests. * Only fire alert on keyboard shortcut unless it is an item only accessible from menu item. * Render menu items as checkboxes. * Make More button more descriptive. * Add description text to More menu. Fix incorrect role for text-color. * Store title in format object.
alexstine commentedOct 23, 2021
•
edited
Loading
Description
When formatting text with items such as Code, Superscript, Subscript, etc, there is now an alert for screen readers to announce that the formatting has been added/removed. I also added this for Bold, Italics, and Underline. However, the alert will only be fired on shortcut key, not button click. Double feedback isn't really needed due to
aria-pressed="true"
.The only item not modified is text-color/Highlight. That needs it's own set of accessibility enhancements and I will do this in another PR.
I also added description text to the More menu and added
role="menuitemcheckbox"
that way users can tell if an item is enabled or disabled. I kept the alerts here as the More menu closes before checked/unchecked state is read. There is a similar issue for the Alignment Control which I may attempt to fix later.How to test
Test 1
For this test, you will hear that formatting is applied/removed while using the keyboard.
Test 2
In this test, you will explore the block formatting toolbar.
How has this been tested?
I tested using NVDA in Firefox. I checked to see which code called the function I modified and ensured params were updated.
Screenshots
Types of changes
This is a new feature.
Checklist:
*.native.js
files for terms that need renaming or removal).The text was updated successfully, but these errors were encountered: