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.

Sign up for GitHub

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

Jump to bottom

Base Control Component is missing a logical case for labels #18166

Closed
mkaz opened this issue Oct 29, 2019 · 1 comment
Closed

Base Control Component is missing a logical case for labels #18166

mkaz opened this issue Oct 29, 2019 · 1 comment
Labels
[Feature] UI Components Impacts or related to the UI component system Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Package] Components /packages/components [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later

Comments

@mkaz
Copy link
Member

mkaz commented Oct 29, 2019

Describe the bug

The base control component at packages/components/src/base-control accepts an id and hideLabelFromVision` properties.

The first case of an id being passed is complete, it should use a label tag and htmlFor property and then use the VisuallyHidden or shown label.

The missing case is when id is not passed and hideLabelFromVision is passed in the label will still be visually displayed, which is not expected.

This probably should depend on #18165 getting merged first.

To reproduce

I didn't find any code specifying parameters as such, but for completeness it should be fixed up.
@mkaz mkaz added Good First Issue An issue that's suitable for someone looking to contribute for the first time [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later labels Oct 29, 2019
@mkaz mkaz mentioned this issue Oct 30, 2019
Merged
5 tasks
@gziolo gziolo added [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components Needs Dev Ready for, and needs developer efforts labels Oct 30, 2019
@fernandovbs fernandovbs mentioned this issue Nov 20, 2019
Merged
5 tasks
@mkaz
Copy link
Member Author

mkaz commented Nov 27, 2019

Fixed in #18646

@mkaz mkaz closed this as completed Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] UI Components Impacts or related to the UI component system Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Package] Components /packages/components [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mkaz @gziolo