bvaughn / react-virtualized

React components for efficiently rendering large lists and tabular data
http://bvaughn.github.io/react-virtualized/
MIT License
26.14k stars 3.06k forks source link

Multigrid scrollToRow/scrollToColumn then scrolling in header no longer syncs with body scroll #1170

Open yang opened 6 years ago

yang commented 6 years ago

What is the current behavior?

In the Multigrid demo:

https://bvaughn.github.io/react-virtualized/#/components/Multigrid

When you enter a scrollToRow of (say) 50, and then use your trackpad/scroll wheel while hovered over the left header column to scroll up/down, only the left header column scrolls, and not the body.

What is the expected behavior?

The whole Multigrid should scroll as normal, as before there was a scrollToRow/Column specified, and as if your mouse were hovered over the body (scrolling within the body still works as expected).

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 67
OS macOS 10.13
React whatever's used in the demo site as of this time
React DOM same as above
react-virtualized same as above, but also ran into this in 9.20.0
jacoporicare commented 5 years ago

I agree, I think this PR https://github.com/bvaughn/react-virtualized/pull/1130 caused it. I cannot use ScrollSync with ArrowKeyStepper together anymore :(.

HUSSTECH commented 5 years ago

Hello, I ran into this issue today, and in the process of digging around in the code to follow the steps already taken in the linked PR and issue, I made an edit which stopped the issue from happening to me. Diff below to allow you to test, also it is on a branch of my fork here

diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js
index e2f4392..684eda8 100644
--- a/source/Grid/Grid.js
+++ b/source/Grid/Grid.js
@@ -819,33 +819,27 @@ class Grid extends React.PureComponent<Props, State> {
   static getDerivedStateFromProps(
     nextProps: Props,
     prevState: State,
   ): $Shape<State> {
     const newState = {};

     if (
       (nextProps.columnCount === 0 && prevState.scrollLeft !== 0) ||
       (nextProps.rowCount === 0 && prevState.scrollTop !== 0)
     ) {
       newState.scrollLeft = 0;
       newState.scrollTop = 0;

-      // only use scroll{Left,Top} from props if scrollTo{Column,Row} isn't specified
-      // scrollTo{Column,Row} should override scroll{Left,Top}
-    } else if (
-      (nextProps.scrollLeft !== prevState.scrollLeft &&
-        nextProps.scrollToColumn < 0) ||
-      (nextProps.scrollTop !== prevState.scrollTop && nextProps.scrollToRow < 0)
-    ) {
+    } else {
       Object.assign(
         newState,
         Grid._getScrollToPositionStateUpdate({
           prevState,
           scrollLeft: nextProps.scrollLeft,
           scrollTop: nextProps.scrollTop,
         }),
       );
     }

I'm still working through the code to figure out why it works! :sweat_smile: but, nothing is broken so far for me. There are two tests failing if you run yarn test, but doing the test steps manually does give the correct result.

Sorry that I can't be more certain, but I just opened up this codebase today. But I wanted to add my update to this thread in case anyone more familiar with the repo can help point in the right direction from here.

Liooo commented 5 years ago

@jacoporicare you're right, thanks. reverting to v9.19.1 did it for me.

ddrozdov commented 5 years ago

@HUSSTECH, your change may break #1123 fix. For me the workaround is, just after setting scrollToRow property to some value, reset it back to undefined using setTimeout, so it doesn't prevent using scrollTop anymore on further user scroll events.

jordanrolph commented 4 years ago

Similar to @ddrozdov, our workaround for this is to reset scrollToRow back to undefined. However we do this inside the onScroll method, rather than as a setTimeout.

This maintains the scrollToRow functionality, and still lets the user manually scroll fixed multigrid columns. We found it easier to implement than using a promise/set timeout method.

Here's a simplified example:

onScroll = scrollInfo => {
    if (this.state.scrollToRow !== undefined) {
        this.setState({ scrollToRow: undefined });
    }
    // do any other stuff you want to happen when the user scrolls
}
<MultiGrid 
    onScroll={this.onScroll)
    scrollToRow={this.state.scrollToRow}
    // other props 
/>
ButuzGOL commented 4 years ago

Seems like it doesn't work for scrollToColumn (

ButuzGOL commented 4 years ago

alternative

const value = { scrollTop: 0, scrollLeft: 0 };
mainGrid.current._bottomRightGrid.scrollToPosition(value);
mainGrid.current._topRightGrid.scrollToPosition(value);
mainGrid.current._topLeftGrid.scrollToPosition(value);
mainGrid.current._topRightGrid.scrollToPosition(value);
dasler-fw commented 4 months ago

I figured out that 'scrollToRow' is really hard to use for dynamic value. Just replace it by 'scrollTop'. For example, if your case is so simple like: 'I need to scroll my component to first row by some dynamic value with comparison' - just set 'scrollTop' to '1'.

In short: If your scrollToRow props is not working, try to replace it by the scrollTop like:

const getScrollTo = (value) => {
  if (isShouldUseLikeScrollToRow) {
    const rowHeight = 50;
    return value * rowHeight; // here write your own formula to calculate the scroll position.
  }
}

<Grid
  scrollTop={getScrollTo(10)}
  scrollToRow={scrollToRow} // don't use it for changing scroll position dynamically
/>