-
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
Try: more accessible labels. #29659
Try: more accessible labels. #29659
Conversation
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
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'm not sure about how this would be handled in RTL. Added a comment 👍
cd8e47e
to
8b08364
Compare
Co-authored-by: Ari Stathopoulos <[email protected]>
Nice, that works well for me. Thank you! |
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.
Than you for the tweaks!
LGTM 👍
$label = ( isset( $attributes['label'] ) ) ? $attributes['label'] : block_core_social_link_get_name( $service ); | ||
$label = ( isset( $attributes['label'] ) ) ? $attributes['label'] : sprintf( | ||
/* translators: %1$s: Social-network name. %2$s: URL. */ | ||
__( '%1$s: %2$s', 'gutenberg' ), |
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.
Does it really need to be applied in the translation? It looks like sprintf
is enough.
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.
If it's not in the translation, then how would an RTL language reverse it, or how would French add a space before the :
? 🤔
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 see now it's only used with aria-label
so it's going to be read by the assistive technology which is yet another set of challenges. No idea, how to approach it 🤷🏻
Description
Fixes #21840.
This PR adds the URL as an additional aria label suffix to social links.
This is a small improvement to the aria label for social links. Users can still add a more verbose label in the interface:
How has this been tested?
Checklist: