Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

fix(a11y): avoid empty label in ToggleSmall #1771

Merged
merged 3 commits into from
Jan 25, 2019
Merged

fix(a11y): avoid empty label in ToggleSmall #1771

merged 3 commits into from
Jan 25, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Jan 20, 2019

Fixes #1183.

Changelog

New

  • Invisible <span> in <ToggleSmall>'s <label> for assistive technologies.

@netlify
Copy link

netlify bot commented Jan 20, 2019 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit 45138a5

https://deploy-preview-1771--carbon-components-react.netlify.com

emyarod
emyarod reviewed Jan 21, 2019
View reviewed changes
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the original reporter stated that the label needs to be before the input, is that still the case?

@lovemecomputer
Copy link
Contributor

lovemecomputer commented Jan 21, 2019 •
edited
Loading

was going to say the same thing. not sure if valuable to merge in this fix in now as one PR, then refactor the label>input order later?
edit: tho i suppose that wouldn't make much different since the label isn't visible anyway!

@asudoh
Copy link
Contributor Author

asudoh commented Jan 22, 2019

Good catch @emyarod! Some research suggested that the order is for screen magnifier: https://www.w3.org/TR/UNDERSTANDING-WCAG20/minimize-error-cues.html, and as @lovemecomputer pointed out, it doesn't seem to affect our small toggle. CC @dakahn

dakahn
dakahn approved these changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. checked with DAP in latest chrome and the August 2018 ruleset

@asudoh
Copy link
Contributor Author

asudoh commented Jan 24, 2019

Great news - Thanks alot @dakahn for testing!

lovemecomputer
lovemecomputer approved these changes Jan 24, 2019
View reviewed changes
Copy link
Contributor

@lovemecomputer lovemecomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@asudoh asudoh merged commit 45972b4 into carbon-design-system:master Jan 25, 2019
@asudoh asudoh deleted the toggle-small-assistive-text branch January 25, 2019 05:01
@carbon-bot
Copy link

🎉 This PR is included in version 6.81.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gdelory
Copy link

gdelory commented Jan 25, 2019

Hi, not sure if that's expected, but for me it's not invisible at all :)

image

@emyarod
Copy link
Member

emyarod commented Jan 25, 2019

@gdelory would you be able to provide a Code Sandbox showcasing the issue you are facing?

@gdelory
Copy link

gdelory commented Jan 28, 2019

@emyarod Please ignore this, I just double checked and I was including manually an older version of carbon-components.css (9.1 something). All my apologies for this. All looks perfect now.

@emyarod emyarod mentioned this pull request Jul 9, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@emyarod emyarod emyarod left review comments

@lovemecomputer lovemecomputer lovemecomputer approved these changes

@dakahn dakahn dakahn approved these changes

Assignees
No one assigned
Labels
status: one more review 👀 status: ready for review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@asudoh @lovemecomputer @carbon-bot @gdelory @emyarod @dakahn