Skip to content

A more accurate getEstimatedTotalWidth #812

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

Closed
tanin47 opened this issue Apr 6, 2025 · 7 comments
Closed

A more accurate getEstimatedTotalWidth #812

tanin47 opened this issue Apr 6, 2025 · 7 comments

Comments

@tanin47
Copy link

tanin47 commented Apr 6, 2025

Hello! Thank you for writing up such a wonderful library. I've been using VariableSizeGrid with react-virtualized-auto-sizer. I know the set of columns and their widths upfront.

I notice that, when there are many columns and I scroll to the right, it seems the initial scroll bar's width isn't accurate. The scroll bar's width jump to a completely new position once I scroll to the right most part.

I've found a fix for it, but I'm unable to tell why the previous code doesn't work: https://github.com/bvaughn/react-window/pull/811/files#diff-b0f813b993eaf874b282e409315e0a2348229d2f79fd9ee5fde673f5bfa127a9

With the fix, the initial scroll bar's width would be correct. When scrolling to the right most part, the scrollbar's width and its position don't jump.

If there's nothing obvious, I can build a PoC for it. I just wanted to open an issue first. Thank you.

@bvaughn
Copy link
Owner

bvaughn commented Apr 18, 2025

Hi @tanin47. Thanks for the comment and the link to the diff.

The code your change has deleted is there to avoid the overhead of having to eagerly measure every column or row in a large grid. (For example, let's say you have a list that's 10,000 rows long. I'm trying to avoid having to eagerly call the getItemSize function 1,000 times just to render the grid initially because (a) this function is user code and it might actually be doing something expensive and (b) in most cases, lists are rendered initially with an offset of 0 and so an estimated total height is sufficient for a scrollbar. As the user scrolls, the list keeps track of how far it has measured and refines the overall size (which can cause the scrollbar size to change but generally this change is gradual and not very noticeable over time).

Your proposed change would make the initial scrollbar size accurate but come with a potentially large performance cost. The recommendation I would normally make for people in the position you've described would be to try to provide a more accurate estimatedColumnWidth value. (I assume the default value of 50px is way off from your actual column sizes from the sound of it).

Let me know your thoughts!

@tanin47
Copy link
Author

tanin47 commented Apr 20, 2025

The app estimates width of each column based on the data inside. I'l try estimatedColumnWidth. Thank you.

As a side note, I use this patch with ~4M rows + ~50 columns in an electron app where it loads and queries large CSVs, and I don't experience any slowness.

Thank you so much. I'll use the option you mentioned.

@tanin47 tanin47 closed this as completed Apr 20, 2025
@bvaughn
Copy link
Owner

bvaughn commented Apr 20, 2025

Let me know if it works!

@tanin47 tanin47 reopened this Apr 27, 2025
@tanin47
Copy link
Author

tanin47 commented Apr 27, 2025

It doesn't quite work because, if I understand correctly, estimatedColumnWidth is a single estimate for every column. In my case, I know each column's width specifically.

If there's a way to provide: estimatedTotalWidth, that would work for me.

@tanin47 tanin47 closed this as completed Apr 27, 2025
@tanin47 tanin47 reopened this Apr 27, 2025
@bvaughn
Copy link
Owner

bvaughn commented Apr 27, 2025

If there's a way to provide: estimatedTotalWidth, that would work for me.

That's not a prop I want to add to this library. This use case kind of falls between the cracks of what I was aiming to support here. My suggestion would be to just use the innerElementType to override the estimated scrollable width. That prop is meant as kind of an escape hatch for more advanced use cases.

function InnerElementType({ style, ...rest }) {
  return (
    <div
      data-testid="example"
      style={{
        ...style,
        width: 100_000, // Whatever your width is
      }}
      {...rest}
    />
  );
}

@bvaughn bvaughn closed this as completed Apr 27, 2025
@tanin47
Copy link
Author

tanin47 commented Apr 27, 2025

This works. Thank you so much.

@bvaughn
Copy link
Owner

bvaughn commented Apr 27, 2025 via email

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

No branches or pull requests

2 participants