-
Notifications
You must be signed in to change notification settings - Fork 621
Fix Scrolling Issues #420
base: master
Are you sure you want to change the base?
Fix Scrolling Issues #420
Conversation
Manually handle scroll restoration.
e5b3170
to
646687c
Compare
Love this. Please merge 👍 |
Can someone merge this? Really need. |
@panoply feel free to checkout and build https://github.com/domchristie/turbolinks/tree/scroll_on_load and report any feedback. Thanks! |
Restore the position property on ScrollManager.
This might prove to be useful in more situations as we could determine how a page was loaded (initial page load, manual reload, page invalidated reload, or turbolinks load), and then annotate load events accordingly. |
@domchristie No problems my end Safari / Chrome / Firefox |
Ran into this issue today, would love this to get merged. |
1d8eaf1
to
25352dc
Compare
According to the spec, the |
It would be really nice to get this merged! |
Running into this issue as well, a merge would be awesome 🙏 |
Running into this issue here. 🤞 🙏 |
also running into this...please merge! 👍 |
Running to this issue too! Please have it merged! |
this seems to still be alive, right? |
it doesn't look like this is going to be merged in any time soon. is there a way to use a fork of this to solve the problem? since this is just the javascript and is loaded by a rubygem, i honestly have no idea how to do this. looks like turbolinks itself has just been declared as superceded by turbo, in fact. would appreciate very much if anyone can help me get this fixed on a [rails 5] application using forks. crossing fingers and tia. |
Can you please finish checking. It would great to have it merged! Thank you. |
This is an attempt at fixing #181 and #241.
When a page is invalidated, either by tracked asset changes or via the
turbolinks-visit-control
directive, it is reloaded but the user is returned to the previous scroll position :/ This can be jarring when moving between long pages with different layouts—someone will click a link and be dumped some way down the page when it loads. This is discussed in #181, and I have outlined a couple of solutions in #181 (comment). This pull request aims to implement the "improved solution" in a more structured way.First, we set the
scrollRestoration
flag tomanual
to indicate to the browser that we'll be handling scroll positions when navigating via the history. This seemingly impacts the scroll position on reload, too. It's great for this bug as it means that if a page is invalidated and reloaded, we do nothing. However, we also don't want to break the refresh button :/ So to fix this we store the scroll position in session storage, and restore it on reload.Configuring
scrollRestoration
has the nice side-effect of fixing scrolling flicker when navigation Back/Forward #241. The downside is that for a complete solution we need to persist scroll values across full page loads (onbeforeunload
), through session storage, which I'm not 100% convinced about.App which demonstrates the unexpected/buggy behaviour: https://secret-bayou-73715.herokuapp.com
App which demonstrates this fix: http://sheltered-reef-10818.herokuapp.com
Fixes #181
Fixes #241
scrollRestoration
is not supported in IE/Edge, so I would welcome some testers in those browsers in particular.Also, I should add some automated tests for this, but I'm having some difficulty in setting up the recent test framework. (I think Intern starts the server then immediately stops with no error.)Thanks to @jason0x43 for his help on this. My old version of Java was not compatible with Intern.