Add update_selection/1 to dynamically change the selection #69
Conversation
| @@ -514,19 +527,38 @@ defmodule LiveSelect.Component do | |||
| }) | |||
| end | |||
|
|
|||
| defp update_selection(nil, _current_selection, _options, _mode, _value_mapper), do: [] | |||
| defp set_selection(nil, _current_selection, _options, _mode, _value_mapper), do: [] | |||
There was a problem hiding this comment.
Renamed this function to not cause confusion with the new option, as this function will effectively update the selection as a whole.
There was a problem hiding this comment.
Let us not rename it (see comment above)
| Enum.map(new, &normalize_selection_value(&1, options, value_mapper)) | ||
| |> Enum.reject(&is_nil/1) | ||
|
|
||
| Enum.uniq(existing ++ new) |
There was a problem hiding this comment.
This forces uniqueness. Otherwise appending would allow duplicate values and I couldn't think of a reason to allow it.
There was a problem hiding this comment.
Let's remove this new function, see comment above
|
Thanks @shamanime for your efforts! I will take a look at this soon |
| ## Dynamically updating the selection | ||
|
|
||
| You can also update the selection dynamically by passing an 1 arity function that receives the current selection to `:update_selection`: | ||
|
|
There was a problem hiding this comment.
I would say:
You can dynamically update the selection by using the
:update_selectionassign.:update_selectionmust be a 1-arity function that receives the current selection and returns the new one:
|
|
||
| In this case, only the values with a label longer than 3 characters will be kept in the selection. | ||
|
|
||
| Another example that appends values to the current selection: |
There was a problem hiding this comment.
I don't think we need the second example
| update(socket, :selection, fn | ||
| selection, | ||
| %{update_selection: update_fn, options: options, mode: mode, value_mapper: value_mapper} -> | ||
| update_selection(update_fn, selection, options, mode, value_mapper) |
There was a problem hiding this comment.
My suggestion is to replace this line with:
update_selection(update_fn.(selection), selection, options, mode, value_mapper)
Where update_selection is the old update_selection function, we don't rename it and we don't add the new update_selection function.
We won't have dupe detection but I think it's overkill. We don't have it anyway when you pass the selection explicitly with :value, why have it in the case of :update_selection? The caller can easily check for dupes and remove them if they want to
There was a problem hiding this comment.
I ran into a problem with this and still need to think of a way to figure it out. The update_fn.(selection) may leave mixed entries (current + new) and it fails when trying to ran the existing ones with the value_mapper again. This is why the function I wrote splits it and only "normalize" the new entries.
There was a problem hiding this comment.
Sorry that was not clear, my function fixes the problem by splitting it and selectively running value_mapper only on new values. If we are to revert and use the og update_selection we must address the value_mapper that may encounter already mapped values.
There was a problem hiding this comment.
Thanks for clarifying. Ok, but I don't think the problem is actually fixed by your function. Take this example:
current_selection = [%{value: 1, label: "one"}, %{value: 2, label: "two"}]
value_mapper = fn n -> %{value: n, label: to_string(n)} end
update_fn = fn current_selection -> current_selection ++ [2, 3] endWith this arguments, your update_selection function
will fail to spot that 2 is already in the selection, because it's looking for unmapped values (2,3) in the list of mapped values (line 547).
I can't think of an elegant solution for this. Can you?
My impulse would be to say: it's the responsibility of the writer of update_fn to leave the selection in a state that can be mapped using the value_mapper they provided.
Same for dupe detection.
There was a problem hiding this comment.
Ok I thought about it and I think another, perhaps better option would to not call value_mapper at all when updating the selection programmatically. This makes sense because the caller can map the selection themselves if they want to (i.e. they can provide the selection already in the right format). value_mapper was intended anyway for the case where the user has no way to map the selection (i.e. when it's coming from existing values in the form).
So something like:
update_selection(update_fn.(selection), selection, options, mode, & &1)
| @@ -514,19 +527,38 @@ defmodule LiveSelect.Component do | |||
| }) | |||
| end | |||
|
|
|||
| defp update_selection(nil, _current_selection, _options, _mode, _value_mapper), do: [] | |||
| defp set_selection(nil, _current_selection, _options, _mode, _value_mapper), do: [] | |||
There was a problem hiding this comment.
Let us not rename it (see comment above)
| Enum.map(new, &normalize_selection_value(&1, options, value_mapper)) | ||
| |> Enum.reject(&is_nil/1) | ||
|
|
||
| Enum.uniq(existing ++ new) |
There was a problem hiding this comment.
Let's remove this new function, see comment above
Closes #68.
This allows updating the selection by passing a function with 1 arity to
:update_selection.It can be used to add/filter values from the existing selection without passing the whole selection as a value.