germsvel / phoenix_test

MIT License
144 stars 20 forks source link

click_button to submit a form does not include the value in the form data #51

Closed rgraff closed 5 months ago

rgraff commented 5 months ago

Clicking a button like below to submit a form will not submit form with field=bar.

<form id="my-form" phx-submit="save">
  <button type="submit" name="field" value="bar">
    <img src="bar.jpg"/>Bar
  </button>
  <button type="submit" name="field" value="foo">
    <img src="foo.jpg"/>Foo
  </button>
</form>

Additionally, since the form only has buttons, you have to call fill_form with an empty hash before submitting the form.

      conn
      |> visit("/my-page")
      |> fill_form("#my-form", %{}) # This line should not be required but currently is to get any button to submit
      |> click_button("Foo")

assert that the we received handled_event("save", %{"field" =>. "bar"}, socket)

germsvel commented 5 months ago

Thanks for opening this issue @rgraff!

You're right on both counts. We need to submit the name and value for the button. And in LiveViews, we're not handling a form with just submits. In the static side of things, we are. But we need to bring parity there.

If you're up for taking a stab at either of those improvements, I'd be happy to review a PR. Otherwise, I'll get to it as soon as I can.

germsvel commented 5 months ago

@rgraff a quick question.

I was starting work on adding the name and value to the form submission. But I tested LiveView's actual behavior (which we're trying to emulate), and it doesn't send the name and value from the button on phx-change or phx-submit.

I think it should since that's part of the HTML button's spec. But I hesitate to introduce this functionality since it would create false positive tests (i.e. our test would pass but if you're expecting that value in your LiveViews in production, you would never get them).

Can you check on your side if you're LiveView's handle_event actually receives the name and value you add to a button? If not, it might be an issue to open up with LiveView itself.

germsvel commented 5 months ago

@rgraff this should now work in main. The commit that introduced the functionality is 8d16b7e, but I had to change quite a few things to make things work like I wanted. So this should be out with the next release! 🎉

rgraff commented 3 months ago

@germsvel this works great. Love this project. Thank you!