Better Pages.Driver.Conn support

[AndroidOatmeal]

Hello friends, here is my much awaited PR to make static pages work more-or-less seamlessly with Pages. See below for a summary of changes.

Change summary

Pages.visit now decides which driver to use

During my testing on our code base, there was some wonkiness switching between live view and conn page drivers. Sometimes I would interact with a page and end up with a live view driver when I should have had a conn driver, and vis versa. For example, see this old case in Pages.Driver.LiveView.new/2 lib/pages/driver/live_view.ex:33. If it were passed in a :redirect tuple, that meant we should switch to a conn driver, but the existing implementation simply passed the static redirect route back to new_live/2 and ultimately failed to figure out where it was going. These cases were difficult to enumerate and hard to track down.

Instead, I took some advice from the PhoenixTest implementation and let Pages.visit/2 figure out which driver to use in a very simple manner. Simply try to get a given path, figure out if we're a live view, a conn, or a redirected conn, and build the appropriate driver. This meant that the driver's respective new/2 functions could be much simpler, since visit/2 was the one assembling the driver structs. I opted to get rid of new/2 all together and added some simple build/* functions on the respective drivers that return a necessary struct.

submit_form/* and update_form/* arity changes

The original impetus for me adding the conn driver functionality was a simple controller that accepted a single parameter:

<.form :let={f} for={%{}} action={Web.Paths.my_controller_action_path()} method="post" class="p-4">
  <div class="py-2 space-6">
    <.submit_button>Bulk create</.submit_button>
  </div>

  <.form_field columns="">
    <:label><%= label(f, :bulk_names, "Bulk names") %></:label>
    <:field>
      <%= textarea(f, :bulk_names,
        value: @bulk_names,
        rows: 100,
        autocomplete: "off"
      ) %>
    </:field>
    <:error><%= error_tag(f, : bulk_names) %></:error>
  </.form_field>
</.form>

Note that this static form had no changes or schema. The corresponding controller action clause looks like this:

def create(conn, %{"bulk_names" => bulk_names}) do
  # do something with bulk_names ...
end

The existing pages API simply wouldn't play nice with this form param structure because a schema atom is required. And unfortunately I couldn't make that argument optional because there are existing default arguments (hidden_attrs) and the compiler wouldn't allow that — it couldn't always figure out which case was correct and which parameter belonged in which slot.

My solution was to provide two flavors of submit_form:

  @spec submit_form(Pages.Driver.t(), Hq.Css.selector(), atom(), attrs_t(), attrs_t()) :: Pages.result()
  def submit_form(page, selector, schema, form_attrs, hidden_attrs) do
    params = %{schema => Map.new(form_attrs)}
    hidden_params = %{schema => Map.new(hidden_attrs)}
    submit_form(page, selector, params, hidden_params)
  end

  @spec submit_form(Pages.Driver.t(), Hq.Css.selector(), atom(), attrs_t(), keyword()) :: Pages.result()
  def submit_form(%module{} = page, selector, params \\ %{}, hidden_attrs \\ []) do
    module.submit_form(page, selector, params, Map.new(hidden_attrs))
  end

If you want to use the legacy :schema API you can, but you need to explicitly provide hidden attrs. In order to prevent duplication between the live and conn drivers, I put the :schema merging logic at the top level Pages.submit_form/5 call. This means that the driver implementations now only need to implement a single function: submit_form(page, selector, params, hidden_attrs). Something very similar was done for update_form/* as well.

Pages.HtmlForm

This was lifted from PhoenixTest and modified to use Hq. I also borrowed logic for the conn driver from this library; specifically adding an active_form field to the struct that could hold onto the state of the form in memory.

The future — unit testing Pages

The PhoenixTest library has actual unit tests for its functionality, something we're sorely missing right now. I haven't looked into the specifics, but the author seems to create real live view and static routes for the purpose of testing various scenarios. [https://github.com/germsvel/phoenix_test/blob/main/test/phoenix_test/live_test.exs](See https://github.com/germsvel/phoenix_test/blob/main/test/phoenix_test/live_test.exs)

Our entire AT suite is green using this PR, but it's certainly possible I broke something along the way when writing this.

That's it. Lemme know your thoughts!

[sax]

I think it would be nice to keep the current API Pages.new while making it forwards compatible if possible. That might make it possible to include the new functionality without forcing people to change all their existing live view tests.

[eahanson]

Hey Andrew, thanks for this PR! I'm still reading through it and thinking about it :)

[sax]

@AndroidOatmeal @cbortz I just pushed some changes to main that allow you to use Pages.update_form and Pages.submit_form with just a bunch of nested params, without the :schema argument.

I've also pushed a bunch of fixes to initializing pages and to navigating between live and dead views. Best of all, I've been adding tests to cover the latter, so if you run into any more weirdness we can try to capture them in new test cases.

I did have to make a breaking change to the Driver behaviour, but I was able to do the rest without breaking changes to the main Pages functional API. With all that said, I'd like to hold off on releasing a new version until I'm sure this works for you. Take a look and let me know if you run into any problems. I'm also happy to jump into zoom if you want to show me stack traces without posting them publicly.