-
Notifications
You must be signed in to change notification settings - Fork 57
Description
I love what you've done with this terrific component, so grateful ❤️ Amazing work.
I'm humbly opening this issue in the hope of tracking a discussion regarding possible adoption of the upcoming used_input?/2 API for conditionally showing error messages depending on whether it is time to show them or not.
LiveSelect and LiveView versions
live_select: 1.4.2phoenix_live_view: 1.0.0-rc.7
Describe the bug
When passing the field.errors to the component after first having checked whether the field is considered to have been used with Phoenix.Component.used_input?/2 function, the error is shown since the field won't match what is expected by LiveView's new logic.
Please consider the following component from the README adapted to operate like the new Phoenix LiveView 1.0 core components for inputs.
attr :id, :any
attr :label, :string
attr :update_min_len, :integer
attr :field, Phoenix.HTML.FormField
attr :errors, :list, default: []
def live_select(%{field: %Phoenix.HTML.FormField{} = field} = assigns) do
# This is practical for debugging since the `_unused*` parameters determine the logic for `used_input?/2`
field.form.params |> dbg()
errors =
if Phoenix.Component.used_input?(field),
do: field.errors,
else: []
assigns =
assigns
|> assign(:errors, Enum.map(errors, &translate_error(&1)))
|> assign(:live_select_opts, assigns_to_attributes(assigns, [:errors, :label]))
~H"""
<div>
<.label for={@field.id}><%= @label %></.label>
<LiveSelect.live_select
field={@field}
text_input_class={[
"mt-2 block w-full rounded-lg border-zinc-300 py-[7px] px-[11px]",
"text-zinc-900 focus:outline-none focus:ring-4 sm:text-sm sm:leading-6",
"border-zinc-300 focus:border-zinc-400 focus:ring-zinc-800/5",
@errors != [] && "border-rose-400 focus:border-rose-400 focus:ring-rose-400/10"
]}
{@live_select_opts}
/>
<.error :for={msg <- @errors}><%= msg %></.error>
</div>
"""
endIn a simple form with the fields name and manager the LiveSelect would have a hidden input with the field manager_text_input.
Looking at the params passed in with field.form.params, the first validation of the form after entering text into the name input would result in the following params.
field.form.params #=> %{
"_unused_manager_text_input" => "",
"manager" => "",
"manager_text_input" => "",
"name" => "a",
}Since calling unused_input?/2 with the field as it is would check for any parameter _unused_manager but not being able to find any such parameter, the LiveSelect is incorrectly considered to be used even though the LiveSelect field manager_text_input actually has not been used yet.
For reference, see the implementation of used_input?/2:
def used_input?(%Phoenix.HTML.FormField{field: field, form: form}) do
used_param?(form.params, field)
end
defp used_param?(_params, "_unused_" <> _), do: false
defp used_param?(params, field) do
field_str = "#{field}"
unused_field_str = "_unused_#{field}"
case params do
%{^field_str => _, ^unused_field_str => _} -> false
%{^field_str => %{} = nested} -> Enum.any?(Map.keys(nested), &used_param?(nested, &1))
%{^field_str => _val} -> true
%{} -> false
end
endExpected behavior
Technically not expected behaviour since the issue concerns a breaking change, but once adapted to the new LiveView interface the expected behaviour would be:
- Interacting with the field in a way that would consider it as "used" shows any present errors
- Editing another input field does not result in the error being shown
Actual behavior
Editing another field causes the LiveSelect field to immediately be considered as used, thus rendering any error even though the time is not right for it to be shown.
Screenshots
None available.
Browsers
Edge
Issue Repo
I'd be happy to provide one if requested.
Additional context
I've been looking into composite fields before but I believe that situation is different since there is no hidden field in that scenario. But if it helps, here is a rather lengthy issue with a final comment form Mr. McCord: