Scroll restoration on back nav no longer works

[david-crespo]

This is not urgent; it's a minor inconvenience. Scroll position works as expected on forward navigations.

In #1248 I patched React Router to allow us to use an element other than window as the target of scroll restoration. This worked both on new navigations (making sure any new page loads at the top rather than staying wherever you already were on the previous page) and on back button navigation, restoring the scroll position you were on before you navigated away. I confirmed now that it worked on that commit and that it stopped working the next and last time the patch was changed, in #1322.

I checked by adding a console.log right at the spot where it should be doing the restore that it does in fact have a scroll position on hand to restore as well as the scroll target element, so the problem is not a cache miss. The target element scrollTo is failing. I thought maybe it had zero height at the time of scroll (unlikely) and that was also not true: offsetHeight appears to be correct at time of scrollTo.

Changes to useScrollRestoration between RR 6.4.2 and 6.6.1 (the version we were upgrading to when it broke) can be viewed here: remix-run/react-router@react-router-dom@6.4.2...react-router-dom@6.6.1#diff-4cdcfae34c14653226bf132d981bb026891ef456aaf8d88805be82d741a83771R1120

I don't see anything there that indicates why it would have stopped working, and like I said, it appears to be trying to scroll at the appropriate time. It is probably best to wait for the official solution (which we may ourselves be contributing). See discussion in remix-run/react-router#9495 and my first attempt remix-run/react-router#9573.

[david-crespo]

I made a lot of progress while working on #1509

Why scroll restore doesn't work sometimes

When you hit back, RR has the scroll height for the page you are going back to saved, and it uses scrollTo on the scrolling element (window, but with our patch it's whatever element we specify) to scroll back to where it was before. The problem is that at the time it tries to scroll, the element we want to to scroll is not at its full height yet. This problem correlates with the presence of a React Table on the page in question — scroll restore on back nav works perfectly on the empty SSH keys page, which has no table (this is shown in the GIF below). It doesn't matter whether the table uses useQueryTable or useReactTable; it doesn't work in either case.

In the recording you can see the element's scroll height at nav time and after a setTimeout, and you can see that it increases between those two measurements in the case of the pages that don't work. In the case of the working page (empty SSH keys) it's the same number.

console.log("scrolling to", restoreScrollPosition, "height", elementRef.current.scrollHeight)
setTimeout(() => console.log("after timeout", restoreScrollPosition, "height", elementRef.current.scrollHeight))

Speculation on fixing it

In order to fix, I think this we'd have to figure out why React Table is taking longer to render (and whether it is actually re-rendering or if something funny is happening in the DOM itself). I tried upgrading RT to the latest version and that didn't do anything, which I expected because I didn't see anything potentially relevant in the release notes. I've spent enough time on this for now, though. It's good enough that we have a starting point for figuring this out next time we feel like poking around.

In the meantime, it's not a big deal and the lack of scroll restore is not very noticeable.

[david-crespo]

Oh my god, the second I posted that comment it occurred to me to look at the Table component, which we use in both the useReactTable and useQueryTable case and what do I find? Simplebar, you little shit. Commenting it out fixes the problem. So now we have a very clear place to investigate. This also explains why the problem showed up a few months ago — Simplebar was added in December 2022 in #1310. Nice!

console/libs/ui/lib/table/Table.tsx

Lines 19 to 32 in 2ae7f66

	<SimpleBar
	scrollableNodeProps={{ ref: overflowRef }}
	className={cn(
	'overflow-x-auto rounded pb-4',
	!scrollStart && 'scrolled',
	isOverflow && !scrollEnd && 'overflowing'
	)}
	autoHide={false}
	>
	<table
	className={cn(className, 'ox-table w-full border-separate text-sans-md')}
	{...props}
	/>
	</SimpleBar>

[david-crespo]

I also tried commenting out useIsOverflow and that did not fix it, and neither did commenting out some props on SimpleBar. Only removing SimpleBar or changing it to a div did the trick. So it's something Simplebar is doing. Unfortunately we're on the latest version. I tried changing the useEffect inside Simplebar to a useLayoutEffect and that didn't do anything.