germsvel / phoenix_test

PhoenixTest provides a unified way of writing feature tests -- regardless of whether you're testing LiveView pages or static (non-LiveView) pages.
https://hex.pm/packages/phoenix_test
MIT License
178 stars 23 forks source link

Handle checkbox phx-click outside of form #109

Closed ftes closed 2 months ago

ftes commented 2 months ago

Fix #101

Out of scope

While this fixes this specific case, a more thorough investigation and solution might be better:

germsvel commented 2 months ago

While I was looking at implementing other inputs (like radio buttons), I realized that we have some prior art in this.

When we click a button, we have to determine if it's a phx-click or inside a form. See https://github.com/germsvel/phoenix_test/blob/57f9a488c33172a8fd7626ddf144e37488b091b0/lib/phoenix_test/live.ex#L58-L83

I like two things about that approach:

  1. I like how there's a cond statement where the final step is a clear instruction to the user. It gives a nice error of what we're expecting.
  2. It has clear path separation at the top level (in the click_button function). I made it difficult for us do that in check when I extracted common code (see 8634423) into the PhoenixTest module. So, I should probably revert that, since that the Live and Static implementations will differ now.

I think we should go that direction. I'll move the form-related code back into each driver.

That will, of course, break your changes here. I'm sorry about that. If that's too much trouble, I can pull your branch and rebase it with the new changes.


Update: I've moved the form helper implementations back into the drivers in b92e18f6168a2a77154e31b258bde8a37ca21e8d

germsvel commented 2 months ago

@ftes I'm going to close this in favor of https://github.com/germsvel/phoenix_test/pull/116. I ended up handling check and uncheck there. It lines up with how I've handled radio buttons and select dropdowns.

Thank you for opening this and for all the hard work!

ftes commented 2 months ago

116 looks better, thanks for letting me know 👍