Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Client event selection_recovery fails when wrapped in input_for #96

Open
atcherry opened this issue Jan 13, 2025 · 3 comments · May be fixed by #97
Open

Client event selection_recovery fails when wrapped in input_for #96

atcherry opened this issue Jan 13, 2025 · 3 comments · May be fixed by #97
Labels
bug Something isn't working

Comments

@atcherry
Copy link

atcherry commented Jan 13, 2025 •
edited
Loading

LiveSelect and LiveView versions
have you tried LiveSelect's and LiveView's latest versions? Please try them and see if the bug still occur
which LiveSelect version are you on? 1.5.2
which LiveView version are you on? 1.0.1

I'm happy to create a PR for this.

Describe the bug
The selection_recovery event gets submitted to a parent LiveView instead of the LiveSelect LiveComponent for live selects wrapped in LiveView's inputs_for component.

Expected behavior
The LiveSelect hook's reconnected callback pushes the selection_recover event to the live select LiveComponent.

Actual behavior
The LiveSelect hook's reconnected callback pushes the selection_recovery event to the parent live view.

Screenshots
If applicable (in most cases it is), do add a screenshot (or even better, a GIF or a video) that describes the problem.

Browsers
Firefox

Issue Repo
The following repo demonstrates the issue.
https://github.com/atcherry/live_select_example

Additional context
When the reconnected callback for the LiveSelect hook gets called, somehow the element that is being referred to in the pushEventTo call lacks DOM information on where the live component is. This can be traced to the owner call in the Phoenix JS code for live_socket which makes a call to el.closest(...). el.closest(...) returns null.

I believe a simple but effective fix would be to call pushEventTo(this.el.id, ...) instead of pushEventTo(this.el, ...). I think I tested this and it worked, but I can make sure that is the case.
@maxmarcon maxmarcon added the bug Something isn't working label Jan 13, 2025
@maxmarcon
Copy link
Owner

Hi @atcherry! Thanks for reporting this and taking the time to dig into it.

Would you be able to produce a MR for this? Your proposed fix sounds good to me

@atcherry
Copy link
Author

Can do! I'm looking into more about what the exact scenario is that causes this. I tried reproducing this in a new project with no success with an unchanging number of inputs. That leads me to think that perhaps it is a bit more nuanced than just live_select being wrapped in inputs_for. Perhaps the issue is an inputs_for that renders a dynamic number of inputs. Regardless, I think the solution is still the same.

@thiagopromano
Copy link

thiagopromano commented Jan 16, 2025 •
edited
Loading

We encountered the same issue, causing a crash loop on the LiveView process. I've tested replacing pushEventTo(this.el, ...)
with pushEventTo(this.el, ...) seems to fix it in our scenario too.

@atcherry atcherry linked a pull request Jan 17, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Push selection_recovery to element id atcherry/live_select
3 participants
@thiagopromano @maxmarcon @atcherry