Allow scrolling of individual elements

[joshkel]

See #9495.

This is an updated version of #9573, in hopes of moving things along.

This version uses refs, since the linked discussion seemed to prefer that interface.

[changeset-bot]

🦋 Changeset detected

Latest commit: b1bbd43

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
react-router-dom	Minor
react-router	Minor
react-router-dom-v5-compat	Minor
react-router-native	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[remix-cla-bot]

Hi @joshkel,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[david-crespo]

Nice. Would love to see this merged, but in the meantime I'll test it with patch-package.

[jrnail23]

Just some feedback from trying this on my own: maybe consider supporting elementId as well as elementRef (presumably mutually exclusive) -- I was able to get this PR's general approach working in my app using elementId, but still having trouble getting it to work with elementRef. In my case, elementRef.current is still empty when it's time to restore the scroll position, so it never gets restored.

I'm really having a hard time understanding how to ensure the scroll container's ref gets populated by the time the scroll restore happens. If ScrollRestoration is placed higher in the React tree than your scroll container, is that even possible? What am I missing here?

Uh, yeah, I managed to stumble across the ScrollRestoration example, and I see that I should've been putting <ScrollRestoration /> after my scroll container. It works now!

[brophdawg11]

@joshkel Thanks for the PR (and thanks @david-crespo for the initial PR this is based on)! I did some local testing and found/fixed one small issue if an app has different <ScrollRestoration> components per layout. There's an example of this type of layout at https://codesandbox.io/s/react-router-scroll-restoration-non-window-forked-gf2lpm?file=/src/App.tsx - it's not functional without this patch, but I was able to test it out locally.

I also added support for passing a CSS selector instead of a ref and thus changed the prop name to scrollContainer.

Let me know if this still looks good to you and I think we should be able to get this merged!

[david-crespo]

I don't know if there's simple guidance about whether to prefer ref or selector that fits in a sentence, but if there is, it would be good to add it. I feel ref is safer and easier to be confident about. But maybe people can figure that out.

[brophdawg11]

Yeah, hard to say. I sort of envisioned refs being used more when the scrollable thing is easily accessible from the component rendering <ScrollRestoration> and selectors if it's a few components away and you don't want to prop-drill a ref around. But I'm not sure if one is technically any better than another?

[ryanflorence]

Thanks for the PR!

I've wanted to allow for this for a long time but I think it needs a bit more design than what's here.

The main thing is you should be able to have multiple scroll containers at the same time, not just one (like both window as well as a sidebar etc.)

Additionally, this should be able to be done outside of react router with useLocation (I'm not sure why the implementation inside React Router is reaching into deeper state than that). Would love to see that happen first to help drive the API we finalize into react router.

[jrnail23]

@ryanflorence, I think being able to support multiple scroll containers here would mostly be matter of the shape of the savedScrollPosition object, perhaps a map of keys containing another map of scrollContainer identifiers/css selectors, etc. (of some kind) to their respective scrollPositions. Although that gets a bit trickier when you use refs rather than selectors for the containers though, since you've got to serialize and deserialize from sessionStorage (at least with this current approach).

Additionally, this should be able to be done outside of react router with useLocation (I'm not sure why the implementation inside React Router is reaching into deeper state than that).

Can you elaborate a bit more on what you're thinking here?

[david-crespo]

I'm interested in trying to write this hook from userland. Seems like the main reason it's integrated into RR deeply is so it can detect when a nav is about to happen, so it can save the scroll position. With useLocation(), the location would only change after the nav, so it would be too late. How would you catch the about-to-nav moment? Could you get there with a useEffect watching for useNavigation().state === 'loading'?

[jrnail23]

I think this might be where useBlocker comes in handy.

[ryanflorence]

Oops, I meant useNavigation

[jrnail23]

But useNavigation doesn't do anything useful unless you're doing async loaders, right?

EDIT: what I mean here is that there's not any useful info in the navigation object unless you're using async loaders, etc., right?

[david-crespo]

This seems to work. Lol. Lmao.

export function useScrollRestoration(elementRef: React.RefObject<HTMLElement>) {
  const cache = useRef(new Map<string, number>())
  const { key } = useLocation()
  const { state } = useNavigation()
  useEffect(() => {
    if (state === 'loading') {
      cache.current.set(key, elementRef.current?.scrollTop ?? 0)
    } else if (state === 'idle' && cache.current.has(key)) {
      elementRef.current?.scrollTo(0, cache.current.get(key)!)
    }
  }, [key, state, elementRef])
}

[jrnail23]

oh wow, @david-crespo , I'm gonna give that a try!
I'm guessing it's going to need some work to support scroll restore on page reload, but this could be a good starting point.

[jrnail23]

@ryanflorence, am I correct in saying that you'd be fine with (or even prefer) a solution that isn't coupled to the existing ScrollRestoration functionality?

[david-crespo]

I mean, if my thing is all it takes, I'd argue it doesn't even need to be built in. An example somewhere in the docs might make more sense. And yeah, sessionStorage instead of a map in memory probably makes sense.

[david-crespo]

Updated to use sessionStorage:

import { useEffect } from 'react'
import { useLocation, useNavigation } from 'react-router-dom'

function getScrollPosition(key: string) {
  const pos = window.sessionStorage.getItem(key)
  return pos && /^[0-9]+$/.test(pos) ? parseInt(pos, 10) : 0
}

function setScrollPosition(key: string, pos: number) {
  window.sessionStorage.setItem(key, pos.toString())
}

/**
 * Given a ref to a scrolling container element, keep track of its scroll
 * position before navigation and restore it on return (e.g., back/forward nav).
 * Note that `location.key` is used in the cache key, not `location.pathname`,
 * so the same path navigated to at different points in the history stack will
 * not share the same scroll position.
 */
export function useScrollRestoration(container: React.RefObject<HTMLElement>) {
  const key = `scroll-position-${useLocation().key}`
  const { state } = useNavigation()
  useEffect(() => {
    if (state === 'loading') {
      setScrollPosition(key, container.current?.scrollTop ?? 0)
    } else if (state === 'idle') {
      container.current?.scrollTo(0, getScrollPosition(key))
    }
  }, [key, state, container])
}

[jrnail23]

@david-crespo , correct me if I'm wrong, but if you're not using async loaders, the scroll positions would never get set, right? I don't use them yet, and navigation.state is always idle for me.

[david-crespo]

Yeah, I wasn't sure how that would work. I thought it was possible that a data router would always do async page transitions even if loaders weren't async. When you say async loaders, you mean you do have loaders but they're synchronous? Or you're not using loaders at all? In either case, you could probably fix that by adding a root loader that's just async () => Promise.resolve(null).

[jrnail23]

@david-crespo , I meant we're not using loaders at all yet.

[jrnail23]

@david-crespo, looks like a pagehide handler is still going to be needed to capture the scroll position for page reloads. But otherwise, the general approach seems to work.
One more thing, though, we'll probably want to keep the getKey and storageKey options as well. getKey preserves the ability to match on URLs, etc. instead of only location.key, and in my case, storageKey helps for customizing storage based on things like basename.

[brophdawg11]

@ryanflorence - @david-crespo is correct in that right now the reason this goes deeper than useLocation at the moment is so the router can save off current scroll positions before the navigation happens if there are no loaders to run, since in that case the navigation is synchronous and we don't go into a navigation.state==="loading" state because there's nothing to load.

The other part we currently handle through the router is preventScrollReset since that's not part of the location it's carried the start of a navigation through the end result (including redirects) and we know when to skip the reset when we land at the destination.

@jrnail23 I believe as long as you have loaders, even if they're synchronous, the navigation.state should flip to loading for one cycle.

[jrnail23]

@brophdawg11 yep, I've added one dummy loader at the root as @david-crespo suggested, and it's working as expected (navigation.state ==== 'loading'). Confirmed that it doesn't seem to matter whether it's async or not.

[david-crespo]

Definitely a stupid idea, but maybe the simplest fix is to have the router go into the loading state whether you have loaders or not — essentially building in the dummy loader. But that would mean this only works for data router.

[jrnail23]

I had the same thought, @david-crespo, so it must be a terrible idea 😛 .
But I think as currently designed, the ScrollRestoration feature is already only available for use with data routers.

[baptwaels]

Hi,

I have encountered the same problem about ScrollRestoration on a Table.
This feature would be a blast ! I wanted to check in on the status of this PR.
Thanks :)

[david-crespo]

Not much going on here but if you're using data router, I had success implementing it in userland — I actually prefer it over the built in version because it's so simple.

#10468 (comment)

[julamb]

If anyone is stumbling across a similar issue, I have been using #10468 (comment) by @david-crespo in production but had occasional bugs where the scroll restoration did not work.

Turns out in some cases container.current?.scrollTop can be a floating point value (like 240.5), this would get stored correctly in session storage but the regex in getScrollPosition would not recognize it and return 0 instead. Here is a fixed version that supports this edge-case:

import { useEffect } from 'react'
import { useLocation, useNavigation } from 'react-router-dom'

function getScrollPosition(key: string) {
  const pos = window.sessionStorage.getItem(key)
  return Number(pos) || 0;
}

function setScrollPosition(key: string, pos: number) {
  window.sessionStorage.setItem(key, pos.toString())
}

/**
 * Given a ref to a scrolling container element, keep track of its scroll
 * position before navigation and restore it on return (e.g., back/forward nav).
 * Note that `location.key` is used in the cache key, not `location.pathname`,
 * so the same path navigated to at different points in the history stack will
 * not share the same scroll position.
 */
export function useScrollRestoration(container: React.RefObject<HTMLElement>) {
  const key = `scroll-position-${useLocation().key}`
  const { state } = useNavigation()
  useEffect(() => {
    if (state === 'loading') {
      setScrollPosition(key, container.current?.scrollTop ?? 0)
    } else if (state === 'idle') {
      container.current?.scrollTo(0, getScrollPosition(key))
    }
  }, [key, state, container])
}