Semantics of the <.input type="checkbox"> in core components

Hey all!

Hope you are doing well :slight_smile:

We are using the new phx 1.7 core components, and the attributes handling of the checkbox one is disturbing to me.

The source code of the entire <.input type="checkbox"> component is available here.

The thing that bothering me the most, is the mixing of the checked and the value attribute:

  def input(%{type: "checkbox", value: value} = assigns) do
    assigns =
      assign_new(assigns, :checked, fn -> Phoenix.HTML.Form.normalize_value("checkbox", value) end)

In raw html, the value attribute is expected to be a string, and to be the value sent when the form is submitted. It can be any string.

But here, it is expected to be a boolean string ie. "true" to act as checked. Any other string, and the checked is false (that’s the point of Phoenix.HTML.Form.normalize_value/2). The checked= attribute, is still expected to be a boolean.

Furthermore, the value="" attributes are hard coded in the component as boolean strings:

        <input type="hidden" name={@name} value="false" />
        <input
          […]
          value="true"
          checked={@checked}
        />

So first of all, I find the value attribute very disturbing, since it’s not at all the same logic as the raw html. Note that I understand that we can drift from the raw html specs, but the vast majority of the core components are really close from the html specs.

Then, the value= is in fact pretty useless, since it’s used only as a default for checked=. But, by having it in the def pattern matching, it is de facto required even if unused by the component itself, or face the runtime error (cannot be catch at compile time I reckon sinc the attr macro is used for all the variations of def input).

I understand that having the value attr set as a boolean string is probably handy for reloading the form after submission though, and it may be the root reason for this design, I don’t know.

To sum up:

  1. The behavior of <.input type="checkbox"> is in fact really different from native html, I find it misleading:

    • Type mismatch of the value attribute (bool string vs any string)
    • Different usage (checked vs checkbox actual value when checked)
    • Can’t set an other actual value than a boolean
  2. The implementation is quite frustratring since it forces me to use an attr I don’t want.

    • <input type="checkbox checked={true} /> raises for no functional reason

I’m not sure how things could be better…

For the first point, maybe renaming the input type in <input type="boolean" /> could be less misleading, and keeping the type="checkbox" acting closer than the html specs? At least, it would assume creating a component that is not from the html specs.

For the second point, something like this would allow at least to not de facto require the value= attribute:

  def input(%{type: "checkbox"} = assigns) do
    assigns =
      assign_new(assigns, :checked, fn -> Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value] || false) end)

I don’t know if I am missing something?

The component is mean to work with the semantics of Phoenix.HTML.Form data. It supports more datatype as HTML forms do, including boolean true/false, which those checkboxes handle.

If you’re not using a Phoenix.HTML.Form form you can still construct the input on your own instead of using the core component: <input … />. Nobody requires you to use <.input … />.

Also the core_components are part of your codebase. Feel free to customize them to your requirements. What phoenix generates is a baseline, which you can leave as is, customize or even remove completely.

This design is made to work for the basic use of checkboxes, to toggle a single value on/off.

The code creates a hidden input with a false value for the field name, so that submitting the form results in a “false” value if the user didn’t check the option, and “true” if they did.

Having a text value is useful in the multiple-checkbox pattern, where the user can choose from more than 1 option for a particular field name. In that case you can roll your own input component that loops through the various options, sets the value for each option, and does its own logic for setting the correct “checked” input.

See Handling multiple checkboxes (MDN)

Of course… but isn’t the point of core components to being “core”? We use Phoenix.HTML.Form on some parts of our app, and not in some other parts, but we still want harmonious UI…

And the <.input> components from core_components are absolutely not tied to Phoenix.Html.Form. Do you imply that we should create another set of core_components when not using Phoenix.Html.Form? I’m pretty sure that you do not.

Also the core_components are part of your codebase. Feel free to customize them to your requirements. What phoenix generates is a baseline, which you can leave as is, customize or even remove completely.

Yup.

The discussion I was looking to have is about the DX of the core components. Developers know html, and having something that feel like html, that as the same naming as html, but that is not behaving like html is misleading. And I think it may be improved, we are not looking for escape hatches :wink:

I kinda agree on this one. But I could still wanting to use the <.input type="checkbox" /> in this case I guess. Especially if I have customized my core input[type=“checked”] component with custom complex styling (since that’s the selling point for tailwind reusibility).

The important bit here is that core_components as they’re generated are to support the markup of the phoenix generators. They’re build to be flexible to not break down as soon as you go beyond that, but they’re not build to support any and all usecases you might have for inputs. Your requirements seem to go beyond what is there, so you can decide if you extend what is there or if you add some additional components. The file is in your codebase so feel free change it however you need.

E.g. this blog post goes and adds an multi checkbox input:

2 Likes

It won’t work for the default component, but a similar one could be:

  def input(%{type: "multi-checkbox", labelled_options: labelled_options} = assigns) when is_list(labelled_options) do
    ~H"""
    <div phx-feedback-for={@name}>
      <input type="hidden" name={@name} value="none" /> <=== optional depending you your use case?
      <%= for {option, label} <- labelled_options do %>
        <label class="flex items-center gap-4 text-sm leading-6 text-zinc-600">
          <input
            type="checkbox"
            id={"#{@id}-#{option}"}
            name={@name}
            value={option}
            checked={@value == option}
            class="rounded border-zinc-300 text-zinc-900 focus:ring-0"
            {@rest}
          />
          <%= label %>
        </label>
      <% end %>
      <.error :for={msg <- @errors}><%= msg %></.error>
    </div>
    """
  end

(Not tested, just off the top of my head)

I think the issue here is there <.input> is closer to the API of Phoenix.HTML.Form like the old checkbox(f, field, …) than it is to <input>. The component is not meant to be shallow wrapper for <input>.

1 Like

Thank you both, but as i said, we are not looking for escape hatches :cold_sweat: We know how to write phoenix components, and we are already not afraid to add our own or even modify our core components modules.

The important bit here is that core_components as they’re generated are to support the markup of the phoenix generators.

I understand this point of view, but my expectations were a bit wider than that (that’s ok if my expectations are not supported natively, don’t worry :D). We are using the core_components as global components available across all our application to share styling and behaviors. I expect our core components to not bother if the view is generated by phoenix generators or hand crafted.

The thing that triggered my post, was only that I spent some times to understand why my <.input type="checkbox"> was not acting as a <input type="checkbox"> (notice how small the diff is?).

From there, I had to read the core components generated by phoenix to understand the difference of behavior. I know they are part of our app and our maintenance, but still, at first, it’s not “our” code, and it felt like reading an external lib source code. I even compared our core_components with phoenix source code to check if our core components have been modified locally or if it is the intended behavior by the phoenix team, trying to understand the reasons behind those choices.

So – maybe (don’t feel attacked) – there is room for a better DX here (either in documentation, in phoenix generated code, i don’t know).

Once again, not looking for solutions. Just sharing a non optimal experience with phoenix. Having a non optimal experience with phoenix is rare enough that I thought that I could share it in case some things could be improved.

that’s all. thanks.

1 Like

The component is not meant to be shallow wrapper for <input>.

But it has the look and feel of it, that’s my point: it’s misleading!

1 Like

I’d expect that maintaining both the interface of “a plain input” and the interface of everything phoenix wants on top won’t be that simple. Even more when you expect people updating their code bases from Phoenix.HTML.Form.changeset/3.

Yeah, there could be more signal that the component is higher level. It also includes labels and such. Maybe just form_input could make this less of an issue.

2 Likes

I have read the thread and I am not sure I can think of an “optimal” solution.

We could rename “checkbox” to “boolean”, but all other input types are named after their HTML counter-parts, not their types, so having this named “boolean” would be confusing in a different way. Perhaps “boolean_checkbox”. But those are all not optimal to justify changing the generators.

Similarly, we could change the name of the function, but I am not exactly sure it helpers either. <.inputs clearly do more than the regular <input, based on the fact it renders labels and everything else, so I am not sure if the name would make such a big difference. I assume the first instinct @gorghoa would have, regardless of the name, is to use whatever is provided.

Then there is the case of multiple checkboxes but then I think it deserves its own input type and I don’t think you can build on top of <.input type="checkbox"> anyway because you will likely make different assumptions regarding styling.

WIth all that said, it sounds like improved documentation is the best way to go about it. Perhaps a summary of HTML input types and their supported types.

EDIT: better docs here: Improve .input docs · phoenixframework/phoenix@b8fe141 · GitHub

5 Likes

Thanks @josevalim for your insights!

I assume the first instinct @gorghoa would have, regardless of the name, is to use whatever is provided.

Correct, knowing myself I would have still assumed that the underlying html element would have handled my native looking attrs like value and checked to behave somehow the same as the native html’s checkbox :smiley:. But seeing that that it doesn’t, I would have immediately reached to the documentaion. So, I also think that the documentation is a good place for disambiguation on this matter :+1: .

Thanks also for editing doc, at least it is stated that the checkbox component is boolean only, it’s great.

But I fear the last part forwarding to mdn of the edited doc somehow adds up to the confusion, since the doc doesn’t state exactly where it will be ok or not to rely on mdn docs for native looking html attributes :upside_down_face: (even though the checkbox is really the exception).

See <input>: The Input (Form Input) element - HTML: HyperText Markup Language | MDN
for more information.

Ideally, we could document the .input[type=checkbox] variation specifically to state the behavior of value. (How it is handled, and why it does not set the underlying html values.

For the second point, regarding that the value attr is de facto required even though it’s barely used by the function and that the macro attr states it as optional, do you reckon that the pattern matching on it is really mandatory?

I would like to propose something like this:

From:

 def input(%{type: "checkbox", value: value} = assigns) do
   assigns =
     assign_new(assigns, :checked, fn -> Phoenix.HTML.Form.normalize_value("checkbox", value) end)

To:

 def input(%{type: "checkbox"} = assigns) do
   assigns =
     assign_new(assigns, :checked, fn -> Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value] || false) end)

I may be perceived as a nitpicker here, and I’m sorry for that :D. Besides, I really do find that those core_components shipped with new phoenix apps are great since they really help to grok what are the best practices on structuring and writing functional components. Thank you for that (and the rest).

1 Like

I think this would work too, right?

def input(%{type: "checkbox"} = assigns) do
   assigns =
     assign_new(assigns, :checked, fn -> Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value]) end)

Since everything except “true” or true are considered false anyway. If so, feel free to PR. I think the pattern match was a mistake indeed.

Thanks Jose, I will PR this!

And here is the PR: Allow `<.input type="checkbox">` without `value` attr in core components by gorghoa · Pull Request #5427 · phoenixframework/phoenix · GitHub

1 Like