maxmarcon / live_select

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

Adds `:filter_values` option to conditionally keep or remove items from the selection #64

Closed shamanime closed 3 months ago

shamanime commented 5 months ago

This is based off #63 this is why it has both changes.

This allows running a function against selected values to keep/discard items.

This is the motivation behind this change:

We allow the user to "tag" other users when creating some records using LiveSelect. There is a field that controls if the record is "sensitive". When that field changes, if it changes to "sensitive", we must remove previously "tagged" users that can't see sensitive information.

maxmarcon commented 3 months ago

Hi and sorry for reacting so late.

In the spirit of keeping the API small, can't we achieve this by filtering the options available to the user and/or programmatically reset the selection using:

send_update(LiveSelect.Component, id: live_select_id, value: filtered_selection)

as I was suggesting in #63 ?

shamanime commented 3 months ago

Hi and sorry for reacting so late.

In the spirit of keeping the API small, can't we achieve this by filtering the options available to the user and/or programmatically reset the selection using:

send_update(LiveSelect.Component, id: live_select_id, value: filtered_selection)

as I was suggesting in #63 ?

No worries.

That's a good suggestion. I didn't consider doing that because I've got a few scenarios with forms having multiple LiveSelects as the source of truth and I'm not keeping the selection state on the LiveView/Component anymore (we were doing that before and it was the main reason to start using LiveSelect), so when the feature was requested and we figured out the events/API that we wanted for the interactions, it felt that the right way was to delegate it to the library because it has such a nice API and makes the code much more cohesive.

The main benefit to me is to keep the LiveView code easy to reason about. Right now we have a very clear flow: Anything happens on the LV -> New options/append/filter_selection [without knowing the current state] are sent to LiveSelect -> LV receives the new updated selection from LiveSelect and updates the from it. It works wonders. By bringing the selection back to the LV we open the door to directly use it (guilty as charged), and end up with two places holding the same state that we may forget to keep in sync.

However I do understand you wanting to keep the API small and there's no hurt feelings if you think this is out of scope.

maxmarcon commented 3 months ago

Thanks for the explanation. I'm starting to see the value in this approach.

Instead of using 2 new options, one to pass a filter and the other to append to the selection, my suggestion would be to generalize the approach by passing an update_selection/1 function that receives the whole selection and returns the new one:

append case:

values_to_append = [1, 2, 3]

send_update(LiveSelect.Component, id: live_select_id, update_selection: fn selection -> selection ++ values_to_append end)

filter case:

send_update(LiveSelect.Component, id: live_select_id, update_selection: fn selection -> Enum.filter(selection, & &1 > 2)
end)

2 birds with a stone. And this general approach will hopefully accommodate future use cases.

What do you think?

EDIT: this would also make sense in single mode

shamanime commented 3 months ago

I think that does make a lot of sense.

I've got a few busy weeks ahead but would love to give it a shot whenever I can if you don't mind.

Would you rather have me update this or open a new PR?

maxmarcon commented 3 months ago

I think it's better to close these 2 PRs and open a new one that references both.

I've got a few busy weeks ahead but would love to give it a shot whenever I can if you don't mind.

That sounds great! Thanks

shamanime commented 3 months ago

Closed in favor of https://github.com/maxmarcon/live_select/issues/68