Commit 3ab13e2e authored by Benjamin Bouvier's avatar Benjamin Bouvier
Browse files

Only scroll at the top of the infinite list on mount

There was an implicit dependency on the scroll handler that wasn't
correct, since it would trigger scroll-to-top at unexpected times.
Splitting the effect into two solves the issue at no cost.
parent 95480b5a
Pipeline #415309 passed with stage
in 5 minutes and 59 seconds
......@@ -73,23 +73,29 @@ const InfiniteList = (props: Props) => {
useEffect(() => {
// The items might change under the hood, after search has been
// cleared out, etc.
// cleared out, etc. Remember that `recomputeWindow` is cheap to call
// in general.
recomputeWindow();
}, [recomputeWindow]);
// On mount only. Make sure to not include dependencies on handlers that
// could be modified at any time.
useEffect(() => {
// On mount.
const target = document.getElementById(containerId);
assert(!!target, 'container must have been mounted');
container.current = target;
container.current.addEventListener('scroll', handleScroll);
// Ensure the top of the list is always visible when remounting the component.
container.current.scrollTop = 0;
target.scrollTop = 0;
}, [containerId]);
// Reset the scroll handler every time it changes. In theory it should
// remain constant, and the scrolled data should be what changes, but React
// hooks don't make this easy to model.
useEffect(() => {
const target = document.getElementById(containerId);
assert(!!target, 'container must have been mounted');
container.current = target;
container.current.addEventListener('scroll', handleScroll);
return () => {
// On unmount.
assert(!!container.current, 'container must have been mounted');
container.current.removeEventListener('scroll', handleScroll);
};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment