Improve accesibility for Manage theme fonts page #271
Conversation
| <div> | ||
| <RangeControl | ||
| <div className="font-size-input"> | ||
| <NumberControl |
There was a problem hiding this comment.
Does the NumberControl work OK if it's placed after the RangeControl? I believe this is the intended design, so it would be great if we could keep it like that:
| aria-label={ sprintf( | ||
| /* translators: %s: Font Family. */ | ||
| __( 'Remove %s Font Family' ), | ||
| fontFamily.name || fontFamily.fontFamily | ||
| ) } |
| <Button | ||
| variant="tertiary" | ||
| onClick={ toggleIsOpen } | ||
| aria-expanded={ isOpen } | ||
| aria-label={ | ||
| isOpen | ||
| ? __( | ||
| 'Collapse Font Family', | ||
| 'create-block-theme' | ||
| ) | ||
| : __( | ||
| 'Expand Font Family', | ||
| 'create-block-theme' | ||
| ) | ||
| } | ||
| > |
| @@ -192,8 +192,9 @@ function ManageFonts() { | |||
| <Button | |||
| onClick={ toggleIsHelpOpen } | |||
| style={ { padding: '0', height: '1rem' } } | |||
| aria-label={ __( 'Alert Popup', 'create-block-theme' ) } | |||
There was a problem hiding this comment.
I'd maybe label this as something like 'More information' or 'Additional information', rather than 'Alert'.
There was a problem hiding this comment.
@mikachan Thanks for your feedback.
|
@mikachan I think we need to create another PR for it. there are many changes in the font page. |
Yeah that's true, it's probably easier to handle the changes in a new PR 👍 |
|
Thanks for your contribution! Font management is being handled by core now, so we're working towards removing it from the plugin on the next release. |
Issue: #230
The expand/collapse icon on each font family heading is missing a label. I think we should add a label based on these best practices.
-> Add
aria-expands: true/falsebased on the user action, Also addedaria-labelto the button andaria-hidden: truefor icon.It would be good to add the name of the font family in each remove button, so it's clear which font family they are linked to. For example, add an aria-label with the contents, "Remove DM Sans font family".
-> Add
aria-labelfor button andaria-hidden: truefor icon.Similar to above, it would be good to add an aria-label to the font face buttons too.
-> Similarlly, Adds
aria-labelfor button andaria-hidden: truefor icon.The tab order of the font size range control is reversed because the flex-direction is set to row-reverse. This is probably because the RangeControl component only allows for the input to be shown on the right, and in this case, it works better on the left. I'm not sure what's possible here but thought it was worth mentioning. Maybe we could look into changing the tab order somehow.
All checked item has been improved, the last one is pending. Last item needs discussion, how we can improve that? Needs
you opinion.