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(ToggleSmall): add optional labels to ToggleSmall #3045

Closed
wants to merge 1 commit into from
Closed

fix(ToggleSmall): add optional labels to ToggleSmall #3045

wants to merge 1 commit into from

Conversation

dakahn
Copy link
Contributor

@dakahn dakahn commented Jun 12, 2019

Closes #2856

Adds optional labels on ToggleSmall and adds an example to the documentation. 👍

Changelog

+ "with labels" story to docs
+ labelA and labelB props to ToggleSmall (mirroring Toggle)
+ a check in render for both props that displays labels over SVGs

To test

Go to the ToggleSmall section of our React documentation (the Netlify builds below) and select the "with labels" option.

@dakahn dakahn requested a review from snidersd June 12, 2019 21:10
@dakahn dakahn added the package: react carbon-components-react label Jun 12, 2019
@netlify
Copy link

netlify bot commented Jun 12, 2019

Deploy preview for the-carbon-components ready!

Built with commit 92d8dbb

https://deploy-preview-3045--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jun 12, 2019

Deploy preview for carbon-elements ready!

Built with commit 92d8dbb

https://deploy-preview-3045--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Jun 12, 2019

Deploy preview for carbon-components-react ready!

Built with commit 92d8dbb

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

@dakahn dakahn requested a review from laurenmrice June 12, 2019 21:34
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.

I think this directly overlaps with #2809 #2885 which is awaiting confirmation from design on visual changes (I guess tagging @laurenmrice to review #2809?)

@snidersd
Copy link

I've tested the changes with DAP and there are errors associated with the unlabeled toggles. (see below)
Screen Shot 2019-06-13 at 9 27 18 AM
WCAG Technique H44: Using label elements to associate text labels with form controls requires that all form controls have a visible label. . In the test section it states:
For all input elements of type checkbox or radio in the Web page:

  1. Check that there is a label element that identifies the purpose of the control after the input element
  2. Check that the for attribute of the label element matches the id of the input element
  3. Check that the label element is visible.

Remove the examples that do not have a visible label to fix the issue.

@emyarod
Copy link
Member

emyarod commented Jun 13, 2019

@snidersd unless I am misunderstanding it sounds like the same issue as #2809 (addressed in pending PR #2885) is that right?

@snidersd
Copy link

@emyarod, you are correct. I referenced #2809 in the original issue #2856.

@dakahn dakahn closed this Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: react carbon-components-react type: a11y ♿
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The react carbon component ToggleSmall doesn't have labels
3 participants