Skip to content

Commit

Permalink
recursively undo cloned trees
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SteffenDE committed Feb 25, 2025
1 parent 573141b commit 76ece61
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 2 deletions.
8 changes: 6 additions & 2 deletions assets/js/phoenix_live_view/dom_patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export default class DOMPatch {
let isFocusedFormEl = focused && fromEl.isSameNode(focused) && DOM.isFormInput(fromEl)
let focusedSelectChanged = isFocusedFormEl && this.isChangedSelect(fromEl, toEl)
// only perform the clone step if this is not a patch that unlocks
if(fromEl.hasAttribute(PHX_REF_SRC) && fromEl.getAttribute(PHX_REF_LOCK) != this.undoRef){
if(fromEl.hasAttribute(PHX_REF_SRC) && !this.undoRef){
if(DOM.isUploadInput(fromEl)){
DOM.mergeAttrs(fromEl, toEl, {isIgnored: true})
this.trackBefore("updated", fromEl, toEl)
Expand All @@ -245,7 +245,11 @@ export default class DOMPatch {
return false
}

// input handling
// if we are undoing a lock, copy potentially nested clones over
if(this.undoRef && DOM.private(toEl, PHX_REF_LOCK)) {

Check failure on line 249 in assets/js/phoenix_live_view/dom_patch.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Unexpected space before opening brace

Check failure on line 249 in assets/js/phoenix_live_view/dom_patch.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Unexpected space before opening brace
DOM.putPrivate(fromEl, PHX_REF_LOCK, DOM.private(toEl, PHX_REF_LOCK))
}
// now copy regular DOM.private data
DOM.copyPrivates(toEl, fromEl)

// skip patching focused inputs unless focus is a select that has changed options
Expand Down
105 changes: 105 additions & 0 deletions test/e2e/support/issues/issue_3684.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
defmodule Phoenix.LiveViewTest.E2E.Issue3684Live do
# https://github.com/phoenixframework/phoenix_live_view/issues/3684
use Phoenix.LiveView

defmodule 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}
/>
<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

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(assigns) do
~H"""
<.live_component
id="badge_form"
module={__MODULE__.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
1 change: 1 addition & 0 deletions test/e2e/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
live "/3651", Issue3651Live
live "/3656", Issue3656Live
live "/3658", Issue3658Live
live "/3684", Issue3684Live
end
end

Expand Down
15 changes: 15 additions & 0 deletions test/e2e/tests/issues/3684.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const {test, expect} = require("../../test-fixtures")
const {syncLV} = require("../../utils")

// https://github.com/phoenixframework/phoenix_live_view/issues/3684
test("nested clones are correctly applied", async ({page}) => {
await page.goto("/issues/3684")
await syncLV(page)

await expect(page.locator("#dewey")).not.toHaveAttribute("checked")

await page.locator("#dewey").click();

Check failure on line 11 in test/e2e/tests/issues/3684.spec.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Extra semicolon

Check failure on line 11 in test/e2e/tests/issues/3684.spec.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Extra semicolon
await syncLV(page);

Check failure on line 12 in test/e2e/tests/issues/3684.spec.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Extra semicolon

Check failure on line 12 in test/e2e/tests/issues/3684.spec.js

View workflow job for this annotation

GitHub Actions / npm test (1.18.1, 27.2)

Extra semicolon

await expect(page.locator("#dewey")).toHaveAttribute("checked")
})

0 comments on commit 76ece61

Please sign in to comment.