-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix(nuxt): improve default router scrollBehaviour
#24960
base: main
Are you sure you want to change the base?
fix(nuxt): improve default router scrollBehaviour
#24960
Conversation
Run & review this pull request in StackBlitz Codeflow. |
4953a3c
to
797aa2d
Compare
fyi, fixed the typescript errors and force pushed my branch to avoid commit clutter |
// Without this setTimeout, the scroll behaviour is not applied, this is however working reliably | ||
// todo: figure out how we can solve this without setTimeout of 0ms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe using nextTick
twice here? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that, unfortunately it doesn't help. I even tried chaining multiple nextTicks() here and it didn't help. I think it has to do with what's happening when using setTimeout: https://medium.com/@sohnu/settimeout-with-time-0-what-does-it-really-mean-3b306880a0f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have added the quote I'm referring to, sry:
But what we have to see here is, all the 3 setTimeouts are taken away from the actual call executions and when there is nothing in the actual call stack, the setTimeouts started to execute with the time specified.
So whenever we want to take the call outside of the flow and run it once after the call stack is free, we can use setTimeout with time 0.
tbh, I have absolutely no clue why this is fixing it. I'm happy for any theory as to what's going on here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran into an issue that had the same sort of resolution. I don't really understand what it is about nextTick
that's not working in every case, but I opened a PR that might be relevant to this discussion: #25817
dc9c381
to
2f1d8b6
Compare
handle first page load and page refresh scenarios (restore scroll position without jumps), fixes hash-navigation when changing pages and browser back/forward positions, especiall when page transitions are involved. fixes nuxt#24941, nuxt#22487, nuxt#25030 and nuxt#19664 Signed-off-by: Bernhard Berger <bernhard.berger@gmail.com>
2f1d8b6
to
a3658ee
Compare
scrollBehaviour
/trigger release |
🚀 Release triggered! You can now install nuxt@npm:nuxt-nightly@pr-24960 |
Would you be able to update the PR following #25483? 🙏 And thank you so much for this ❤️ |
I'm a bit limited on time this week. Hopefully I can get to it next week. |
Marking as draft just for triage - feel free to unmark whenever it's ready from your end 🙏 |
Turns out I've been drowned in work and family stuff ever since then. I have not abandoned it, it's still on my mind, however I just have zero spare time at the moment. |
handle first page load and page refresh scenarios (restore scroll position without jumps), fixes hash-navigation when changing pages and browser back/forward positions, especiall when page transitions are involved.
🔗 Linked issue
fixes #24941
fixes #22487
fixes #19664
fixes #25030
❓ Type of change
📚 Description
handle first page load and page refresh scenarios (restore scroll position without jumps), fixes hash-navigation when changing pages and browser back/forward positions, especially when page transitions are involved.
📝 Checklist