Skip to content

Commit

Permalink
Form serialization is hard, kinda (#3069)
Browse files Browse the repository at this point in the history
So... When trying to fix #3056, Chris changed the way the parameters
are built in serializeForm. This broke the way we were handling
select elements with the "multiple" option. I tried to fix this in #3067,
but actually broke Chris' code.
This commit refactors the serializeForm function to actually add a
temporary input element to the form to properly encode the values using
the `FormData` API. This fixes the issue with the multiple select and also
ensure that we send the value of the submitter at the correct position.

Also adds some tests that were harder to build than I'd like to admit,
because doing Ecto sort_param/drop_param stuff without Ecto is not trivial...

Thanks for reading!
  • Loading branch information
SteffenDE committed Feb 1, 2024
1 parent 69cea0c commit 7d19dea
Show file tree
Hide file tree
Showing 5 changed files with 267 additions and 32 deletions.
51 changes: 32 additions & 19 deletions assets/js/phoenix_live_view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,28 @@ import ViewHook from "./view_hook"
import JS from "./js"

let serializeForm = (form, metadata, onlyNames = []) => {
let {submitter, ...meta} = metadata
let formData = new FormData(form)
let toRemove = []
const {submitter, ...meta} = metadata

// We must inject the submitter in the order that it exists in the DOM
// releative to other inputs. For example, for checkbox groups, the order must be maintained.
let injectedElement
if(submitter && submitter.name){
const input = document.createElement("input")
input.type = "hidden"
// set the form attribute if the submitter has one;
// this can happen if the element is outside the actual form element
const formId = submitter.getAttribute("form")
if(formId){
input.setAttribute("form", form)
}
input.name = submitter.name
input.value = submitter.value
submitter.parentElement.insertBefore(input, submitter)
injectedElement = input
}

const formData = new FormData(form)
const toRemove = []

formData.forEach((val, key, _index) => {
if(val instanceof File){ toRemove.push(key) }
Expand All @@ -67,24 +86,18 @@ let serializeForm = (form, metadata, onlyNames = []) => {
// Cleanup after building fileData
toRemove.forEach(key => formData.delete(key))

let params = new URLSearchParams()
// Go through form els in order to mirror ordered generation of formData.entries().
// We must traverse elements in order manually so that we append the submitter in
// the order that it exists in the DOM releative to other inputs. For example,
// for checkbox groups, the order must be maintained. Likewise for checkboxes,
// we can only append to params if the checkbox is checked, so we use formData.getAll()
// to check if the current element value exists in the form data.
Array.from(form.elements).forEach(el => {
if(el.name && onlyNames.length === 0 || onlyNames.indexOf(el.name) >= 0){
const values = formData.getAll(el.name)
if((el.name && values.indexOf(el.value) >= 0) || submitter === el){
values.forEach(val => params.append(el.name, val))
}
const params = new URLSearchParams()

for(let [key, val] of formData.entries()){
if(onlyNames.length === 0 || onlyNames.indexOf(key) >= 0){
params.append(key, val)
}
})
}

if(submitter && submitter.name && !params.has(submitter.name)){
params.append(submitter.name, submitter.value)
// remove the injected element again
// (it would be removed by the next dom patch anyway, but this is cleaner)
if(submitter && injectedElement){
submitter.parentElement.removeChild(injectedElement)
}

for(let metaKey in meta){ params.append(metaKey, meta[metaKey]) }
Expand Down
2 changes: 1 addition & 1 deletion lib/phoenix_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2346,7 +2346,7 @@ defmodule Phoenix.Component do
<button type="button" name="mailing_list[emails_sort][]" value="new" phx-click={JS.dispatch("change")}>
add more
</label>
</button>
```
We used `inputs_for` to render inputs for the `:emails` association, which
Expand Down
25 changes: 13 additions & 12 deletions test/e2e/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,26 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
end

live_session :default, layout: {Phoenix.LiveViewTest.E2E.Layout, :live} do
scope "/" do
scope "/", Phoenix.LiveViewTest do
pipe_through(:browser)

live "/stream", Phoenix.LiveViewTest.StreamLive
live "/stream/reset", Phoenix.LiveViewTest.StreamResetLive
live "/stream/reset-lc", Phoenix.LiveViewTest.StreamResetLCLive
live "/stream/limit", Phoenix.LiveViewTest.StreamLimitLive
live "/healthy/:category", Phoenix.LiveViewTest.HealthyLive
live "/stream", StreamLive
live "/stream/reset", StreamResetLive
live "/stream/reset-lc", StreamResetLCLive
live "/stream/limit", StreamLimitLive
live "/healthy/:category", HealthyLive

live "/upload", Phoenix.LiveViewTest.E2E.UploadLive
live "/form", Phoenix.LiveViewTest.E2E.FormLive
live "/js", Phoenix.LiveViewTest.E2E.JsLive
live "/upload", E2E.UploadLive
live "/form", E2E.FormLive
live "/form/dynamic-inputs", E2E.FormDynamicInputsLive
live "/js", E2E.JsLive
end

scope "/issues" do
scope "/issues", Phoenix.LiveViewTest.E2E do
pipe_through(:browser)

live "/3026", Phoenix.LiveViewTest.E2E.Issue3026Live
live "/3040", Phoenix.LiveViewTest.E2E.Issue3040Live
live "/3026", Issue3026Live
live "/3040", Issue3040Live
end
end

Expand Down
90 changes: 90 additions & 0 deletions test/e2e/tests/forms.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,93 @@ test("can submit form with button that has phx-click", async ({ page }) => {

await expect(page.getByText("Form was submitted!")).toBeVisible();
});

test("can dynamically add/remove inputs (ecto sort_param/drop_param)", async ({ page }) => {
await page.goto("/form/dynamic-inputs");
await syncLV(page);

const formData = () => page.locator("form").evaluate(form => Object.fromEntries(new FormData(form).entries()));

await expect(await formData()).toEqual({
"my_form[name]": "",
"my_form[users_drop][]": ""
});

await page.locator("#my-form_name").fill("Test");
await page.getByRole("button", { name: "add more" }).click();

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users][0][name]": "",
}));

await page.locator("#my-form_users_0_name").fill("User A");
await page.getByRole("button", { name: "add more" }).click();
await page.getByRole("button", { name: "add more" }).click();

await page.locator("#my-form_users_1_name").fill("User B");
await page.locator("#my-form_users_2_name").fill("User C");

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users_drop][]": "",
"my_form[users][0][name]": "User A",
"my_form[users][1][name]": "User B",
"my_form[users][2][name]": "User C"
}));

// remove User B
await page.locator("button[name=\"my_form[users_drop][]\"][value=\"1\"]").click();

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users_drop][]": "",
"my_form[users][0][name]": "User A",
"my_form[users][1][name]": "User C"
}));
});

test("can dynamically add/remove inputs using checkboxes", async ({ page }) => {
await page.goto("/form/dynamic-inputs?checkboxes=1");
await syncLV(page);

const formData = () => page.locator("form").evaluate(form => Object.fromEntries(new FormData(form).entries()));

await expect(await formData()).toEqual({
"my_form[name]": "",
"my_form[users_drop][]": ""
});

await page.locator("#my-form_name").fill("Test");
await page.locator("label", { hasText: "add more" }).click();

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users][0][name]": "",
}));

await page.locator("#my-form_users_0_name").fill("User A");
await page.locator("label", { hasText: "add more" }).click();
await page.locator("label", { hasText: "add more" }).click();

await page.locator("#my-form_users_1_name").fill("User B");
await page.locator("#my-form_users_2_name").fill("User C");

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users_drop][]": "",
"my_form[users][0][name]": "User A",
"my_form[users][1][name]": "User B",
"my_form[users][2][name]": "User C"
}));

// remove User B
await page.locator("input[name=\"my_form[users_drop][]\"][value=\"1\"]").click();

await expect(await formData()).toEqual(expect.objectContaining({
"my_form[name]": "Test",
"my_form[users_drop][]": "",
"my_form[users][0][name]": "User A",
"my_form[users][1][name]": "User C"
}));
});
131 changes: 131 additions & 0 deletions test/support/e2e/form_dynamic_inputs_live.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
defmodule Phoenix.LiveViewTest.E2E.FormDynamicInputsLive do
use Phoenix.LiveView

alias Phoenix.LiveView.JS

@impl Phoenix.LiveView
def mount(params, _session, socket) do
{:ok,
socket
|> assign_form(%{})
|> assign(:checkboxes, params["checkboxes"] == "1")
|> assign(:submitted, false)}
end

defp assign_form(socket, params) do
form =
Map.take(params, ["name"])
|> Map.put(
"users",
build_users(
params["users"] || %{},
params["users_sort"] || [],
params["users_drop"] || []
)
)
|> to_form(as: :my_form, id: "my-form", default: [])

assign(socket, :form, form)
end

defp build_users(value, sort, drop) do
{sorted, pending} =
if is_list(sort) do
Enum.map_reduce(sort -- drop, value, &Map.pop(&2, &1, %{"name" => nil}))
else
{[], value}
end

result =
sorted ++
(pending
|> Map.drop(drop)
|> Enum.map(&key_as_int/1)
|> Enum.sort()
|> Enum.map(&elem(&1, 1)))

Enum.with_index(result)
|> Map.new(fn {item, i} -> {to_string(i), item} end)
end

defp key_as_int({key, val}) when is_binary(key) and byte_size(key) < 32 do
case Integer.parse(key) do
{key, ""} -> {key, val}
_ -> {key, val}
end
end

@impl Phoenix.LiveView
def handle_event("validate", %{"my_form" => params}, socket) do
{:noreply, assign_form(socket, params)}
end

def handle_event("save", %{"my_form" => params}, socket) do
socket
|> assign_form(params)
|> assign(:submitted, true)
|> then(&{:noreply, &1})
end

@impl Phoenix.LiveView
def render(assigns) do
~H"""
<.form
for={@form}
phx-change="validate"
phx-submit="save"
style="display: flex; flex-direction: column; gap: 4px; max-width: 500px;"
>
<input
type="text"
id={@form[:name].id}
name={@form[:name].name}
value={@form[:name].value}
placeholder="name"
/>
<.inputs_for :let={ef} field={@form[:users]} default={[]}>
<div style="padding: 4px; border: 1px solid gray;">
<input type="hidden" name="my_form[users_sort][]" value={ef.index} />
<input
type="text"
id={ef[:name].id}
name={ef[:name].name}
value={ef[:name].value}
placeholder="name"
/>
<button
:if={!@checkboxes}
type="button"
name="my_form[users_drop][]"
value={ef.index}
phx-click={JS.dispatch("change")}
>
Remove
</button>
<label :if={@checkboxes}>
<input type="checkbox" name="my_form[users_drop][]" value={ef.index} /> Remove
</label>
</div>
</.inputs_for>
<input type="hidden" name="my_form[users_drop][]" />
<button
:if={!@checkboxes}
type="button"
name="my_form[users_sort][]"
value="new"
phx-click={JS.dispatch("change")}
>
add more
</button>
<label :if={@checkboxes}>
<input type="checkbox" name="my_form[users_sort][]" /> add more
</label>
</.form>
<p :if={@submitted}>Form was submitted!</p>
"""
end
end

0 comments on commit 7d19dea

Please sign in to comment.