Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling scrollToRow() on List no longer works after updating React to v16.4.1 #1179

Closed
michaeldzjap opened this issue Jul 25, 2018 · 20 comments · Fixed by #1288
Closed

Calling scrollToRow() on List no longer works after updating React to v16.4.1 #1179

michaeldzjap opened this issue Jul 25, 2018 · 20 comments · Fixed by #1288

Comments

@michaeldzjap
Copy link

michaeldzjap commented Jul 25, 2018

I have a fairly complicated app where this issue is occurring, so it is not so easy to include a simple example showing the problem. Essentially what I am doing is calling

this._pdfView.getWrappedInstance().list.scrollToRow(index);

where this._pdfView is a ref to a connected component (hence the getWrappedInstance() call), which in turn has a getter to access the ref to a List component. I use a combination of WindowScroller, AutoSizer and List. Important to note is that this is an issue specifically with React v16.4.1. If I revert to React v16.3.0 everything works like expected.

If this information is not sufficient, I will try to attach a minimal example that reproduces the problem. I understand that otherwise it will be difficult for you to track down the problem.

What is the current behavior?

Calling scrollToRow() no longer scrolls to the right row. In fact, it doesn't seem to be doing anything anymore.

What is the expected behavior?

It should scroll to the row given by the index.

Which versions of React and react-virtualized, and which browser / OS are affected by this issue? Did this work in previous versions of react-virtualized?

Browser Chrome v67
OS OSX 10.13.6
React 16.4.1
React DOM 16.4.1
react-virtualized 9.20.1
@john-veresk
Copy link
Contributor

john-veresk commented Jul 26, 2018

Have same problem too, but with scrollToCell in Grid. It is working with 16.3, but breaks on >=16.4.

I'm using MultiGrid wrapped in AutoSizer and call scrollToCell like this:
this._grid._bottomRightGrid.scrollToCell({columnIndex, rowIndex})

I've spent some time in debugger and noticed, that setState on this line is not updating Grid's state (scrollTop/scrollLeft fields):
https://github.com/bvaughn/react-virtualized/blob/master/source/Grid/Grid.js#L1528

It seems setState returns previous cache of the state and not updating it.

@john-veresk
Copy link
Contributor

john-veresk commented Jul 26, 2018

How to reproduce

I'm finely reproduced this issue. Only with MultiGrid, but the problem is the same. If you use combination of WindowScroller, AutoSizer and List/Grid - you should get the same result.

Working example. React 16.3: https://codesandbox.io/s/llq660v04l
Broken after updating to React 16.4.1: https://codesandbox.io/s/1v4r4nqpz4

Change values in input with label Scroll to column or Scroll to row. In the first example it will scroll to specified column, but nothing will happen if we update React to 16.4.1 (second example)

@stephane71
Copy link

Hi,

I have the same problem with the scrollToPosition and React 16.4.

Here is what I have found:
A bug fix has been done on React 16.4 on the getDerivedStateFromProps, see https://reactjs.org/blog/2018/05/23/react-v-16-4.html#bugfix-for-getderivedstatefromprops

The important part is:

getDerivedStateFromProps is now called every time a component is rendered, regardless of the cause of the update. Previously, it was only called if the component was re-rendered by its parent, and would not fire as the result of a local setState.

Here is the code from React Virtualized Grid in the getDerivedStateFromProps

Object.assign(
newState,
Grid._getScrollToPositionStateUpdate({
prevState,
scrollLeft: nextProps.scrollLeft,
scrollTop: nextProps.scrollTop,
}),

The nextProps.scrollTop always erase the new value of prevState.scrollTop because it's not anymore the previous state but the new one.

@john-veresk
Copy link
Contributor

I've made a PR which should fix this issue.

@bhearsum
Copy link

bhearsum commented Jan 9, 2020

Has this regressed for anyone else? It looks like it got backed out in #1446.

@wuweiweiwu
Copy link
Contributor

Will investigate!

@shubhamraj-appa-curefit

Facing similar issues with scroll to index / scroll to row

@bhearsum
Copy link

bhearsum commented Jan 22, 2020

I just noticed that in my case, I'm going from react/react-dom 16.8.6 -> 16.12.0 and hitting the issue -- not 16.3 -> 16.4 like the original rerporter. Makes me wonder if there's a new issue that has come up.

EDIT: Turns out my testing was invalid. This most recently broke with the 9.21.2 release, regardless of React version. See also #1226

@L5eoneill
Copy link

L5eoneill commented Jan 23, 2020

(Deleted... user error :-D My widget that customizes Virtualized table wasn't sending a prop for scrollToRow)

@erikrahm
Copy link

erikrahm commented Feb 3, 2020

@wuweiweiwu were you able to track this down at all?

@michchan
Copy link

michchan commented Mar 2, 2020

@mozbhearsum

NOT Working with WindowScroller

For my case, I tested it works when NOT using the window scroller (for both scrollToRow and scrollToPosition),
while it doesn't work when using with WindowScroller.

"react": "^16.8.5",
"react-dom": "^16.8.5",
"react-virtualized": "^9.21.2"

@erikrahm
Copy link

erikrahm commented Mar 2, 2020

@mozbhearsum

NOT Working with WindowScroller

For my case, I tested it works when NOT using the window scroller (for both scrollToRow and scrollToPosition),
while it doesn't work when using with WindowScroller.

"react": "^16.8.5",
"react-dom": "^16.8.5",
"react-virtualized": "^9.21.2"

Yeah it's still not working with WindowScroller

@bhearsum
Copy link

bhearsum commented Mar 2, 2020

@mozbhearsum

NOT Working with WindowScroller

For my case, I tested it works when NOT using the window scroller (for both scrollToRow and scrollToPosition),
while it doesn't work when using with WindowScroller.

"react": "^16.8.5",
"react-dom": "^16.8.5",
"react-virtualized": "^9.21.2"

Doesn't work for me. I haven't tried any workarounds, though.

@michchan
Copy link

michchan commented Mar 3, 2020

@mozbhearsum

NOT Working with WindowScroller

For my case, I tested it works when NOT using the window scroller (for both scrollToRow and scrollToPosition),
while it doesn't work when using with WindowScroller.

"react": "^16.8.5",
"react-dom": "^16.8.5",
"react-virtualized": "^9.21.2"

I opened another issue for that #1507

@sAnti09
Copy link

sAnti09 commented Jul 23, 2020

Any updates on this?

@zhizas
Copy link

zhizas commented Jul 27, 2020

Any updates on this?

+1

@sAnti09
Copy link

sAnti09 commented Aug 7, 2020

This still doesn't work. I've moved out from react virtualized to react window.

@wuweiweiwu
Copy link
Contributor

please try the latest version! https://www.npmjs.com/package/react-virtualized/v/9.22.2

@bhearsum
Copy link

please try the latest version! https://www.npmjs.com/package/react-virtualized/v/9.22.2

It's not fixed for me.

@michchan
Copy link

My quick workaround was like that:
#1507 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet