-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Accessibility for Font size and Font family dropdowns in toolbar #13749
Accessibility for Font size and Font family dropdowns in toolbar #13749
Conversation
…command value (font size/family).
…ity-selected-state-for-dropdowns
…n configured options.
… in configured options.
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.
While reviewing this PR I attempted to make the heading dropdown accessible similarly because I think this won't end at font* features. Soon, I discovered this didn't play out so well
The PoC is here https://github.com/ckeditor/ckeditor5/compare/ck/13250-accessibility-selected-state-for-dropdowns-v2?expand=1.
The problem is that while font* dropdowns have invisible labels, the heading presents the label to the user. As a result, the label is read by the screen reader twice after using the new (correct) aria labels.
- The first time it is read because
.ck-button
(ButtonView
) hasaria-labelledby
that points to the label. - The second time because the screen reader discovers the "value" of the button (text of the label).
More details:
- This is happening because of
role="combobox"
set on the<button>
element. - This proves that my initial approach to setting labels that inspired Marta could be wrong. It works only as long as the label is invisible.
- We may need to rethink how dropdowns are labeled, which now that I think about it, totally makes sense:
- We need a label for the whole dropdown, for instance "Font family" (this one is missing).
- But the API is there:
DropdownView#ariaDescribedById
exists and has been used in UIs such as table properties (as a child ofLabeledFieldView
).
- But the API is there:
- We need a label for the dynamic value of the feature, for instance "Helvetica", unlike what I proposed in the past and unlike what has been done in this PR.
- Then we need attributes that couple the dropdown button with the list. This is covered in this PR.
- Only then we'll get as close to the standards as possible.
- We need a label for the whole dropdown, for instance "Font family" (this one is missing).
After additional research we agreed with Olek that we should change the implementation from using a The new PoC was prepared for font family, font size and heading features. It's already covered with tests. Some implementation details:
|
As we agreed to go with a different solution, I will close this PR. The new version can be found in #13893. |
Suggested merge commit message (convention)
Fix (font): Screen readers should now announce selected option in dropdown lists for font size and font family.
Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.