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

Allow <.input type="checkbox"> without value attr in core components #5427

Merged

Conversation

gorghoa
Copy link
Contributor

@gorghoa gorghoa commented Apr 19, 2023

Hi!

Currently, the <.input type="checkbox"> in the core components is "unsafe" to use from a DX point of view:

Using <.input type="checkbox" checked={true} /> without an attribute value="…" raises a runtime exception because the value key is not in assigns.

The @value is not a required attr in the def input, yet, the def input for checkbox was pattern matching for it in the function argument, so the value attr is de facto required even if announced optional.

I can’t see why the value is mandatory, the body of the function is barely using it as a fallback if @checked is not set. So I propose to just dropping it from the function pattern matching, and later check if present in the assigns map.

See also this discussion I started on elixir forum before making this PR.

Thanks for your kind review 🙏 !

@Gazler
Copy link
Member

Gazler commented Oct 3, 2023

Thanks <3

@Gazler Gazler merged commit 2c7fa8e into phoenixframework:main Oct 3, 2023
@gorghoa gorghoa deleted the core-components-checkbox-semantics branch October 3, 2023 14:34
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.

3 participants