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.

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

advanced and restore visits keep current scroll position with scroll: :preserve #1081

Closed
maxnovee opened this issue Nov 27, 2023 · 6 comments · Fixed by #1096
Closed

advanced and restore visits keep current scroll position with scroll: :preserve #1081

maxnovee opened this issue Nov 27, 2023 · 6 comments · Fixed by #1096

Comments

@maxnovee
Copy link

a restoration visit with back button no longer scrolls back to saved position when scroll: :preserve
not sure if that's now a bug or a feature, but breaks a previous nice user experience on mobile views with advanced links and left slides.

@maxnovee
Copy link
Author

some use case sample:
a long list of orders on the /orders page. scroll down to the bottom. click a link to /orders/101 and on the show page slide left on a mobile to get back to previous page.
scroll: :reset comes with a nice scroll to saved position turbo-7 way, while scroll: :preserve stays on the top left position.

or this should be tweaked with a custom per-page scroll setting on both /orders and /orders/101 pages?

@maxnovee maxnovee changed the title restore visit saved position gets reset with scroll=preserve restore visit saved position gets reset with scroll=preserve Nov 27, 2023
@maxnovee maxnovee changed the title restore visit saved position gets reset with scroll=preserve restore visit saved position gets reset with scroll=preserve Nov 27, 2023
@brunoprietog
Copy link
Collaborator

The new configuration should only affect page refreshes, not restoration visits, so I think it could be considered a regression

@maxnovee
Copy link
Author

probably, there is an edge case and this breaks only when you navigate from a scrollable page to a page that fits the screen and then back to scrollable, that's when the scrollToTop happens on restore visit.

but it works just fine when you move back and forth between pages that are both bigger than 100vh.

but another silly thing, when moving between scrollable pages and a next page gets loaded and scrolled straight to position you were going away from on a previous page.

@maxnovee maxnovee changed the title restore visit saved position gets reset with scroll=preserve advanced and restoration visits keep current scroll position with scroll: :preserve Nov 28, 2023
@maxnovee
Copy link
Author

The initial bug report was actually related to the autofocus effect and not relevant anymore.

The actual bug covers advanced and restoration visits between pages where the new page, opened with the current scroll position, differs from turbo-7 behavior. This issue is particularly noticeable when navigating between pages larger than 100vh. The functionality works correctly with a scroll: :reset but is broken when the :preserve option is enabled.

scroll.reset.mov
scroll.preserve.mov

@maxnovee maxnovee changed the title advanced and restoration visits keep current scroll position with scroll: :preserve advanced and restore visits keep current scroll position with scroll: :preserve Nov 28, 2023
@jorgemanrubia
Copy link
Member

@maxnovee thanks for the report. I think this is a bug indeed: the scroll: :preserve option should only affect page refreshes, but current implementation will preserve scroll in any navigation as long as the option is in place.

@jorgemanrubia
Copy link
Member

Fix here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants