Move phx-feedback-for to the parent element of a form element

[sezaru]

Move phx-feedback-for to the parent element of a form element

[sezaru]

Thinking more about this, I would also argue that we should also move the has-error field to the div instead of the input.

If we do that, we would also solve #159

He is an example of that to the select input:

  def select(assigns) do
    error_class = if field_has_errors?(assigns), do: "has-error", else: ""

    assigns =
      assigns |> assign_defaults("pc-select") |> assign(:error_class, error_class)

    ~H"""
    <div phx-feedback-for={Form.input_name(@form, @field)} class={@error_class}>
      <%= Form.select(
        @form,
        @field,
        @options,
        [class: @classes] ++ Map.to_list(@rest)
      ) %>
    </div>
    """
  end

And change the CSS to this:

div.has-error:not(.phx-no-feedback) > select {
  @apply !border-red-500 focus:!border-red-500 !text-red-900 !placeholder-red-700 !bg-red-50 dark:!text-red-100 dark:!placeholder-red-300 dark:!bg-red-900 focus:!ring-red-500;
}

[sezaru]

Btw, if you are OK with the change I can create a PR with it for all form components

[mplatts]

One issue could be you now are returning a div instead of an input, which may be unexpected or could create issues with styling. You might want to add styling to that div for layout purposes.

Maybe a halfway point could be adding your new classes so people can wrap their input with <div phx-feedback-for={Form.input_name(@form, @field)}> and it'll still work with the phx-update="ignore"? That's what I do with <.form_field>

[sezaru]

Not sure if I understand your suggestion, can you clarify it? Do you mean manually adding the div outside the component? If so, that would work, but I believe that the user would also need to remove/overload the current CSS from petal components and add a new one like the one I showed above.

In that case, it seems to me that the user is better off just copying the full form.ex file from Petal Components and changing it instead of trying to work around it.

I don't see what is the problem with actually returning the div instead of the input as the root tbh, as long as that div doesn't have any styling on it, it should just be "invisible" to the end user no?

Also, even core_components does that, seems like that is the "correct" way of handling these cases since inputs have a lot of corner cases (like the select input discussed in #159).

Edit: Also, if there is a need to allow adding classes/styling that div, then we can just add a new optional attr to the components to add classes to it no?

[mplatts]

Yeah, I meant manually adding a div - perhaps in a new custom component that utilises the <.text_input> component or whichever one you were using. eg

eg

def my_new_field do
  ~H"""
  <div phx-feedback-for={@field}>
    <.form_label form={@form} field={@field} />
    <.text_input phx-hook="xxx" field={@field} form={@form} field={@field} />
    <.form_field_error form={@form} field={@field} />
  </div>
  """
end

The core_components input function is more like our form_field function - it contains the label, input and error components.

Can I ask what hook you are implementing?

[mplatts]

Done in v1.1.0 with the new <.field> component. 791276f

Docs coming soon