Don't scroll page to top when opening or closing a side modal

[david-crespo]

Closes #3321

Because side modals get their own routes, and the scroll height cache is keyed (in part) by route, opening a side modal counts as a new page with no saved scroll height, so it scrolls the document to top. The fix is to handle the modal scenario specially by opting out of resetting scroll when we switch to a new location that has the same underlying page path as the previous one.

2026-08-18-scroll-height-fix.mp4

Example flows

Each of these scenarios is covered by e2e tests.

The underlying "page path" for a location is the path of the last non-titleOnly crumb, i.e., where the nav breadcrumbs point: a regular page's own path, or for a side modal, the path of the page under it. When a nav starts, we save the current scroll under the outgoing location's key. When it completes, if the page path changed we scroll to the new location's cached position (0 if none); if it didn't (side modal open/close), we leave the scroll alone and just record it under the new key so back/forward still works.

Normal page → page nav (…/disks → …/snapshots, then back)

Step	Location	Page path	Cache	Scroll
Scroll the disks page	…/disks (k1)	…/disks		143
Click Snapshots: nav starts	…/disks (k1)	…/disks	k1: 143	143
Nav completes: page changed → scroll to cached (none)	…/snapshots (k2)	…/snapshots	k1: 143	0
Back: nav starts	…/snapshots (k2)	…/snapshots	k1: 143, k2: 0	0
Nav completes: page changed → scroll to cached	…/disks (k1)	…/disks	k1: 143, k2: 0	143

Page → modal → page (open the New disk form, then dismiss it)

Step	Location	Page path	Cache	Scroll
Scroll the disks page	…/disks (k1)	…/disks		143
Click New disk: nav starts	…/disks (k1)	…/disks	k1: 143	143
Nav completes: same page → leave scroll, record under new key	…/disks-new (k2)	…/disks	k1: 143, k2: 143	143
Dismiss: nav starts	…/disks-new (k2)	…/disks	k1: 143, k2: 143	143
Nav completes: same page → leave scroll, record under new key	…/disks (k3)	…/disks	k1: 143, k2: 143, k3: 143	143

Modal submit → new page (create a VPC, land on VPC detail page)

Step	Location	Page path	Cache	Scroll
Modal open over scrolled VPCs page	…/vpcs-new (k2)	…/vpcs	k1: 143, k2: 143	143
Submit: nav starts	…/vpcs-new (k2)	…/vpcs	k1: 143, k2: 143	143
Nav completes: page changed → scroll to cached (none)	…/vpcs/my-vpc (k3)	…/vpcs/my-vpc	k1: 143, k2: 143	0

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
console	Ready	Preview	Aug 19, 2026 12:52am

[fakemonster]

this branch isn't tested, strictly speaking. i could delete this line and pass

[david-crespo]

Oh yeah. You have to open a model, then go to another page, and then go back to test it. I’ll add it to the tests.

[fakemonster]

i had a good deal of trouble following the code paths here. mostly to do with reconciling the conditions and the comments around them, or rather, being certain this wasn't overlooking some unnamed conditions. i think we can handle that, add some clarity to the existing cases, and prevent future spaghettification by using a custom type to put explicit names on what the conditions are observing:

/**
 * The type of navigation that occurred based on the navigation state and how
 * the new location compares to the previous one.
 */
type NavigationKind =
  /** A navigation is in flight; the old page is still on screen. */
  | 'loading'
  /** Landed on a new location, but the same page is underneath, e.g. a side
   *  modal opening/closing. */
  | 'movedToSamePage'
  /** Landed on a new location showing a different page.  */
  | 'movedToNewPage'
  /** Re-rendered at the location we're already on; nothing to do. */
  | 'noChange'

function getNavigationKind(
  state: Navigation['state'],
  key: string,
  page: string,
  prev: PrevLocation | null
): NavigationKind {
  return match(state)
    .with('loading', (): NavigationKind => 'loading')
    .with('submitting', (): NavigationKind => 'noChange')
    .with('idle', (): NavigationKind => {
      // same key means we re-rendered at the location we're already on
      if (prev?.key === key) return 'noChange'
      return prev?.page === page ? 'movedToSamePage' : 'movedToNewPage'
    })
    .exhaustive()
}

and then in place of the big if block:

    match(getNavigationKind(state, key, page, prev.current))
      .with('loading', () => setScrollPosition(key, window.scrollY))
      // leave the scroll alone and record it under the new location's key so
      // back/forward to it restores correctly
      .with('movedToSamePage', () => setScrollPosition(key, window.scrollY))
      .with('movedToNewPage', () => window.scrollTo(0, getScrollPosition(key)))
      .with('noChange', () => {})
      .exhaustive()