rsnext/test/e2e/app-dir/router-autoscroll
Zack Tanner cb432eb42d
Fix scroll bailout logic when targeting fixed/sticky elements (#53873)
### What?
When navigating to a new page with fixed or sticky positioned element as the first element, we were bailing on scroll to top behavior, which often isn't expected.

### Why?
Currently, we decide to bail on scroll to top behavior on navigation if the content that is swapped into view is visible within the viewport. Since fixed/sticky positioned elements are often intended to be relative to the current viewport, it's most likely not the case that you'd want it to be considered in this heuristic. For example, if you were scrolled far down on a page, and you navigated to a page that makes use of a sticky header, you would not be scrolled to the top of the page because that sticky header is technically visible within the viewport. 

### How?
I've updated the previous implementation that was intended to skip targeting invisible elements to also skip over fixed or sticky elements. This should help by falling back to the next level of the layout tree to determine which element to scroll to.

I've deleted the `// TODO-APP` comments as I couldn't think of a scenario in which we'd need a global scrollTop handler -- if we've bailed on every element up the tree, it's likely the page wasn't scrollable.

Some additional considerations:
- Is the warning helpful or annoying?
- Is the parallel route trade-off an acceptable one? (ie, a parallel modal slot might not be considered in the content visibility check unless if it’s fixed positioned)

Closes NEXT-1393
Fixes #47475
2023-08-15 13:31:39 +00:00
..
app Fix scroll bailout logic when targeting fixed/sticky elements (#53873) 2023-08-15 13:31:39 +00:00
next.config.js Support scroll: false for Link component for app router (#51869) 2023-07-04 10:25:25 +02:00
router-autoscroll.test.ts Fix scroll bailout logic when targeting fixed/sticky elements (#53873) 2023-08-15 13:31:39 +00:00