phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.18k stars 930 forks source link

Removes checkbox when adding a new entry #317

Closed DaTrader closed 5 years ago

DaTrader commented 5 years ago

Environment

Actual behavior

When running LiveView Todo demo (https://dennisbeatty.com/2019/04/24/how-to-create-a-todo-list-with-phoenix-liveview.html), and submitting a new todo entry, the previous last entry gets its checkbox removed (see screenshot with the inspected element). Interestingly, the problem repeats on every second entry (the first entry is entered correctly, the second entry is added with the first entry checkbox removed, the third entry is entered correctly and it corrects the second entry - gets its checkbox back, the fourth entry entry is added the third entry checkbox getting removed, and so on).

The template code is rather simple:

<%= text_input( :todo, :title, placeholder: "What do you want to get done?") %> <%= submit( "Add", phx_disable_with: "Adding...") %>

<%= for todo <- @todos do %>

<%= checkbox( :todo, :done, phx_click: "toggle_done", phx_value: todo.id, value: todo.done) %> <%= todo.title %>

<% end %>

image

Expected behavior

A new entry should be added with the previous ones kept intact.

chrismccord commented 5 years ago

I haven't tested the example app yet, but in the meantime it would be super helpful if you could try the latest LV to see if this has been fixed as we fixed a related issue recently. Be sure to blow away the LV js and reinstall:

$ mix deps.update phoenix_live_view
$ cd assets
$ rm -rf node_modules/phoenix_live_view
$ npm install
$ cd ..
$ mix phx.server

And report back. Thanks!

DaTrader commented 5 years ago

I've just removed and reinstalled LV js as per your instructions. The bug is still there. Btw, please see the warning below that I get when installing js.

image

snewcomer commented 5 years ago

@DaTrader 👋 Cloned the repo and I'm unable to reproduce this. Checked on Chrome/Edge/Firefox. Are you seeing this only on a specific browser?

DaTrader commented 5 years ago

@snewcomer No, the browser is not the problem. It behaves the same in FF and Chrome. But, you are right. I have now cloned the repo myself into another folder and it works properly. Gotta check what went wrong with the first instance. Apologies for the false alarm.

DaTrader commented 5 years ago

@snewcomer Ok. At first I thought it was something with me switching from milligram to bootstrap, but it wasn't. Several hours later and I think I nailed it, not the bug itself, but how to repeat it. The thing is that the demo from the repo works because it uses an older version of liveview (and phoenix). I discovered that accidentally, because it wouldn't compile with the LV install instructions provided here on the github so I realized it was using an older version (thanks to its mix.lock file). The LV version the demo uses does not have live_link functions so the compilation fails, but unlike the latest version it does not have this bug that I am talking about here.

Now, try it yourself and you'll be able to repeat the bug:

DaTrader commented 5 years ago

I am reopening the issue because of the above.

snewcomer commented 5 years ago

@DaTrader Thanks a ton for re-opening. I did a bisect and found the offending commit - d934d0e9f11a55000e4b4136f8812cbb8c877314/8a3db391c239bc6f951043e91f0ecd417c917219.

I will have to look into why. It is a merge commit.

snewcomer commented 5 years ago

Confirmed this is not an issue with morphdom (current or past versions 2.5.2-2.5.5). Will keep digging.

snewcomer commented 5 years ago

@DaTrader I found the issue. Effectively your input checkbox in a loop needs to look like this or else you will have invalid HTML - duplicate ids is problem:

<%= checkbox(:todo, :done, phx_click: "toggle_done", id: todo.id, phx_value: todo.id, value: todo.done) %>

Screen Shot 2019-08-31 at 10 40 43 AM

vs.

Screen Shot 2019-08-31 at 10 42 58 AM

Really you can construct the input[id] any way you want. Just as long as they are not duplicated on the page! Lmk if you have any other issues and thanks for bringing this up!

yeongsheng-tan commented 5 years ago

@DaTrader I found the issue. Effectively your input checkbox in a loop needs to look like this or else you will have invalid HTML - duplicate ids is problem:

<%= checkbox(:todo, :done, phx_click: "toggle_done", id: todo.id, phx_value: todo.id, value: todo.done) %>

Screen Shot 2019-08-31 at 10 40 43 AM

vs.

Screen Shot 2019-08-31 at 10 42 58 AM

Really you can construct the input[id] any way you want. Just as long as they are not duplicated on the page! Lmk if you have any other issues and thanks for bringing this up!

Worked like a charm!

DaTrader commented 5 years ago

@snewcomer Yes, it works! Thanks a lot for the tip and for the time spent on finding this bug.