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(number-input): removes role from icon and adds conditional aria label #4816

Merged
merged 3 commits into from
Dec 5, 2019
Merged

fix(number-input): removes role from icon and adds conditional aria label #4816

merged 3 commits into from
Dec 5, 2019

Conversation

abbeyhrt
Copy link
Contributor

@abbeyhrt abbeyhrt commented Dec 4, 2019

Closes #4532

This PR removes the unnecessary role="img" from WarningFilled16. To avoid both a label and an aria label on the input, it changes the aria-label on the input to only be applied when there is no label provided.

Changelog

New

  • tests for aria-label on input based on label

Changed

  • how ariaLabel is applied to input

Removed

  • role="img" on WarningFilled16

Testing / Reviewing

Check to make sure that NumberInput has no DAP violations

@abbeyhrt abbeyhrt requested a review from a team as a code owner December 4, 2019 20:41
@ghost ghost requested review from aledavila and dakahn December 4, 2019 20:41
@netlify
Copy link

netlify bot commented Dec 4, 2019

Deploy preview for the-carbon-components ready!

Built with commit 623c4f8

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

@netlify
Copy link

netlify bot commented Dec 4, 2019

Deploy preview for carbon-components-react ready!

Built with commit 623c4f8

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

@netlify
Copy link

netlify bot commented Dec 4, 2019

Deploy preview for carbon-elements ready!

Built with commit 623c4f8

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

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @abbeyhrt!

Copy link
Contributor

@aledavila aledavila 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 155ebed into carbon-design-system:master Dec 5, 2019
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.

AVT 1 - Number Input has DAP violations
5 participants