Components: replace VisuallyHidden from components with UI version#50403
Components: replace VisuallyHidden from components with UI version#50403simison wants to merge 1 commit into
VisuallyHidden from components with UI version#50403Conversation
…h `@wordpress/ui` version
simison
added
[Status] Needs Review
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Block] Contact Form
[Feature] Contact Form
[Feature] Publicize
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
| <VisuallyHidden | ||
| render={ | ||
| <span> | ||
| { | ||
| /* translators: accessibility text */ | ||
| __( '(opens in a new tab)', 'jetpack-components' ) | ||
| } | ||
| </span> | ||
| } | ||
| </VisuallyHidden> | ||
| /> |
There was a problem hiding this comment.
I'd recommend only including attributes in render if it's inherently tied to the element / component in the render and not part of the root component's own interface. Since VisuallyHidden accepts children, I'd pass it there, which would also simplify this a bit.
We could probably do a better job of documenting rules about where consumers should apply props when using render. Though for VisuallyHidden in particular, we do document this composition example.
<VisuallyHidden render={ <span /> }>
{
/* translators: accessibility text */
__( '(opens in a new tab)', 'jetpack-components' )
}
</VisuallyHidden>(Same comment applies below)
| <Text | ||
| className={ clsx( styles.deltaValue, styles.deltaPlaceholder ) } | ||
| style={ { color: deltaColor } } | ||
| aria-label={ __( 'No comparison data', 'jetpack-charts' ) } | ||
| > |
There was a problem hiding this comment.
aria-label should not be used on presentational content like a <span />. I think the previous approach was correct.
The generic role is intended for use as the implicit role of generic elements in host languages (such as HTML div or span)
https://www.w3.org/TR/wai-aria-1.2/#generic
5.2.8.6 Roles which cannot be named (Name prohibited): [...] generic
2 participants
Part of #48160
Proposed changes
VisuallyHiddencomponent from@wordpress/components(storybook) with@wordpress/uiversion (storybook).Later on, I'll enable a linter which will complain about using the old component (#48487)
Related product discussion/links
Does this pull request change what data or activity we track or use?
Testing instructions