-
Notifications
You must be signed in to change notification settings - Fork 959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Radio input checked state regression between v1.0.2..v1.0.3 #3684
Comments
Calling this out again because I'm not sure my "edit" to the issue will send a DM notification (if it did already, apologies for any annoyance 🙃) -- Update for @SteffenDE the bug rode in on this commit: |
Thank you, I’ll investigate! |
@hattmarris can you provide some details on how your radio setup looks like? If you can provide something to reproduce, that would be best. Here's an example radio setup that works as expected, so you can see what the differences are and maybe adapt it to show your problem: Application.put_env(:sample, Example.Endpoint,
http: [ip: {127, 0, 0, 1}, port: 5001],
server: true,
live_view: [signing_salt: "aaaaaaaa"],
secret_key_base: String.duplicate("a", 64)
)
Mix.install([
{:plug_cowboy, "~> 2.5"},
{:jason, "~> 1.0"},
{:phoenix, "~> 1.7"},
# please test your issue using the latest version of LV from GitHub!
{:phoenix_live_view,
github: "phoenixframework/phoenix_live_view", branch: "main", override: true},
{:ecto, "~> 3.11"},
{:phoenix_ecto, "~> 4.5"}
])
# build the LiveView JavaScript assets (this needs mix and npm available in your path!)
path = Phoenix.LiveView.__info__(:compile)[:source] |> Path.dirname() |> Path.join("../")
System.cmd("mix", ["deps.get"], cd: path, into: IO.binstream())
System.cmd("npm", ["install"], cd: Path.join(path, "./assets"), into: IO.binstream())
System.cmd("mix", ["assets.build"], cd: path, into: IO.binstream())
defmodule Example.ErrorView do
def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end
defmodule Example.HomeLive do
use Phoenix.LiveView, layout: {__MODULE__, :live}
defp changeset(params) do
data = %{}
types = %{items: {:array, :string}, name: :string, radio: :string, select: :string}
{data, types}
|> Ecto.Changeset.cast(params, Map.keys(types))
|> Ecto.Changeset.validate_required(:items)
|> Ecto.Changeset.validate_length(:items, min: 1)
|> Ecto.Changeset.validate_required(:radio)
|> Ecto.Changeset.validate_required(:select)
end
def mount(_params, _session, socket) do
{:ok, assign(socket, form: to_form(changeset(%{}), as: :foo), payload: nil)}
end
def handle_event("validate", %{"foo" => params}, socket) do
changeset = changeset(params)
{:noreply,
assign(socket, form: to_form(changeset, as: :foo, action: :validate), payload: params)}
end
def handle_event("submit", %{"foo" => params}, socket) do
changeset = changeset(params)
{:noreply,
assign(socket, form: to_form(changeset, as: :foo, action: :submit), payload: params)}
end
def render("live.html", assigns) do
~H"""
<script src="/assets/phoenix/phoenix.js">
</script>
<script src="/assets/phoenix_live_view/phoenix_live_view.js">
</script>
<%!-- uncomment to use enable tailwind --%>
<script src="https://cdn.tailwindcss.com">
</script>
<script>
let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
liveSocket.connect()
</script>
{@inner_content}
"""
end
def render(assigns) do
~H"""
<.form
for={@form}
id="foo"
phx-change="validate"
phx-submit="submit"
class="max-w-lg p-8 flex flex-col gap-4"
>
<.input type="text" field={@form[:name]} />
<hr />
<fieldset>
<input type="hidden" name={@form[:radio].name} value="" />
<legend>Radio example:</legend>
<div>
<input
type="radio"
id="huey"
name={@form[:radio].name}
value="huey"
checked={@form[:radio].value === "huey"}
/>
<label for="huey">Huey</label>
</div>
<div>
<input
type="radio"
id="dewey"
name={@form[:radio].name}
value="dewey"
checked={@form[:radio].value === "dewey"}
/>
<label for="dewey">Dewey</label>
</div>
<div>
<input
type="radio"
id="louie"
name={@form[:radio].name}
value="louie"
checked={@form[:radio].value === "louie"}
/>
<label for="louie">Louie</label>
</div>
<%= if used_input?(@form[:radio]) do %>
<.error :for={msg <- @form[:radio].errors}>{inspect(msg)}</.error>
<% end %>
</fieldset>
<hr />
<p>Select multiple example:</p>
<input type="hidden" value="" name={@form[:items].name <> "[]"} />
<.input field={@form[:items]} type="select" multiple options={["foo", "bar", "baz"]} />
<p>Select example:</p>
<.input
field={@form[:select]}
type="select"
required
prompt="Choose an item"
options={["foo", "bar", "baz"]}
/>
<button type="submit">Submit</button>
<hr />
<div class="flex flex-col gap-4">
<p>Payload:</p>
<pre><%= inspect(@payload) %></pre>
<p>Selected items: {inspect(@form[:items].value)}</p>
<p>Selected radio: {inspect(@form[:radio].value)}</p>
<p>Used input @form[:name]: {used_input?(@form[:name])}</p>
<p>Used input @form[:radio]: {used_input?(@form[:radio])}</p>
<p>Used input @form[:items]: {used_input?(@form[:items])}</p>
<p>Form errors: {inspect(@form.errors)}</p>
</div>
</.form>
"""
end
@doc """
Renders an input with label and error messages.
A `Phoenix.HTML.FormField` may be passed as argument,
which is used to retrieve the input name, id, and values.
Otherwise all attributes may be passed explicitly.
## Types
This function accepts all HTML input types, considering that:
* You may also set `type="select"` to render a `<select>` tag
* `type="checkbox"` is used exclusively to render boolean values
* For live file uploads, see `Phoenix.Component.live_file_input/1`
See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input
for more information. Unsupported types, such as hidden and radio,
are best written directly in your templates.
## Examples
<.input field={@form[:email]} type="email" />
<.input name="my-input" errors={["oh no!"]} />
"""
attr :id, :any, default: nil
attr :name, :any
attr :label, :string, default: nil
attr :value, :any
attr :type, :string,
default: "text",
values: ~w(checkbox color date datetime-local email file month number password
range search select tel text textarea time url week)
attr :field, Phoenix.HTML.FormField,
doc: "a form field struct retrieved from the form, for example: @form[:email]"
attr :errors, :list, default: []
attr :checked, :boolean, doc: "the checked flag for checkbox inputs"
attr :prompt, :string, default: nil, doc: "the prompt for select inputs"
attr :options, :list, doc: "the options to pass to Phoenix.HTML.Form.options_for_select/2"
attr :multiple, :boolean, default: false, doc: "the multiple flag for select inputs"
attr :rest, :global,
include: ~w(accept autocomplete capture cols disabled form list max maxlength min minlength
multiple pattern placeholder readonly required rows size step)
slot :inner_block
def input(%{field: %Phoenix.HTML.FormField{} = field} = assigns) do
errors = if Phoenix.Component.used_input?(field), do: field.errors, else: []
assigns
|> assign(field: nil, id: assigns.id || field.id)
|> assign(:errors, errors)
|> assign_new(:name, fn -> if assigns.multiple, do: field.name <> "[]", else: field.name end)
|> assign_new(:value, fn -> field.value end)
|> input()
end
def input(%{type: "checkbox"} = assigns) do
assigns =
assign_new(assigns, :checked, fn ->
Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value])
end)
~H"""
<div>
<label class="flex items-center gap-4 text-sm leading-6 text-zinc-600">
<input type="hidden" name={@name} value="false" />
<input
type="checkbox"
id={@id}
name={@name}
value="true"
checked={@checked}
class="rounded border-zinc-300 text-zinc-900 focus:ring-0"
{@rest}
/>
{@label}
</label>
<.error :for={msg <- @errors}>{inspect(msg)}</.error>
</div>
"""
end
def input(%{type: "select"} = assigns) do
~H"""
<div>
<.label for={@id}>{@label}</.label>
<select
id={@id}
name={@name}
class="mt-2 block w-full rounded-md border border-gray-300 bg-white shadow-sm focus:border-zinc-400 focus:ring-0 sm:text-sm"
multiple={@multiple}
{@rest}
>
<option :if={@prompt} value="" selected>{@prompt}</option>
{Phoenix.HTML.Form.options_for_select(@options, @value)}
</select>
<.error :for={msg <- @errors}>{inspect(msg)}</.error>
</div>
"""
end
def input(%{type: "textarea"} = assigns) do
~H"""
<div>
<.label for={@id}>{@label}</.label>
<textarea
id={@id}
name={@name}
class={[
"mt-2 block w-full rounded-lg text-zinc-900 focus:ring-0 sm:text-sm sm:leading-6 min-h-[6rem]",
@errors == [] && "border-zinc-300 focus:border-zinc-400",
@errors != [] && "border-rose-400 focus:border-rose-400"
]}
{@rest}
><%= Phoenix.HTML.Form.normalize_value("textarea", @value) %></textarea>
<.error :for={msg <- @errors}>{inspect(msg)}</.error>
</div>
"""
end
# All other inputs text, datetime-local, url, password, etc. are handled here...
def input(assigns) do
~H"""
<div>
<.label for={@id}>{@label}</.label>
<input
type={@type}
name={@name}
id={@id}
value={Phoenix.HTML.Form.normalize_value(@type, @value)}
class={[
"mt-2 block w-full border rounded-lg text-zinc-900 focus:ring-0 sm:text-sm sm:leading-6",
@errors == [] && "border-zinc-300 focus:border-zinc-400",
@errors != [] && "border-rose-400 focus:border-rose-400"
]}
{@rest}
/>
<.error :for={msg <- @errors}>{inspect(msg)}</.error>
</div>
"""
end
@doc """
Renders a label.
"""
attr :for, :string, default: nil
slot :inner_block, required: true
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
@doc """
Generates a generic error message.
"""
slot :inner_block, required: true
def error(assigns) do
~H"""
<p class="mt-3 flex gap-3 text-sm leading-6 text-rose-600">
{render_slot(@inner_block)}
</p>
"""
end
end
defmodule Example.Router do
use Phoenix.Router
import Phoenix.LiveView.Router
pipeline :browser do
plug(:accepts, ["html"])
end
scope "/", Example do
pipe_through(:browser)
live("/", HomeLive, :index)
end
end
defmodule Example.Endpoint do
use Phoenix.Endpoint, otp_app: :sample
socket("/live", Phoenix.LiveView.Socket)
plug Plug.Static, from: {:phoenix, "priv/static"}, at: "/assets/phoenix"
plug Plug.Static, from: {:phoenix_live_view, "priv/static"}, at: "/assets/phoenix_live_view"
plug(Example.Router)
end
{:ok, _} = Supervisor.start_link([Example.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity) |
Thanks @SteffenDE ! I'll look into trying to replicate with a minimal setup (haven't done this before, so may take a bit) but in the meantime I can describe the most consistent reproduction: A LiveComponent <.award_type_radios award_type={@award_type} form={@form} badge={@badge} myself={@myself} /> The actual radio inputs do not call any Phoenix CoreComponent, but are just rendered in a loop as HTML inputs: <%= for award_type <- Badge.award_type_values() do %>
<div
class="flex items-center gap-4"
phx-click="change-award-type"
phx-value-award-type={award_type}
phx-target={@myself}
>
<input
type="radio"
id={award_type}
name="award-type"
value={award_type}
checked={(@award_type == award_type) |> IO.inspect(label: "checked")}
required
/>
</div>
<% end %> There are currently 2 award_type_values -- Most interesting difference I can think of between example you posted and our setup is that it's a Hope that helps, I'll look into reproducing more minimally on my end 👍 |
Okay @SteffenDE I think we've got a minimal reproduction here -- the situation occurs when radio inputs are children of a parent form with a We confirmed this example works in Application.put_env(:sample, Example.Endpoint,
http: [ip: {127, 0, 0, 1}, port: 5001],
server: true,
live_view: [signing_salt: "aaaaaaaa"],
secret_key_base: String.duplicate("a", 64)
)
Mix.install([
{:plug_cowboy, "~> 2.5"},
{:jason, "~> 1.0"},
{:phoenix, "~> 1.7"},
# please test your issue using the latest version of LV from GitHub!
{:phoenix_live_view,
github: "phoenixframework/phoenix_live_view", branch: "main", override: true},
{:ecto, "~> 3.11"},
{:phoenix_ecto, "~> 4.5"}
])
# build the LiveView JavaScript assets (this needs mix and npm available in your path!)
path = Phoenix.LiveView.__info__(:compile)[:source] |> Path.dirname() |> Path.join("../")
System.cmd("mix", ["deps.get"], cd: path, into: IO.binstream())
System.cmd("npm", ["install"], cd: Path.join(path, "./assets"), into: IO.binstream())
System.cmd("mix", ["assets.build"], cd: path, into: IO.binstream())
defmodule Example.ErrorView do
def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end
defmodule Example.BadgeForm do
use Phoenix.LiveComponent
def mount(socket) do
socket =
socket
|> assign(:type, :huey)
{:ok, socket}
end
def update(assigns, socket) do
socket =
socket
|> assign(:form, assigns.form)
{:ok, socket}
end
def render(assigns) do
~H"""
<div>
<.form
for={@form}
id="foo"
class="max-w-lg p-8 flex flex-col gap-4"
phx-change="change"
phx-submit="submit"
>
<.radios type={@type} form={@form} myself={@myself} />
</.form>
</div>
"""
end
defp radios(assigns) do
~H"""
<fieldset>
<legend>Radio example:</legend>
<%= for type <- [:huey, :dewey] do %>
<div
phx-click="change-type"
phx-value-type={type}
phx-target={@myself}
>
<input
type="radio"
id={type}
name="type"
value={type}
checked={(@type == type) |> IO.inspect(label: "checked")}
/>
<label for={type}>{type}</label>
</div>
<% end %>
</fieldset>
"""
end
def handle_event("change-type", %{"type" => type}, socket) do
type = String.to_existing_atom(type)
socket = assign(socket, :type, type)
{:noreply, socket}
end
end
defmodule Example.HomeLive do
use Phoenix.LiveView, layout: {__MODULE__, :live}
defp changeset(params) do
data = %{}
types = %{
type: :string
}
{data, types}
|> Ecto.Changeset.cast(params, Map.keys(types))
|> Ecto.Changeset.validate_required(:type)
end
def mount(_params, _session, socket) do
{:ok, assign(socket, form: to_form(changeset(%{}), as: :foo), payload: nil)}
end
def render("live.html", assigns) do
~H"""
<script src="/assets/phoenix/phoenix.js">
</script>
<script src="/assets/phoenix_live_view/phoenix_live_view.js">
</script>
<%!-- uncomment to use enable tailwind --%>
<script src="https://cdn.tailwindcss.com">
</script>
<script>
let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
liveSocket.connect()
</script>
{@inner_content}
"""
end
def render(assigns) do
~H"""
<.live_component
id="badge_form"
module={Example.BadgeForm}
action={@live_action}
form={@form}
/>
"""
end
def handle_event("change", params, socket) do
{:noreply, socket}
end
def handle_event("submit", params, socket) do
{:noreply, socket}
end
end
defmodule Example.Router do
use Phoenix.Router
import Phoenix.LiveView.Router
pipeline :browser do
plug(:accepts, ["html"])
end
scope "/", Example do
pipe_through(:browser)
live("/", HomeLive, :index)
end
end
defmodule Example.Endpoint do
use Phoenix.Endpoint, otp_app: :sample
socket("/live", Phoenix.LiveView.Socket)
plug(Plug.Static, from: {:phoenix, "priv/static"}, at: "/assets/phoenix")
plug(Plug.Static, from: {:phoenix_live_view, "priv/static"}, at: "/assets/phoenix_live_view")
plug(Example.Router)
end
{:ok, _} = Supervisor.start_link([Example.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity) |
Perfect, thank you! |
Sure thing! And FWIW - I'm realizing this is definitely a case of live view changes surfacing a questionable pattern in our code too, so I really appreciate your time. As an immediate fix, we're going to refactor code in our app to match on |
Fixes: #3684. Relates to: #3652 The issue was that when we locked a form with an input inside where the input changed, the form was cloned and then later the input inside was cloned as well, stored inside the already cloned parent. When undoing, previously the simple patch copied over the PHX_PRIVATE of the cloned elements, but with the full DOMPatch, the privates of the source element were copied, discarding the clone of the input. With this change, we check if we are undoing locks and manually copy over nested clones to ensure that they can be correctly applied afterwards.
Fixes: #3684. Relates to: #3652 The issue was that when we locked a form with an input inside where the input changed, the form was cloned and then later the input inside was cloned as well, stored inside the already cloned parent. When undoing, previously the simple patch copied over the PHX_PRIVATE of the cloned elements, but with the full DOMPatch, the privates of the source element were copied, discarding the clone of the input. With this change, we check if we are undoing locks and manually copy over nested clones to ensure that they can be correctly applied afterwards.
* recursively undo cloned trees Fixes: #3684. Relates to: #3652 The issue was that when we locked a form with an input inside where the input changed, the form was cloned and then later the input inside was cloned as well, stored inside the already cloned parent. When undoing, previously the simple patch copied over the PHX_PRIVATE of the cloned elements, but with the full DOMPatch, the privates of the source element were copied, discarding the clone of the input. With this change, we check if we are undoing locks and manually copy over nested clones to ensure that they can be correctly applied afterwards. * lint
* recursively undo cloned trees Fixes: #3684. Relates to: #3652 The issue was that when we locked a form with an input inside where the input changed, the form was cloned and then later the input inside was cloned as well, stored inside the already cloned parent. When undoing, previously the simple patch copied over the PHX_PRIVATE of the cloned elements, but with the full DOMPatch, the privates of the source element were copied, discarding the clone of the input. With this change, we check if we are undoing locks and manually copy over nested clones to ensure that they can be correctly applied afterwards. * lint
Environment
1.18.2
1.7.19
1.0.4
Linux Ubuntu 24.04
Actual behavior
We are encountering a bug with radio type inputs checked state that seems to be a regression in version
1.0.3
. The back end state bound to the checked attribute logs correctly, but the DOM patching does not correctly update the input checked state toggling between two radios.I've isolated to have occurred in changes between
v1.0.2..v1.0.3
(exists in >= 1.0.3 only). I can't replicate the error in1.0.2
or lower, so I'm going to start combing through those changes to see which commit may be related. Will report back with any more info.Here's the bug (v1.0.3 and v1.0.4):
Screencast.from.2025-02-19.14-17-26.webm
Expected behavior
Here's expected (<= v1.0.2):
Screencast.from.2025-02-19.14-20-07.webm
Update for @SteffenDE the bug rode in on this commit:
ad9beca
The text was updated successfully, but these errors were encountered: