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

fix(a11y): stop SR announcement for hidden labels for Textinput, TimePicker #18080

Closed
wants to merge 3 commits into from

Conversation

2nikhiltom
Copy link
Contributor

@2nikhiltom 2nikhiltom commented Nov 15, 2024

Closes #16752 #16862

SR should not announce the label when hideLabel is true

Changelog

New
Added aria-hidden attribute

Testing / Reviewing

  • open deploy preview - turn on screen reader

  • for both Textinput and TimePicker components
    -set the hide label Boolean to "True" and observe the SR do not announce label

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit aa868e6
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-web-components/deploys/67371c36cc456e00081dc9c7
😎 Deploy Preview https://deploy-preview-18080--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit aa868e6
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/67371c3609a4db00086a053e
😎 Deploy Preview https://deploy-preview-18080--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit aa868e6
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/67371c360c06a00008a91e58
😎 Deploy Preview https://deploy-preview-18080--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.19%. Comparing base (ff55263) to head (aa868e6).
Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #18080   +/-   ##
=======================================
  Coverage   82.19%   82.19%           
=======================================
  Files         404      404           
  Lines       14151    14151           
  Branches     4418     4468   +50     
=======================================
  Hits        11631    11631           
  Misses       2359     2359           
  Partials      161      161           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@2nikhiltom 2nikhiltom changed the title fix(a11y): stop SR announcement for hidden labels for Textinput fix(a11y): stop SR announcement for hidden labels for Textinput, TimePicker Nov 15, 2024
@2nikhiltom 2nikhiltom marked this pull request as ready for review November 15, 2024 10:35
@2nikhiltom 2nikhiltom requested a review from a team as a code owner November 15, 2024 10:35
@alisonjoseph
Copy link
Member

I believe that the point of hiding a label only visually (using our visually hidden class) is specifically so that the label is still readable by screen readers. Otherwise we could just throw a display: none on it. This feels like something we probably shouldn't be doing. https://www.w3.org/WAI/tutorials/forms/labels/

@guidari were these initial issues reported by our testing team?

@guidari
Copy link
Contributor

guidari commented Nov 18, 2024

Hey @alisonjoseph I agree with you. But this was reported by Raghu, he pointed that in different components as well. I guess we can double check that with Raghu, because the label might have useful information for the screen readers, but it isn't useful visually.

@guidari
Copy link
Contributor

guidari commented Nov 18, 2024

@alisonjoseph @2nikhiltom
I just talked with Gower in the CAG call, and he also agrees that the label has to be announced, otherwise that mighe lead to a11y issues.
I guess we can close a couple issues related to that here. We can also talk with Raghu as well, just let him know about this.

@alisonjoseph
Copy link
Member

Thanks for checking @guidari.

@2nikhiltom
Copy link
Contributor Author

Closing these similar issues on other components as well
#16613 #16750 #16601 #16531

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y]: TextInput - Playground - Visually hidden label announced by the SR
3 participants