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

Help with keeping scrollLeft property in components in each row when scrolling vertically. #721

Open
merrylewang opened this issue Apr 16, 2024 · 7 comments

Comments

@merrylewang
Copy link

merrylewang commented Apr 16, 2024

Hi,

So I have a special use case in one particular column where there's a set scrollLeft value that is shared by the components in each cell, and that is controlled by a custom scrollbar not involved with the fixedDataTable. When I scroll vertically though, for whatever reason, the scrollLeft value gets reset to 0 for most of the rows. Once I adjust the custom scrollbar again, then whatever is in the visible window displays properly once more.

I wasn't sure how the built in vertically scrolling works in FixedDataTable, but it seems like something is happening that wipes the scrollLeft value. It seems like the rows are doing some recycling, but I'm not sure about the details.

Do you have any tips or advice?

I'm using FDT 2.0.10. And I previously tried with FDT 1.2.0 which had a similar issue but only for one of the rows that previously had "visibility: hidden" before scrolling up.

@pradeepnschrodinger
Copy link
Collaborator

Without a reproducible example or a video it's a bit difficult for me to confirm what's actually happening.
Any chance you can provide it? You can refer this codesandbox template.

I wasn't sure how the built in vertically scrolling works in FixedDataTable, but it seems like something is happening that wipes the scrollLeft value. It seems like the rows are doing some recycling, but I'm not sure about the details.

Yea, FDT recycles/reuses rows for performance, so any state in your cells may be reused across rows.
There are still some edge cases where an existing row gets unmounted if it goes outside the viewport.

So for example if you're storing scrollLeft as local state in a cell, and that cell gets unmounted along with the row, then any local changes to scrollLeft will also be wiped out.

I would suggest keeping your cell renderers pure as much as possible, and pulling up state from your cell renderers to the table level or above to avoid these sort of issues.

@merrylewang
Copy link
Author

merrylewang commented Apr 17, 2024

Thanks for the response. I was able to make a quick demo version of what I was running into over here: codesandbox.

Screen.Recording.2024-04-17.at.10.39.37.AM.mov

As I scroll up in the end, the scrollLeft stays the same as intended for a little bit, but then it snaps back to 0 at a certain time.


This is when I use the 1.2.0 version of FDT. It only does the reset for some rows and the snap back pretty quickly, but still not ideal.

Screen.Recording.2024-04-17.at.10.44.36.AM.mov

@pradeepnschrodinger
Copy link
Collaborator

Thanks, the demo helps out a lot.

This is quite strange. The underlying issue seems to be that the browser resets the scroll state of a node whenever it gets moved to a different position.
So when FDT recycles rows by shifting them around, your cells in that row loose their scroll state.

Here's a demo to highlight this -- https://codesandbox.io/s/test-scroll-state-vkxn5y?file=/src/App.js.
It renders the <SpecialCell /> component taken from your demo, along with a button that simply flips the position of the components in the DOM.
In my testing, the browser seems to be resetting the scroll whenever the button is pressed...

@AmanGupta2708 @karry08 @abhisheksingh2410 , have you guys seen anything like this before?

@pradeepnschrodinger
Copy link
Collaborator

@merrylewang , here's a sandbox where I put a hacky workaround.
I know this doesn't really solve the underlying issue, but sharing in case it might unblock your needs.

The main change is that I modified <SpecialCellRenderer> to use FDT's builtin context so that the cell gets rerendered whenever the state of the table (like scroll values) change.

  // workaround to make the cell rerender when vertical scroll changes
  const data = useContext(Context);

  useLayoutEffect(() => {
    if (!ref.current) {
      return;
    }

    if (ref.current.scrollLeft !== props.scroll) {
      ref.current.scrollTo({ left: props.scroll });
    }
  });

The useLayoutEffect runs before dom updates and lets us check if the cell's scroll offset differs from what's expected on every render, and if so simply sets it back.

@merrylewang
Copy link
Author

I tried copying what you had in the sandbox, but wasn't able to get it to work in my own application. Do you know what I'm doing wrong? Currently the only two things I'm adding is wrapping with Context.Consumer and switching to useLayoutEffect.

@pradeepnschrodinger
Copy link
Collaborator

Without seeing the full code it's a bit hard to debug.
If sharing the application is not possible, could you instead share snippets of:

  1. How the cell renderer is passed to FDT:
    eg:
    <Column
      columnKey="email"
      header={<Cell>Email</Cell>}
      cell={(cellProps) => {
        return (
          <Context.Consumer>
            {(contextValue) => (
              <SpecialCell
                {...cellProps}
                scroll={this.state.scrollLeft}
              />
            )}
          </Context.Consumer>
        );
      }}
      width={300}
      fixedRight={true}
    />
  1. And how the cell renderer actually makes use of the props?
    eg:
  useLayoutEffect(() => {
    if (!ref.current) {
      return;
    }

    if (ref.current.scrollLeft !== props.scroll) {
      ref.current.scrollTo({ left: props.scroll });
    }
  });

  return (
    <div style={outer} ref={ref}>
      <div style={inner}>hi</div>
    </div>
  );

And I'm also happy to get in a call to help debug what's going wrong. You can send me an email at [email protected]

@merrylewang
Copy link
Author

Actually, I was able to get your fix working in my code. Not sure what exactly changed from before, but it works now. Thank you for your help on this!

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

2 participants