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

Remove aria-label from buttons that open in a new window. #389

Open
alan-cole opened this issue Sep 20, 2024 · 1 comment
Open

Remove aria-label from buttons that open in a new window. #389

alan-cole opened this issue Sep 20, 2024 · 1 comment
Assignees
Labels
State: Done The issue is complete and waiting for a release Type: Defect Issue is a defect
Milestone

Comments

@alan-cole
Copy link
Collaborator

alan-cole commented Sep 20, 2024

Summary

At: https://github.com/civictheme/uikit/blob/main/components/01-atoms/button/button.twig#L77

an aria label of aria-label="Opens in a new tab" is set, which a screen reader would read out instead of the content within the button.

If removing this, the screen reader will still alert the user that the link opens in a new window, as..

<span class="ct-visually-hidden">(Opens in a new tab/window)</span>

...is included in the button if allow_html is disabled and is_new_window is enabled. This is because text-icon.twig will include it.

Steps to reproduce

Use a screen reader on:

https://uikit.civictheme.io/?path=/story/atoms-form-controls-button--button&knob-Allow%20HTML%20in%20text_General=true&knob-Is%20external_General=true&knob-Kind_General=button&knob-Size_General=regular&knob-Text_General=Button%20text&knob-Theme_General=light&knob-Type_General=primary

With the following settings:

Screenshot 2024-09-20 at 4 46 07 PM

Observed outcome

Screen reader will read out "Opens in a new tab" rather than "Will this text read out?"

Expected outcome

Screen reader should read out "Will this text read out? Opens in a new tab"

@alan-cole alan-cole added the Type: Defect Issue is a defect label Sep 20, 2024
alan-cole added a commit that referenced this issue Sep 25, 2024
The aria-label in this case would override whatever text was present within the button, meaning buttons lose context if used with a screen reader.
alan-cole added a commit that referenced this issue Sep 25, 2024
@github-project-automation github-project-automation bot moved this to Todo in UI Kit Oct 1, 2024
@fionamorrison23 fionamorrison23 added this to the 1.9 milestone Oct 1, 2024
@fionamorrison23 fionamorrison23 moved this from Todo to In Progress in UI Kit Oct 1, 2024
@fionamorrison23 fionamorrison23 added the PR: Ready for test Pull request is ready for manual testing label Oct 1, 2024
@sonamchaturvedi28
Copy link

Test Env: DEV
Test Status: PASS
Test Result:

  • Screen reader reads out "Will this text read out? Opens in a new tab".
Screenshot 2024-10-08 at 2 21 58 PM

@fionamorrison23 fionamorrison23 added State: Done The issue is complete and waiting for a release and removed PR: Ready for test Pull request is ready for manual testing labels Oct 10, 2024
@fionamorrison23 fionamorrison23 moved this from In Progress to Done in UI Kit Oct 10, 2024
@fionamorrison23 fionamorrison23 modified the milestones: 1.9, 1.10 Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Done The issue is complete and waiting for a release Type: Defect Issue is a defect
Projects
Status: Done
Development

No branches or pull requests

4 participants