Support custom scroll roots with data-turbo-scroll-root - #1551
Open
jmalcic wants to merge 5 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scroll restoration in Turbo works on the premise that the window is the scroll container: the
ScrollObserveris hardcoded to read the scroll properties ofwindow, and saves and restores these properties when navigating forwards and backwards. This premise breaks down for a number of common layouts: those with fixed sidebars and scrollable main content, and layouts with sticky headers or footers and scrollable main content where use ofposition: stickywould conflict with the Popover API, where correct hiding of anchor-positioned elements like menus depends on the anchor being clipped by the scroll container, not just visually covered (currently only Chromium supports the hiding).This PR introduces a new attribute,
data-turbo-scroll-root, which allows scroll restoration to target an arbitrary scroll container within the body. The default of targeting the window remains unchanged, and the attribute is the only way to change the scroll root. Restoration works across mixed navigations, both between a window and element scroll container, and between two different element scroll containers, because the recording and restoration is relative to the same container.To support this, scroll tracking is paused at the beginning of a visit and resumed at the end (whether successful or cancelled) to prevent scrolling during navigation from clobbering the intended value for restoration. The scroll root itself is cached, but only until the next render, to prevent the cached value referring to a detached element. I’ve added tests for all the relevant cases and combinations I could think of; unfortunately there’s a known CDP limitation on native scroll restoration which means Chromium is skipped for two tests for native behaviour.
I also wanted to flag that in principle it would be possible to split this change to introduce the mechanism first (pausing, and then cached scroll root resolution) before the new functionality (actually being able to change the scroll root). But I figured it would be hard to evaluate the mechanism without being able to see the full change upfront.