Do not prevent submitting form when phx-click is set (#3066)

* Do not prevent submitting form when phx-click is set

To fix #3056
fbe2226
introduced a check that prevents submit buttons from submitting the form
when phx-click is set. This change is not necessary, because for buttons
that should not submit the form, the `type="button"` attribute should be
used instead.

Adjusts the documentation to reflect this.

* add test

assets/js/phoenix_live_view/live_socket.js

@@ -649,7 +649,7 @@ export default class LiveSocket {
         return
       }
 
-      if(target.getAttribute("href") === "#" || target.form){ e.preventDefault() }
+      if(target.getAttribute("href") === "#"){ e.preventDefault() }
 
       // noop if we are in the middle of awaiting an ack for this el already
       if(target.hasAttribute(PHX_REF)){ return }

lib/phoenix_component.ex

@@ -2344,7 +2344,7 @@ defmodule Phoenix.Component do
 
   <input type="hidden" name="mailing_list[emails_drop][]" />
 
-  <button name="mailing_list[emails_sort][]" value="new" phx-click={JS.dispatch("change")}>
+  <button type="button" name="mailing_list[emails_sort][]" value="new" phx-click={JS.dispatch("change")}>
     add more
   </label>
   ```
@@ -2367,10 +2367,12 @@ defmodule Phoenix.Component do
   dropped all entries. This hidden input is required whenever dropping associations.
 
   Finally, we also render another button with the sort param name `mailing_list[emails_sort][]`
-  and `value="new"` name with accompanied "add more" text. Ecto will treat unknown sort params
-  as new children and build a new child. This button is optional and only necessary
-  if you want to dyamically add entries. You can optionally add a similar button
-  before the `<.inputs_for>`, in the case you want to prepend entries.
+  and `value="new"` name with accompanied "add more" text. Please note that this button must
+  have `type="button"` to prevent it from submitting the form.
+  Ecto will treat unknown sort params as new children and build a new child.
+  This button is optional and only necessary if you want to dyamically add entries.
+  You can optionally add a similar button before the `<.inputs_for>`, in the case you want
+  to prepend entries.
   """
   @doc type: :component
   attr.(:field, Phoenix.HTML.FormField,

test/e2e/tests/forms.spec.js

@@ -11,7 +11,7 @@ test.describe("restores disabled and readonly states", () => {
     let changesA = attributeMutations(page, "input[name=a]");
     let changesB = attributeMutations(page, "input[name=b]");
     // can submit multiple times and readonly input stays readonly
-    await page.locator("button[type=submit]").click();
+    await page.locator("#submit").click();
     await syncLV(page);
     // a is readonly and should stay readonly
     await expect(await changesA()).toEqual(expect.arrayContaining([
@@ -28,16 +28,16 @@ test.describe("restores disabled and readonly states", () => {
       { attr: "readonly", oldValue: "", newValue: null },
     ]));
     await expect(page.locator("input[name=a]")).toHaveAttribute("readonly");
-    await page.locator("button[type=submit]").click();
+    await page.locator("#submit").click();
     await syncLV(page);
     await expect(page.locator("input[name=a]")).toHaveAttribute("readonly");
   });
 
   test("button disabled state is restored after submits", async ({ page }) => {
     await page.goto("/form");
     await syncLV(page);
-    let changes = attributeMutations(page, "button[type=submit]");
-    await page.locator("button[type=submit]").click();
+    let changes = attributeMutations(page, "#submit");
+    await page.locator("#submit").click();
     await syncLV(page);
     // submit button is disabled while submitting, but then restored
     await expect(await changes()).toEqual(expect.arrayContaining([
@@ -171,3 +171,15 @@ test.describe("form recovery", () => {
     ]))
   });
 })
+
+test("can submit form with button that has phx-click", async ({ page }) => {
+  await page.goto("/form?phx-auto-recover=custom-recovery");
+  await syncLV(page);
+
+  await expect(page.getByText("Form was submitted!")).not.toBeVisible();
+
+  await page.getByRole("button", { name: "Submit with JS" }).click();
+  await syncLV(page);
+
+  await expect(page.getByText("Form was submitted!")).toBeVisible();
+});

test/support/e2e/form_live.ex

@@ -1,15 +1,19 @@
 defmodule Phoenix.LiveViewTest.E2E.FormLive do
   use Phoenix.LiveView
 
+  alias Phoenix.LiveView.JS
+
   @impl Phoenix.LiveView
   def mount(_params, _session, socket) do
     {:ok,
-     assign(socket, :params, %{
+     socket
+     |> assign(:params, %{
        "a" => "foo",
        "b" => "bar",
        "id" => "test-form",
        "phx-change" => "validate"
-     })}
+     })
+     |> assign(:submitted, false)}
   end
 
   @impl Phoenix.LiveView
@@ -35,7 +39,7 @@ defmodule Phoenix.LiveViewTest.E2E.FormLive do
   end
 
   def handle_event("save", _params, socket) do
-    {:noreply, socket}
+    {:noreply, assign(socket, :submitted, true)}
   end
 
   def handle_event("custom-recovery", _params, socket) do
@@ -62,11 +66,16 @@ defmodule Phoenix.LiveViewTest.E2E.FormLive do
     >
       <input type="text" name="a" readonly value={@params["a"]} />
       <input type="text" name="b" value={@params["b"]} />
-      <button type="submit" phx-disable-with="Submitting">Submit</button>
+      <button type="submit" phx-disable-with="Submitting" phx-click={JS.dispatch("test")}>
+        Submit with JS
+      </button>
+      <button id="submit" type="submit" phx-disable-with="Submitting">Submit</button>
       <button type="button" phx-click="button-test" phx-disable-with="Loading">
         Non-form Button
       </button>
     </form>
+
+    <p :if={@submitted}>Form was submitted!</p>
     """
   end
 end