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

[Checkbox] Support form attribute #2484

Closed
wants to merge 2 commits into from
Closed

Conversation

Manduro
Copy link
Contributor

@Manduro Manduro commented Oct 25, 2023

Description

This lets you associate the underlying native <input> element to a <form> anywhere in the document instead of requiring an ancestor <form> element.

Related change for Select: #2447

This lets you associate the underlying native `<input>` element to a `<form>` anywhere in the document instead of requiring an ancestor `<form>` element.
@chaance
Copy link
Member

chaance commented Sep 26, 2024

I definitely want to support the form prop, but we still need to do the DOM check if the prop isn't passed.

@Manduro
Copy link
Contributor Author

Manduro commented Sep 27, 2024

@chaance That happens automatically:

If this attribute is not set, the <button> is associated with its ancestor <form> element, if any.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#form

@chaance
Copy link
Member

chaance commented Sep 27, 2024

@chaance That happens automatically:

If this attribute is not set, the <button> is associated with its ancestor <form> element, if any.

developer.mozilla.org/en-US/docs/Web/HTML/Element/button#form

Yes, but we use isFormControl to determine whether or not to render the bubble input in the first place, so my point is we still want to check for an ancestor form if there is no form attribute.

@Manduro
Copy link
Contributor Author

Manduro commented Sep 27, 2024

@chaance That is what the code does. button.form references the ancestor form automatically as per the mdn docs.

@chaance
Copy link
Member

chaance commented Sep 27, 2024

Ohhh that's my bad, I misread your code. Yeah this makes sense, thanks 👍

@chaance
Copy link
Member

chaance commented Sep 27, 2024

Actually sorry, gotta push back once more: if the user renders the checkbox as anything other than a button via asChild this would still break. I think we need to account for that.

@chaance
Copy link
Member

chaance commented Sep 27, 2024

I'm actually going to close this for now. Per #2530 (comment) I'd like to introduce a more wholistic solution to cover all affected primitives, and I still need to do some manual testing for edge cases.

Tracking in #2530

@chaance chaance closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants