scrollToRow on List with dynamic heights and CellMeasurer doesn't always scroll to the proper location
On a List with many items, all of dynamic height that are measured using a CellMeasurer, sometimes, jumping to a smaller row index n from larger row m index fails. This more frequently happens when m is much larger than n.
Here's a plunkr to reproduce this behavior: https://plnkr.co/edit/B1463DyuByuIlmWTynRn
Steps:
- Enter a number between 600-800 in the box, and hit "Jump to row..."
- Enter a number between 200-400 in the box, and hit "Jump to row...". Check whether the row is visible.
- Repeat steps 1 and 2, using a different number in the ranges each time, until you notice that the row in step 2 isn't visible after the jump.
It usually takes anywhere from 1-5 repetitions for it to first occur. I've also noticed the error occurs with decreasing frequency the more jumps are done.
Sounds like a bug, although I haven't looked at it yet. I doubt it ranks very high on our priorities list, but if you'd like to dig in and submit a PR- it would be welcome!
I have solution
/**
* Scroll to bottom of List/Grid
*/
const step = () => {
const element = // .ReactVirtualized__Grid or List element
const maxScrollTop = element.scrollHeight - element.clientHeight;
if (maxScrollTop > element.scrollTop) {
listRef.scrollToPosition(maxScrollTop); // Ref to <List /> component instance (or Grid)
requestAnimationFrame(step);
}
};
step();
You should do scroll until row index is >= startIndex and <= stopIndex scrollTop can be found by List#getOffsetForRow
I am also having this problem.
I think the issue is that scrollToRow functionality is using the value for defaultHeight in the CellMeasurerCache.
To see this in action, try going to @josephong's original Plunkr link. Try increasing the defaultHeight value in the cache to 10000, and scrollToRow will overshoot. Do the opposite and it will undershoot.
Looks like a "quick and dirty" solution is to call this.ref.measureAllRows() each time scrollToRow is called.
I would submit a PR myself, but to be quite honest it would take too long for me to understand the codebase.
I have a feeling that for someone familiar with the codebase this should be an easy fix! Would really appreciate if someone could solve it :)
I have the same issue, my workaround is to call scrollToRow again in the setState callback, like:
this.listNode.scrollToRow(idx);
this.setState({ scrollingToRow: idx }, () => {
this.listNode.scrollToRow(idx);
});
Are there any other workarounds or solutions ?
BTW @oliverodaa, calling this.ref.measureAllRows() not solving for me
I was having same problem in List solved it by using hack of setting an estimatedRowSize parameter to average row size value.
StackOverflow Answer
I was using the scrollToIndex prop to do the scrolling but due to this bug I also had to use scrollToRow() on componentDidUpdate as my workaround. The workaround is not ideal as it still has a little bounce/jump effect when the component updates.
I found calling scrollToRow twice, once as normal and once in a setTimeout 100ms later was effective. calling measureAllRows or any of the other list/grid methods did nothing to solve the problem.
Okay I have revisited this after a year to try to get a better solution. Here's what I came up with:
- ScrollToRow should be a promise that resolves only after the row is rendered.
- As @ryanwilliamquinn said, calling twice is really the only workable solution I found.
- It's not necessary to call twice if you have already measured the height of the row you are scrolling to
So we basically just try twice and then resolve the promise after the second try.
const getElementById = id => listRef.current.Grid._scrollingContainer.querySelector(`#${id}`);
let promisedRow = null;
const scrollToRow = index =>
new Promise(async resolve => {
// If the elem is already on the page, do nothing
if (getElementById(`row-${index}`)) return resolve();
// If onRowsRendered is not called enough times to resolve the promise, this resolves it
setTimeout(() => promisedRow && resolve(), 250);
// If we've seen the row before, scroll once. If we haven't, scroll twice.
// See this issue for explanation: https://github.com/bvaughn/react-virtualized/issues/995
promisedRow = { index, resolve, remainingTries: cache._rowHeightCache[`${index}-0`] ? 0 : 1 };
listRef.current.scrollToRow(index);
});
// This function is called in onRowsRendered as a kind of "callback" after the scroll event occurs
const scrollToRowCallback = () => {
if (!promisedRow) return;
const { index, resolve, remainingTries } = promisedRow;
if (remainingTries) {
promisedRow.remainingTries--;
listRef.current.scrollToRow(index);
} else {
promisedRow = null;
resolve();
}
};
<List {...otherProps} onRowsRendered={scrollToRowCallback} />
I am also having this problem. I have to use dom to get element's top and pass it to scrollToPosition dynamiclly...
Just want to add that the only solution that I have made work is the following:
this.listRef.current?.scrollToRow(this.state.messages.length - 1);
setTimeout(() => {
this.listRef.current?.scrollToRow(this.state.messages.length - 1);
}, 0);
Essentially, like others have said, you must call it twice.
Unfortunately, those trying to use the measureAllCells() method before the call will not get the correct data. The method doesn't actually measure all the cells - it will leverage defaultHeight (which means no calculating is actually being done). Because of this, you're out of luck.
Luckily, I've been messing around with the double scrollTo trick with all kinds of nonsensical defaultHeight values and it seems to consistently work.
A final comment:
You could actually get measureAllCells() to give correct truly measured data if you set defaultHeight: 0 within the cache - after which you would not need the double scrollToRow.
Of course, depending on your product, this is either not feasible, or totally feasible. Tread lightly.
A final comment:
You could actually get
measureAllCells()to give correct truly measured data if you setdefaultHeight: 0within the cache - after which you would not need the doublescrollToRow.Of course, depending on your product, this is either not feasible, or totally feasible. Tread lightly.
If you use defaultHeight: 0, your list will render all cells in one time for the first rendering, so using the library doesn't make any sense.
Yep. Hence, tread carefully. Usually, it doesn't make sense.
this works in my case
const [cb, setCb] = useState(0)
useEffect(() => { listRef.current.scrollToRow(messages.length) setCb((x) => x + 1) }, [activeChannel, messages.length])
useEffect(() => { listRef.current.scrollToRow(messages.length) }, [cb])
Okay I have revisited this after a year to try to get a better solution. Here's what I came up with:
- ScrollToRow should be a promise that resolves only after the row is rendered.
- As @ryanwilliamquinn said, calling twice is really the only workable solution I found.
- It's not necessary to call twice if you have already measured the height of the row you are scrolling to
So we basically just try twice and then resolve the promise after the second try. ...
This is a really nice solution. I just needed to call the scrollToRow function after a timeout (inside scrollToRowCallback). Like this:
scrollToRowCallback = () => {
if (!this.promisedRow) return;
const { index, resolve, remainingTries } = this.promisedRow;
if (remainingTries) {
this.promisedRow.remainingTries--;
setTimeout(() => {
this.listRef.current.scrollToRow(index);
}, 100);
} else {
this.promisedRow = null;
resolve();
}
};