-
Notifications
You must be signed in to change notification settings - Fork 894
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.
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
Add InfiniteScroll hook after patch, fixes #2667 #2674
Add InfiniteScroll hook after patch, fixes #2667 #2674
Conversation
I believe this was already address in 0.19? Can you try 0.19.1 and report back? Thanks! |
Confirm testing this with a patch somewhere the scroll continues to work properly. So I need a minimal reproduction to look into it further. |
Will try to post a minimal reproduction later, though I can confirm that it happens with 0.19.1. I added a breakpoint here and it is not called after a patch:
|
That function won't necessary be invoked since the DOM nodes may not have changed, so it's not necessarily an issue. I have pushed a change to main on a hunch of what could be causing this for you. Can you try and report back? |
I currently don't see the commit that you mention on main 🤔 |
I added an example to reproduce here: #2667 (comment) |
@chrismccord I tried your cm-maybe-fix-stream-component branch if that's what you meant, but that does not fix the issue for me. It seems like the onBeforeElUpdated function is not called with the element that contains the viewport binding in my case. See also the example to reproduce I linked earlier. |
Hello, I have another example to reproduce the problem here : https://github.com/ChristopheBelpaire/phx_viewport_bottom_bug It is a page with two tabs, on the second tab, there is an inifinite scroll |
2103295
to
67d6a4d
Compare
❤️❤️❤️🐥🔥 |
This PR fixes the InfiniteScroll hook (phx-viewport-top/bottom) not being applied after a LiveView patch navigation.
As I'm not really into the LiveView Javascript code, I'm not sure if that is the correct way to fix this, so please feel free to close this PR if it's wrong :)
Fixes #2667
Fixes #2896