maxmarcon / live_select

Dynamic (multi)selection field for LiveView
https://hex.pm/packages/live_select
Apache License 2.0
185 stars 35 forks source link

Upgrading from v1.2.2 to v1.3.1 resets the input value #55

Closed damir closed 8 months ago

damir commented 9 months ago

Hi Max,

I added 200 ms delay in handle_params so it is visible in the recording below.

The log is the same in both versions:

[debug] HANDLE EVENT "option_click" in LiveUIWeb.Admin.UserLive.Index
  Component: LiveSelect.Component
  Parameters: %{"idx" => "0"}
[debug] Replied in 118µs

[debug] HANDLE EVENT "users-update-filter" in LiveUIWeb.Admin.UserLive.Index
  Parameters: %{"_target" => ["company_id"], "company_id" => "1", "company_id_text_input" => "ACME INC", "filters" => %{"0" => %{"field" => "company_id"}, "1" => %{"field" => "email", "op" => "ilike", "value" => ""}}}
[debug] Replied in 173µs

[debug] HANDLE PARAMS in LiveUIWeb.Admin.UserLive.Index
  Parameters: %{"_target" => ["company_id"], "company_id" => "1", "company_id_text_input" => "ACME INC", "filters" => %{"0" => %{"field" => "company_id"}, "1" => %{"field" => "email", "op" => "ilike", "value" => ""}}}
[debug] QUERY OK source="users" db=0.3ms idle=1814.9ms
SELECT ...
[debug] QUERY OK source="users" db=0.2ms idle=1816.1ms
↳ Flop.meta/3, at: lib/flop.ex:929
[debug] Replied in 2ms

It is triggered when the updated stream is added to the socket:

socket |> stream(:items, items, reset: true)

Looks like the whole component is re-rendered:

Screenshot 2024-01-28 at 11 32 33 AM

https://github.com/maxmarcon/live_select/assets/7764/331491fd-2fbc-4bba-b21b-6b5ff668a933

maxmarcon commented 9 months ago

Hi Damir. Can you please share a minimal example (ideally with a repo) that reproduces the issue? Thanks

shamanime commented 9 months ago

I am facing the exact same issue, I have two components on the page and the component is reset suddenly after a few ms. Debugging it right now. Let's see if I can help.

maxmarcon commented 9 months ago

@shamanime thanks, that would be really helpful. If you could create a minimal repo that reproduces the issue, that should be enough for me to dig into it and fix it.

shamanime commented 9 months ago

I've updated this repo to simulate the issue: https://github.com/shamanime/async_stream_test

Run it, add a user, go to the show page, click edit, update cars/fruits and click save. This action won't redirect/patch, instead it will just reload the form and then the live_select options disappear.

I've sent you a DM on Elixir's Slack. Ping me there if you need more details or want to pair on it.

maxmarcon commented 9 months ago

thanks a ton @shamanime ! I can replicate the issue using your repo. I will dig into it soon.

maxmarcon commented 9 months ago

Thanks again for putting together the repo.

The problem here is that live select doesn't know how to map the shape of the AsyncStreamTest.Accounts.Item into an option.

You're working around this problem in your update/2 callback in the FormComponent, however, the callback is no longer being called when the form is simply reloaded, and so the selected options disappear. In any case, having to write this logic in the update/2 callback is cumbersome and counterintuitive.

Live select looks at the value of the form's field, and assumes it's in the expected format for an option. I realize now that this is a bit naive and only works for simple fields like integers and strings, but not for fields whose values can be structs, like in your case.

Perhaps a possible solution could be to provide a value_mapper assign. The user could use the assign to pass a function that maps the value in the form to the value expected by live select.

For example, in your case, something like this:

<.live_select value_mapper={fn %Item{name: name, id: id} -> %{label: name, value: id} end} />

What do you think?

maxmarcon commented 9 months ago

it seems to be more complex than I initially thought. The element of an embedding could also be a changeset .

I need to think about it with more calm. Any idea is welcome!

maxmarcon commented 9 months ago

FYI: Ok I think I wrapped my head around this. Will come back with a plan when I find some time. Stay tuned!

maxmarcon commented 8 months ago

@shamanime I made LiveSelect changeset aware, so it can now handle embedded schemas easily. I also added a value_mapper assign with which you can pass a function to map your embedded schema to LiveSelect options.

These changes aren't in main yet, but in a separate value_mapper branch.

I created a PR for your application that explains and showcases the changes:

https://github.com/shamanime/async_stream_test/pull/1

Everything should be much simpler now and work without problems. There are still some open questions though, I'd be very thankful for your feedback! 🙏

maxmarcon commented 8 months ago

Closed with 1.4.0