Skip to content
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

Audit usage of explicit aria-label on Button component implementations #66271

Open
2 tasks done
afercia opened this issue Oct 21, 2024 · 2 comments
Open
2 tasks done

Audit usage of explicit aria-label on Button component implementations #66271

afercia opened this issue Oct 21, 2024 · 2 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality

Comments

@afercia
Copy link
Contributor

afercia commented Oct 21, 2024

Description

It appears that in some cases some Button components use an explicit aria-label prop to render an aria-label HTML attribute, which in turn determines the icon button tooltip (if it's an icon button). Example:

<Tooltip text={ label }>
<Button
{ ...props }
className="component-border-radius-control__linked-button"
size="small"
icon={ isLinked ? link : linkOff }
iconSize={ 24 }
aria-label={ label }
/>
</Tooltip>

This pattern should be audited across the entire codebase as it's not a best practice.

Setting an explicit aria-label signals two possible cases, both less than ideal:

  • The aria-label is used in a way that the accessible name mismatches the Button text or tooltip. This is not OK for accessibility.
  • The aria-label is just unnecessary, like in the example above. In that case, the wrapping Tooltip component uses the same label variable used for aria-label. In this case, the Tooltip component can be removed and the variable passed to the label prop of the Butotn.

Step-by-step reproduction instructions

N/A

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality labels Oct 21, 2024
@ciampo
Copy link
Contributor

ciampo commented Oct 21, 2024

Related to #66021

cc @WordPress/gutenberg-components

@im3dabasia
Copy link
Contributor

im3dabasia commented Dec 11, 2024

Hey @afercia ,

I have studied the issue you described above and found three instances that follow a similar pattern. I have attached my findings below.

Please provide me with feedback on them, and if these need to be fixed, may I open PRs for the same?

<Tooltip text={ label }>
<Button
{ ...props }
size="small"
icon={ isLinked ? link : linkOff }
iconSize={ 24 }
aria-label={ label }
/>
</Tooltip>

<Tooltip text={ label }>
<Button
{ ...props }
className="component-box-control__linked-button"
size="small"
icon={ isLinked ? link : linkOff }
iconSize={ 24 }
aria-label={ label }
/>
</Tooltip>

<Tooltip text={ label }>
<View className={ className }>
<Button
{ ...buttonProps }
size="small"
icon={ isLinked ? link : linkOff }
iconSize={ 24 }
aria-label={ label }
ref={ forwardedRef }
/>
</View>
</Tooltip>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants