react-virtualized icon indicating copy to clipboard operation
react-virtualized copied to clipboard

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

Open yang opened this issue 7 years ago • 8 comments

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

yang avatar Jul 11 '18 18:07 yang

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

jacoporicare avatar Jul 23 '18 12:07 jacoporicare

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.

HUSSTECH avatar Dec 19 '18 21:12 HUSSTECH

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

Liooo avatar Dec 27 '18 23:12 Liooo

@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.

ddrozdov avatar Feb 25 '19 14:02 ddrozdov

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 
/>

jordanrolph avatar Oct 09 '19 10:10 jordanrolph

Seems like it doesn't work for scrollToColumn (

ButuzGOL avatar Jan 15 '20 16:01 ButuzGOL

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);

ButuzGOL avatar Jan 17 '20 14:01 ButuzGOL

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
/>

dasler-fw avatar Mar 05 '24 09:03 dasler-fw