Phoenix_live_view: LV doesn't listen to events generated by form input fields

[azazel75]

Hi I have this issue where the form fields that are used to input filter data are ignored by the LiveView and I don't know why. I'm not certain that the issue here is a bug in the LV code, it could be an error in the app's code, but it seems to me that there's no websocket traffic when the issue manifests.

Please see the linked test application.

Environment

• Elixir version (elixir -v): Elixir 1.10.3 (compiled with Erlang/OTP 22)
• Phoenix version (mix deps): 1.5.1
• Phoenix LiveView version (mix deps): 0.12.1
• NodeJS version (node -v): 13.13.0
• NPM version (npm -v): 6.14.4
• Operating system: NixOS GNU/Linux unstable
• Browsers you attempted to reproduce this bug on (the more the merrier): Firefox 76.0, Chromium 81.0.4044.138
• Does the problem persist after removing "assets/node_modules" and trying again? Yes

Actual behavior

I've produced a sample app that shows the error, you can find it here: https://github.com/azazel75/test_live_filter

Steps to reproduce the erroneous behavior:

1. with the root of the site opened, click on the LISTING button at
   the beginning of the page. The Phoenix's demo root page should
   disappear and a table listing should appear. Also note the URL that
   has been patched;

2. note the field length above the table, should be 40

3. insert Mono in the input field under the Name column. Note the
   length field again: nothing happened, that's the bug. also note
   the URL that now should be /listing/accommodation;

Expected behavior

That the table gets filtered. To see the expected behavior, with the same page still open, do the following:

1. do a reload of the page where you are, i.e. simulating a LV's
   redirect;

2. repeat points 3 and 4, now length should mark 4 and the URL
   should be /listing/accommodation?name=Mono.

Changing the URL generated for the buttons into a redirect fixes the issue, but also resets the LV/browser/page state, and that's what I want to avoid.

[nshoes]

I believe the issue is conditionally hiding/showing your table in when viewing listing/accommodation. Note that in page_live.html.leex the generated deps search functionality is rendered but conditionally shown, but your table is conditionally rendered. This minor difference is important because DOM elements need to be in the DOM for phoenix_live_view's JS to attach handlers that power attributes like phx-change and friends to work.

In page.live.html.leex, if you change:

<%= if @list_name != nil do %>

To:

<div class="<%= if @list_name != nil, do: "visible", else: "hidden" %>">

And update your second head of handle_params/3 to:

  def handle_params(params, _url, socket) do
    {
      :noreply,
      assign(socket,
        list_name: nil,
        data: [],
        fields_spec: Test.get_fields_spec(),
        params:
          Enum.map(
            Map.drop(params, ["list_name"]),
            fn {k, v} -> {String.to_atom(k), v} end
          )
      )
    }
  end

Everything will work :) Note that the code above is not what you should move forward with, it's just an example. Consider splitting out PageLive into separate LiveViews.

[azazel75]

I believe the issue is conditionally hiding/showing your table in when viewing listing/accommodation.

I bet it is as well, but it's not something forbidden, isn't it?

This minor difference is important because DOM elements need to be in the DOM for phoenix_live_view's JS to attach handlers that power attributes like phx-change and friends to work.

The phx-change and the form in general is rendered at the same time the input fields are rendered, there's no way around it.

The "solution" you propose works by changing the way the test view works, in a way that I would like to avoid.

The first part of the page, the one in the CSS hiding switch <div class="<%= if @list_name == nil, do: "visible", else: "hidden" %>"> </div> in real world contains a form of the record under edit and the other panels contain views of the to many relations of the same record.

The listing isn't rendered in the root because it cannot be rendered without knowing which relation's listing to show, and at the same time I would like to avoid loading all the relations records in advance. So your code that changes the second handle_params to do a fields_spec: Test.get_fields_spec(), is somewhat cheating.. I would have to pre-render the empty table for every relation in advance

[azazel75]

Note that the code above is not what you should move forward with, it's just an example.

why?

Consider splitting out PageLive into separate LiveViews.

In the real code the listing is in another component, but I don't see why so few lines of code/HTML cannot be on the same view. Splitting the page in multiple LV would mean to reset the page state each time one of the button at the top is clicked, effectively deleting any state of what's on the firs "panel" (the only one that's not conditionally rendered but instead is hidden by using CSS)

[nshoes]

I bet it is as well, but it's not something forbidden, isn't it?

Because of how JS handlers work (the element needs to be present when JS handlers are attaching), it is :)

The "solution" you propose works by changing the way the test view works, in a way that I would like to avoid.

So your code that changes the second handle_params to do a fields_spec: Test.get_fields_spec(), is somewhat cheating

Agreed! It was just the minimal amount of changed code necessary to prove what was wrong. Also note that I said "the code above is not what you should move forward with, it's just an example".

why?

I think you answered that yourself! It changed the code in a way that's not very clear or maintainable. Again, it was just a demonstration to highlight the core issue.

Splitting the page in multiple LV would mean to _reset_ the page state each time one of the button at the top is clicked, effectively deleting any state of what's on the firs "panel" (the only one that's not conditionally rendered but instead is hidden by using CSS)

Sure, but you're swimming against the tide now. Elixir processes, and thus their state, are cheap. The solution here is to create two LiveVews. Reap the benefits of isolated processes and state.

[dbernheisel]

I encountered this myself when conditionally rendering (vs hiding). Maybe worth a note in the guides to help folks avoid this pitfall? I'll be happy to submit a PR.

[azazel75]

I bet it is as well, but it's not something forbidden, isn't it?

Because of how JS handlers work (the element needs to be present when JS handlers are attaching), it is :)

I know that, but here the things aren't so simple. What my application seems to prove is that if the form exists when the LV is first mounted then it's events are hooked up, If it isn't they don't. And this seems a bug to me. I've not debugged the JS code because I'm not familiar with it and maybe someone that works on it everyday may locate the issue more quickly, anyway there are some scenario about how this can happen that come to mind:

• for some (undocumented?) reason the LV JS code cannot handle this situation. It does the a correct merge of the HTML code under the if so why isn't hooking up the events?

• the code represents some kind of corner cases that is unexpected.... maybe the hook are setup before the input elements are setup before the input are added to the DOM as you say? But the events are listened on the form, that will contain the elements... I don't know yet.

BTW, are you stating that you looked at the LV JS code and determined that the actual one is the expected behavior ?

Sure, but you're swimming against the tide now. Elixir processes, and thus their state, are cheap. The solution here is to create two LiveVews. Reap the benefits of isolated processes and state.

You say that I'm swimming against the tide, but really how do would solve the problem to keep the data of a (maybe) half filled form when switching between the panels if they where different LVs?

[nshoes]

I know that, but here the things aren't so simple. What my application seems to prove is that if the form exists when the LV is first mounted then it's events are hooked up, If it isn't they don't.

But they are! Your code proves that if the DOM nodes aren't available on the first calls to mount/3, events won't be hooked up. We are starting to go in circles here :) It may be possible to re-attach handlers in a hook but I'm not sure and would have to experiment. I'll have some tonight to try it out if you don't reach a resolution before then.

BTW, are you stating that you looked at the LV JS code and determined that the actual one is the expected behavior ?

Nope, but I have a lot of JS experience and I know how event handlers work. I'd be surprised if LiveViews JS did something really out of the ordinary.

You say that I'm swimming against the tide, but really how do would solve the problem to keep the data of a (maybe) half filled form when switching between the panels if they where different LVs?

LiveViews are their own process. No state is shared between Elixir processes by design. If your application requires some state to live between the normal LiveView lifecycle process, then you'll have to reach for something like ETS or a GenServer.

[nshoes]

@dbernheisel that would be a nice addition to the docs, I'm sure more people have run into this.

[chrismccord]

Let me take a look and will report back. Our js event listeners sit on window, so we will catch any conditionally rendered DOM click/submit/etc so that should not be the issue in this case.

[chrismccord]

The issue is the form tag inside a tr wrapping th's is invalid HTML. If you inspect the HTML, you can see the browser is rewriting it as follows:

<tr class="search">
  <form phx-change="list_filter"></form>
  <th></th>
  <th>
    <div class="input-control">
      <input id="search_name" name="search[name]" phx-debounce="300" type="text" value="Mono">               
    </div>
  </th>
  <th>
    <div class="input-control">
      <input id="search_oid" name="search[oid]" phx-debounce="300" type="text" value="">
    </div>
  </th>
</tr>

If you change it to something like the following, you'll see it works:

      <tr class="search">
        <form phx-change="list_filter">
        <%= for {fname, fdata} <- @fields_spec do %>
          <%= if fdata[:visible] do %>
            <%= if fdata[:type] == :string do %>
                  <% curr_value = Keyword.get(@params, fname, "") %>
                  <%= text_input :search, fname,
                  phx_debounce: "300",
                  value: curr_value %>
                </div>
            <% else %>
            <% end %>
          <% end %>
        <% end %>
      </form>
    </tr>

We weren't picking up the events because the inputs are not inside the form :) This is a browser engine thing and nothing we can do so the solution is to ensure you have valid HTML. Thanks!

[azazel75]

@chrismccord Thanks for answering, but I don't undestand... This is my original code, what does it misses?

      <tr class="search"><form phx-change="list_filter">
        <%= for {fname, fdata} <- @fields_spec do %>
          <%= if fdata[:visible] do %>
            <%= if fdata[:type] == :string do %>
              <th>
                <div class="input-control">
                  <% curr_value = Keyword.get(@params, fname, "") %>
                  <%= text_input :search, fname,
                  phx_debounce: "300",
                  value: curr_value %>
                </div>
              </th>
            <% else %>
              <th></th>
            <% end %>
          <% end %>
        <% end %>
      </form></tr>

Have they to be direct children? Seems a bit extreme to me

[azazel75]

Also doesn't explain why if the HTML code isn't but inside an IF it works as expected

[josevalim]

You cannot have a form inside a tr. That's invalid HTML. Check what the browser is showing once you inspect the HTML there.

[azazel75]

Thanks @josevalim for better explaining the problem, of course you are right. In light of this problem I changed the structure of the code with the goal of keeping the input elements inside the columns, and the form out of the table structure.
The most obvious solution is to wrap the table with the form element, like:

<form phx-change="list_filter">
  <table>
    <thead>
      <tr>
        <!-- column labels -->
      </tr>
      <tr class="search">
        <%= for {fname, fdata} <- @fields_spec do %>
          <%= if fdata[:visible] do %>
            <%= if fdata[:type] == :string do %>
        <th>
          <div class="input-control">
              <% curr_value = Keyword.get(@params, fname, "") %>
              <%= text_input :search, fname,
              phx_debounce: "300",
              value: curr_value,
              %>
          </div>
        </th>
            <% else %>
        <th></th>
            <% end %>
          <% end %>
        <% end %>
      </tr>
    </thead>
  </table>
</form>

The other way is not to wrap the table and instead use the form attribute on the input fields, like:

<form id="list_filter" phx-change="list_filter"></form>
<table>
  <thead>
    <tr>
      <!-- column labels -->
    </tr>
    <tr class="search">
      <%= for {fname, fdata} <- @fields_spec do %>
        <%= if fdata[:visible] do %>
          <%= if fdata[:type] == :string do %>
      <th>
        <div class="input-control">
            <% curr_value = Keyword.get(@params, fname, "") %>
            <%= text_input :search, fname,
                phx_debounce: "300",
                value: curr_value,
                form: "list_filter" %>
        </div>
      </th>
          <% else %>
      <th></th>
          <% end %>
        <% end %>
      <% end %>
    </tr>
  </thead>
</table>

This second probably could allow fields of different forms to be one near the other.
Both works flawlessly. I'll use the second
@dbernheisel probably the most important thing to document here is the problem that in some situation an bugged form like the one that caused me to create this bug report could partly work

Thank you all