phoenixframework
Composite form inputs don't correctly hide error messages for untouched parts
Environment
- Elixir version (elixir -v): 1.16.0
- Phoenix version (mix deps): 1.7.10
- Phoenix LiveView version (mix deps): 0.20.2
- Operating system: Linux
- Browsers you attempted to reproduce this bug on (the more the merrier): Chromium
- Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: yes
Actual behavior
Inspired by the (old) Phoenix.HTML.Form.datetime_select/3 and motivated by this issue, I created a <.input type="money" ... /> component that should allow inputting a money amount and selecting its currency by using the same approach followed on datetime_select/3 of having composite inputs for a single form value.
The problem here (which I guess is similar to the #2069 one) is that phx-feedback-for is not hiding the error message for untouched inputs within the same component, since phx-feedback-for expects you to provide it a single input name, while the custom component houses more than a single input. This behavior could be achieved by defining the phx-feedback-for for the custom component as well for the existing datetime_select/3 (check the gist below).
https://gist.github.com/Wigny/07800284de5a044204b09e24a810213e
Expected behavior
A good and expected experience would be to display the error message for all inputs derived from a FormField only when an invalid input has already been touched. For the example above, the error message should not be displayed since the only invalid input of the component has not been touched yet, while the other input remains valid.
I'm not sure how feasible it is to address this issue, so any guidance on how to provide a good error message experience in a composed input component like that would be really helpful.
Here's what I do: Inside of your handle_event for your form change, add this code before casting your changeset to a form:
# Remove errors from fields that have changed from the last validation
changeset = Map.put(changeset, :errors, Keyword.drop(changeset.errors, Map.keys(changeset.changes)))
@Wigny I experimented with this an came up with the idea of phx-feedback-group, see https://github.com/phoenixframework/phoenix_live_view/pull/3005. Let me know what you think.
Hey @SteffenDE, could you say if we are expected to have a similar behaviour provided by the now deprecated phx-feeback-group when using the new used_input?/1 helper in those "composite inputs"?
I updated my original gist to use the used_input? function but I still face the same problem as when phx-feedback-for was being used. Is there any plan or is it already possible to tackle this inconvenience with this new helper?
@chrismccord you've had an example for this with used_inputs, or at least you had an idea on how this should be quite trivial iirc. I do think we have a problem here with the current implementation though, as the code for Phoenix.Component.used_inputs? does not expect nested data like
%{
"datetime" => %{
"_unused_day" => "",
"_unused_hour" => "",
"_unused_minute" => "",
"_unused_month" => "",
"_unused_year" => "",
"day" => "21",
"hour" => "12",
"minute" => "59",
"month" => "5",
"year" => "2024"
},
"price" => %{"_unused_amount" => "", "amount" => "", "currency" => "BRN"}
}
There's currently no way I see to use used_input? with a form field that has these params, as used_input?(form[:"price[amount]") would check params["_unused_price[amount]"].
@chrismccord As an example: with phx-feedback-group one was able to use Phoenix.HTML.Form.datetime_select like this:
<div phx-feedback-for={@form[:datetime].name}>
<.label for={@form[:datetime].id}>Datetime</.label>
<div class="mt-2">
<%= Phoenix.HTML.Form.datetime_select(
@form,
:datetime,
Enum.map(~w[year month day hour minute]a, fn select ->
{select,
class:
"rounded-md border border-gray-300 bg-white shadow-sm focus:border-zinc-400 focus:ring-0 sm:text-sm", "phx-feedback-group": @form[:datetime].name}
end)
) %>
</div>
<.error :for={error <- @form[:datetime].errors}><%= translate_error(error) %></.error>
</div>
To translate this to used_input? I'd do something like this:
<div>
<.label for={@form[:datetime].id}>Datetime</.label>
<div class="mt-2">
<%= Phoenix.HTML.Form.datetime_select(
@form,
:datetime,
Enum.map(~w[year month day hour minute]a, fn select ->
{select,
class:
"rounded-md border border-gray-300 bg-white shadow-sm focus:border-zinc-400 focus:ring-0 sm:text-sm"}
end)
) %>
</div>
<.error :for={error <- if(Enum.any?(~w[year month day hour minute]a, fn field -> used_input?(@form[:"datetime[#{field}]"]) end), do: @form[:datetime].errors, else: [])}><%= translate_error(error) %></.error>
</div>
But that won't work for the same reasons as above.
@Wigny you can implement a custom helper (I called it nested_used_input? to check the nested fields:
Application.put_env(:composite_input, CompositeInput.Endpoint,
http: [ip: {127, 0, 0, 1}, port: 4001],
server: true,
render_errors: [
formats: [html: CompositeInput.ErrorHTML],
layout: false
],
live_view: [signing_salt: "aaaaaaaa"],
secret_key_base: String.duplicate("a", 64)
)
Application.put_env(:ex_money, :default_cldr_backend, CompositeInput.Cldr)
Mix.install([
{:plug_cowboy, "~> 2.6"},
{:jason, "~> 1.0"},
{:phoenix, "~> 1.7.10"},
{:phoenix_live_view, "~> 1.0.0-rc.0"},
# {:phoenix_live_view, path: "/Users/steffen/oss/phoenix_live_view", override: true},
{:phoenix_html, "~> 3.0"},
{:phoenix_ecto, "~> 4.4"},
{:ecto, "~> 3.11"},
{:ex_cldr, "~> 2.37"},
{:ex_cldr_numbers, "~> 2.32"},
{:ex_cldr_currencies, "~> 2.15"},
{:ex_money, "~> 5.15", runtime: false},
{:ex_money_sql, "~> 1.10"}
])
defmodule CompositeInput.ErrorHTML do
def render(template, _assigns) do
Phoenix.Controller.status_message_from_template(template)
end
end
defmodule CompositeInput.Layouts do
use Phoenix.Component
def render("root.html", assigns) do
~H"""
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<meta name="csrf-token" content={Phoenix.Controller.get_csrf_token()} />
<.live_title>CompositeInput</.live_title>
<script
src={"https://unpkg.com/phoenix@#{Application.spec(:phoenix, :vsn)}/priv/static/phoenix.min.js"}
>
</script>
<script
src="https://cdn.jsdelivr.net/gh/phoenixframework/phoenix_live_view@main/priv/static/phoenix_live_view.js"
>
</script>
<%!-- <script src="http://localhost:8000/priv/static/phoenix_live_view.js"></script> --%>
<script src="https://cdn.tailwindcss.com?plugins=forms">
</script>
<script type="module">
const liveSocket = new LiveView.LiveSocket("/live", Phoenix.Socket);
liveSocket.connect();
tailwind.config = {
plugins: [
tailwind.plugin(({ addVariant }) => addVariant("phx-click-loading", [".phx-click-loading&", ".phx-click-loading &"])),
tailwind.plugin(({ addVariant }) => addVariant("phx-submit-loading", [".phx-submit-loading&", ".phx-submit-loading &"])),
tailwind.plugin(({ addVariant }) => addVariant("phx-change-loading", [".phx-change-loading&", ".phx-change-loading &"])),
]
};
</script>
</head>
<body>
<%= @inner_content %>
</body>
</html>
"""
end
def render("app.html", assigns) do
~H"""
<main class="px-4 py-20 sm:px-6 lg:px-8">
<div class="mx-auto max-w-2xl">
<%= @inner_content %>
</div>
</main>
"""
end
end
defmodule CompositeInput.Cldr do
use Cldr,
locales: ["en", "pt"],
default_locale: "pt",
providers: [Cldr.Number, Money]
end
defmodule CompositeInput.CoreComponents do
use Phoenix.Component
def button(assigns) do
~H"""
<button
type={@type}
class={[
"phx-submit-loading:opacity-75 rounded-lg bg-zinc-900 hover:bg-zinc-700 py-2 px-3",
"text-sm font-semibold leading-6 text-white active:text-white/80"
]}
>
<%= render_slot(@inner_block) %>
</button>
"""
end
def nested_used_input?(%Phoenix.HTML.FormField{field: field, form: form}, subkey) do
nested_params = Map.get(form.params, to_string(field), :unset)
cond do
nested_params == :unset -> false
not is_map_key(nested_params, "#{subkey}") -> false
is_map_key(nested_params, "_unused_#{subkey}") -> false
true -> true
end
end
def input(%{field: %Phoenix.HTML.FormField{} = field, type: "money"} = assigns) do
errors =
if nested_used_input?(field, :amount) or nested_used_input?(field, :currency) do
Enum.map(field.errors, &translate_error/1)
else
[]
end
assigns =
assigns
|> assign(:field, nil)
|> assign(:errors, errors)
|> assign_new(:id, fn -> field.id end)
|> assign_new(:name, fn -> field.name end)
|> assign_new(:value, fn -> field.value end)
|> update(:value, fn
%{"amount" => amount, "currency" => currency} -> %{amount: amount, currency: currency}
%{amount: amount, currency: currency} -> %{amount: amount, currency: currency}
nil -> nil
end)
|> assign_new(:default_currency, fn ->
backend = Money.default_backend()
Cldr.Currency.current_currency_from_locale(backend.get_locale())
end)
~H"""
<div>
<.label for={@id}><%= @label %></.label>
<div class="relative mt-2 rounded-lg">
<input
type="text"
inputmode="numeric"
id={"#{@id}_amount"}
name={"#{@name}[amount]"}
value={@value[:amount]}
placeholder="0.00"
class={[
"block w-full rounded-lg border-0 pr-20 text-zinc-900 ring-1 ring-inset ring-zinc-300 placeholder:text-zinc-400 focus:ring-1 focus:ring-inset focus:ring-indigo-600 sm:text-sm sm:leading-6",
@errors == [] && "ring-zinc-300 focus:ring-zinc-400",
@errors != [] && "ring-rose-400 focus:ring-rose-400"
]}
/>
<div class="absolute inset-y-0 right-0 flex items-center">
<label for={"#{@id}_currency"} class="sr-only">Currency</label>
<select
id={"#{@id}_currency"}
name={"#{@name}[currency]"}
class="h-full rounded-lg border-0 bg-transparent py-0 pl-2 pr-7 text-zinc-500 focus:ring-1 focus:ring-inset focus:ring-zinc-600 sm:text-sm"
>
<%= Phoenix.HTML.Form.options_for_select(
Money.known_currencies(),
@value[:currency] || @default_currency
) %>
</select>
</div>
</div>
<.error :for={msg <- @errors}><%= msg %></.error>
</div>
"""
end
def label(assigns) do
~H"""
<label for={@for} class="block text-sm font-semibold leading-6 text-zinc-800">
<%= render_slot(@inner_block) %>
</label>
"""
end
def error(assigns) do
~H"""
<p class="mt-3 flex gap-3 text-sm leading-6 text-rose-600 phx-no-feedback:hidden">
<%= render_slot(@inner_block) %>
</p>
"""
end
def translate_error({msg, opts}) do
Regex.replace(~r"%{(\w+)}", msg, fn _, key ->
opts
|> Keyword.get(String.to_existing_atom(key), key)
|> to_string()
end)
end
end
defmodule CompositeInput.HomeLive do
use Phoenix.LiveView, layout: {CompositeInput.Layouts, :app}
import CompositeInput.CoreComponents
defmodule Data do
use Ecto.Schema
import Ecto.Changeset
@primary_key false
embedded_schema do
field(:datetime, :naive_datetime)
field(:price, Money.Ecto.Map.Type)
end
def changeset(data, attrs) do
data
|> cast(attrs, [:datetime, :price])
|> validate_required([:datetime, :price])
|> validate_change(:datetime, fn field, value ->
if NaiveDateTime.compare(value, NaiveDateTime.local_now()) == :gt do
[{field, "can't be in the future"}]
else
[]
end
end)
end
end
def render(assigns) do
~H"""
<.form for={@form} phx-change="validate" phx-submit="save" class="mt-10 space-y-8 bg-white">
<.input type="money" field={@form[:price]} label="Price" />
<div>
<.label for={@form[:datetime].id}>Datetime</.label>
<div class="mt-2">
<%= Phoenix.HTML.Form.datetime_select(
@form,
:datetime,
Enum.map(~w[year month day hour minute]a, fn select ->
{select,
class:
"rounded-md border border-gray-300 bg-white shadow-sm focus:border-zinc-400 focus:ring-0 sm:text-sm"}
end)
) %>
</div>
<.error :for={
error <-
if(
Enum.any?(~w[year month day hour minute]a, fn field ->
@form && nested_used_input?(@form[:datetime], field)
end),
do: @form[:datetime].errors,
else: []
)
}>
<%= translate_error(error) %>
</.error>
</div>
<div class="mt-2 flex items-center justify-between gap-6">
<.button type="submit">
Submit
</.button>
</div>
</.form>
"""
end
def mount(_params, _session, socket) do
data = %Data{}
changeset =
Data.changeset(data, %{datetime: NaiveDateTime.add(NaiveDateTime.local_now(), 1, :day)})
{:ok,
socket
|> assign(:data, data)
|> assign(:form, to_form(changeset))}
end
def handle_event("validate", %{"data" => data_params}, socket) do
changeset = Data.changeset(socket.assigns.data, data_params)
{:noreply, assign(socket, :form, to_form(%{changeset | action: :validate}))}
end
def handle_event("save", %{"data" => data_params}, socket) do
changeset = Data.changeset(socket.assigns.data, data_params)
case Ecto.Changeset.apply_action(changeset, :insert) do
{:ok, data} -> {:noreply, assign(socket, :data, data)}
{:error, changeset} -> {:noreply, assign(socket, :form, to_form(changeset))}
end
end
end
defmodule CompositeInput.Router do
use Phoenix.Router
import Phoenix.LiveView.Router
pipeline :browser do
plug(:accepts, ["html"])
plug(:put_root_layout, {CompositeInput.Layouts, :root})
end
scope "/", CompositeInput do
pipe_through(:browser)
live("/", HomeLive, :index)
end
end
defmodule CompositeInput.Endpoint do
use Phoenix.Endpoint, otp_app: :composite_input
socket("/live", Phoenix.LiveView.Socket)
plug(CompositeInput.Router)
end
{:ok, _} = Supervisor.start_link([CompositeInput.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity)
I'm not sure if this is the best solution here though. We should probably make this more convenient.
Hey @SteffenDE, thanks for the suggestion. I think this small helper can give us better control of whether to show error messages.
We should probably make this more convenient.
Agreed that it would be great, but I appreciate your effort on that nonetheless, thanks.
You can now simply check nested inputs for usage, such as your examples:
used_input?(@form[:datetime])
used_input?(@form[:money])
Thanks!