polymorphic_embed icon indicating copy to clipboard operation
polymorphic_embed copied to clipboard

sort_param and drop_param for cast_polymorphic_embed

Open woylie opened this issue 1 year ago • 1 comments

Ecto 3.10 introduces the sort_param and drop_param options for cast_assoc and cast_embed: https://hexdocs.pm/ecto/Ecto.Changeset.html#cast_assoc/3-sorting-and-deleting-from-many-collections. This is also showcased in a LiveView context here. I think it would be useful to add those options to cast_polymorphic_embed as well.

woylie avatar Apr 28 '23 15:04 woylie

As this repo seems to be somewhat inactive, I implemented this here: https://github.com/SteffenDE/polymorphic_embed/commit/023d2c4ecfef23c997f27586cf26f9bc9cb85b5b

Note that I do not commit on maintaining my fork, this is just for use in personal project currently. Seems to work fine though. If someone wants to create a PR out of that feel free.

SteffenDE avatar Jun 17 '23 13:06 SteffenDE

This is now supported. @SteffenDE all your changes from your branch have been added. Awesome work.

mathieuprog avatar May 26 '24 11:05 mathieuprog

@SteffenDE btw, basic question as I'm not a LiveView user, the equivalent of PolymorphicEmbed.HTML.Component.polymorphic_embed_inputs_for/1, which you added, is Phoenix.Component.inputs_for/1 in LiveView.

But where is the equivalent for polymorphic_embed_inputs_for/2 (from the LiveView example) in Phoenix/LiveView? https://github.com/mathieuprog/polymorphic_embed/blob/634c9cf0e6517d7b90e4f62d0f3bf4c227545231/README.md#displaying-form-inputs-and-errors-in-liveview

mathieuprog avatar May 27 '24 07:05 mathieuprog

Hi @mathieuprog! PolymorphicEmbed.HTML.Component.polymorphic_embed_inputs_for/1 is the replacement for both input helper functions. This is the same in which Phoenix.Component.inputs_for/1 replaces all Phoenix.HTML.Form.inputs_for helpers.

In fact, the component variant is not LiveView specific. This might be confusing as Phoenix.Component is in the LiveView repo, but actually it is just a function component that can be used anywhere where HEEx templates are used.

So, the example would be written as:

<.form
  :let={f}
  for={@changeset}
  id="reminder-form"
  phx-change="validate"
  phx-submit="save"
>
  <.polymorphic_embed_inputs_for field={f[:channel]} :let={channel_form}>
    <%= case get_polymorphic_type(f, :channel) do %>
      <% :sms -> %>
        <.input field={channel_form[:number]} label="Number" />

      <% :email -> %>
        <.input field={channel_form[:address]} label="Email Address" />
    <% end %>
  </.polymorphic_embed_inputs_for>
</.form>

In this example, I also replaced the "old" label and text_input functions with their "new" CoreComponents HEEx component counterpart (https://github.com/phoenixframework/phoenix/blob/main/priv/templates/phx.gen.live/core_components.ex).

The component approach is mainly beneficial for LiveView apps, as it allows for proper change tracking. In "dead" views its only benefit is probably the "cleaner", HTML-like look.

SteffenDE avatar May 27 '24 08:05 SteffenDE

Thank you. So polymorphic_embed_inputs_for/2 function is to be removed then?https://github.com/mathieuprog/polymorphic_embed/blob/master/lib/polymorphic_embed/html/form.ex#L44 ?

It was specifically added for LiveView.

mathieuprog avatar May 27 '24 08:05 mathieuprog

Actually it seems that all the _inputs_for functions defined in HTML.Form are obsolete? https://github.com/mathieuprog/polymorphic_embed/blob/master/lib/polymorphic_embed/html/form.ex

Do we even still need this module at all?

mathieuprog avatar May 27 '24 08:05 mathieuprog

Actually it seems that all the _inputs_for functions defined in HTML.Form are obsolete? https://github.com/mathieuprog/polymorphic_embed/blob/master/lib/polymorphic_embed/html/form.ex

Do we even still need this module at all?

The component syntax should be preferred now. Calling functions in LiveView can prevent proper change tracking. So I don't think we need that module anymore.

woylie avatar May 27 '24 09:05 woylie

Thanks woylie. On my side I don't have a codebase with form rendering to test such changes.

If anyone could clean up the unnecessary functions/modules and update the readme, that'd be appreciated. Or I'll do it later.

mathieuprog avatar May 27 '24 09:05 mathieuprog

Do we even still need this module at all?

I think the whole module can be deprecated. It doesn't hurt to keep it around for backwards compatibility though and as 4.0 is already released it would be a breaking change. So I'd say mark is as deprecated in the next minor release and remove it in the next major.

But talking about breaking changes: the 4.0 release also requires phoenix_html 4.0, which might be good to mention in the changelog.

SteffenDE avatar May 27 '24 11:05 SteffenDE

@woylie @SteffenDE

Could you have a look at this? https://github.com/mathieuprog/polymorphic_embed/issues/101

Could you share me your opinion on the package name?

mathieuprog avatar May 30 '24 00:05 mathieuprog