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

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

Closed
yang opened this issue Jul 11, 2018 · 8 comments

Comments

@yang
Copy link

yang commented Jul 11, 2018

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
Copy link

I agree, I think this PR #1130 caused it.
I cannot use ScrollSync with ArrowKeyStepper together anymore :(.

@HUSSTECH
Copy link

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! 😅 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
Copy link

Liooo commented Dec 27, 2018

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

@ddrozdov
Copy link

@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
Copy link

jordanrolph commented Oct 9, 2019

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
Copy link

Seems like it doesn't work for scrollToColumn (

@ButuzGOL
Copy link

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

ryan953 added a commit to getsentry/sentry that referenced this issue Apr 27, 2023
…c row (#48084)

The new button to make things scroll, it looks like this:


![SCR-20230427-jyet](https://user-images.githubusercontent.com/187460/234952387-4415b1c9-8efc-4a80-844a-774cda5263f2.png)


Also note that I had to make the text truncation happen on the other
side (dots are on the left now) for values in this table. This impacts
the Replay>Tags tab, but I don't think it's harmful.

With tips from bvaughn/react-virtualized#1170

Depends on #48073
@dasler-fw
Copy link

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

@github-actions github-actions bot added the Stale label Oct 6, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants