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

TextControl: label not connecting to input if props contains an id #24749

Closed
truongwp opened this issue Aug 23, 2020 · 3 comments · Fixed by #52028
Closed

TextControl: label not connecting to input if props contains an id #24749

truongwp opened this issue Aug 23, 2020 · 3 comments · Fixed by #52028
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@truongwp
Copy link
Contributor

truongwp commented Aug 23, 2020

Describe the bug
If I passed an id to TextControl, the <input> tag uses that id but the <label> tag not. It makes the input is not focused when I click to the label.

To reproduce
Use this component in an app:

<TextControl
	id="my-custom-id"
	label="Test title"
/>

Expected behavior
Both the id of input and label should be my-custom-id.

Solutions
Add a prop for the label for or check if props contains an id, use that id for the label.

@talldan
Copy link
Contributor

talldan commented Aug 28, 2020

@truongwp From what I gather, id is not a supported prop. The component generates a unique id for you, so it doesn't have to be supplied as a prop.

Having said that, it does seem like a relatively simple change, but I think it should be done across all the Control components in @wordpress/components to be consistent.

@talldan talldan added [Package] Components /packages/components [Type] Enhancement A suggestion for improvement. labels Aug 28, 2020
@truongwp
Copy link
Contributor Author

@talldan Sometimes I need to pass my own id so it will be great if we can improve this. Currently, I must create the similar WordPress components to achieve my need.

@andrewhayward
Copy link
Contributor

I'd like to work on this, I'll be opening a PR soon // cc @ciampo

andrewhayward pushed a commit to andrewhayward/gutenberg that referenced this issue Jun 28, 2023
This patch adds support for custom IDs in the `TextControl` component.

Currently any passed ID prop is ignored, and a auto-generated value is
used instead.

---
Resolves WordPress#24749
@mirka mirka linked a pull request Jul 3, 2023 that will close this issue
mirka pushed a commit that referenced this issue Jul 12, 2023
* Adding support for defined IDs in `TextControl`

This patch adds support for custom IDs in the `TextControl` component.

Currently any passed ID prop is ignored, and a auto-generated value is
used instead.

---
Resolves #24749

* Updating tests

Changing the test setup for labels to be more `testing-library` idiomatic.

* Updating CHANGELOG

Adding line item under Enhancements to include `TextControl` changes.

* Fixing use of `useInstanceId`

Dropping the old `useUniqueId` pattern in favour of the more succinct and complete `useInstanceId` usage.
@ciampo ciampo moved this from Todo to Done in Andrew's onboarding Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants