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

Issue with Collection scrolling when data is loaded async #525

Closed
garylesueur opened this issue Jan 4, 2017 · 7 comments
Closed

Issue with Collection scrolling when data is loaded async #525

garylesueur opened this issue Jan 4, 2017 · 7 comments

Comments

@garylesueur
Copy link

https://plnkr.co/edit/81aoXFH8mkZhoxaKib2o

So first off, I've setup a plunker as requested. This is a snippet of what we are/were planning to use in production.

As it stands, the grid works as desired. When we started using real data, we came across a big problem.

To see the issue, on script.jsx - line 45, if you comment out the initial "this.populate()" then you can see the entire grid / page scrolling rather than just the items as desired.

At the moment I am cheating by seeding the grid with initial data but I'd like to not do this.

Thanks.

@bvaughn
Copy link
Owner

bvaughn commented Jan 4, 2017

Thank you for the Plnkr and the detailed bug description. Looks like this bug can be reproduced on the official site as well: https://bvaughn.github.io/react-virtualized/#/components/Collection

  1. Open page
  2. Change cell count to 0
  3. Change cell count back to 1000

The problem seems to be that the overflow: auto style is not present as of step 3. But logging the style object shows that I'm specifying that style... I'm a bit confused.

@bvaughn
Copy link
Owner

bvaughn commented Jan 4, 2017

Interesting... these lines are forcefully turning off scrollbars if the content is not tall or wide enough to require scrolling.

It seems that once I've set a value for overflowX: hidden then subsequently setting a style with overflow: auto (eg no overflowX property at all) doesn't stick.

This seems like a bug outside of react-virtualized.

I can fix it in react-virtualized though by making CollectionView styles do what Grid does.

bvaughn pushed a commit that referenced this issue Jan 4, 2017
It seems that after specifying an overflowX (or overflowY) style then subsequent overflow styles get dropped. This causes the broken behavior originally reported in #525. This commit resolves that issue.
@bvaughn
Copy link
Owner

bvaughn commented Jan 4, 2017

Fixed with b5b8e29. Will go out in a release sometime in the next couple of days.

In the meanwhile you should be able to work around this on your end by passing a style prop like:

<Collection
  {...props}
  style={{
    overflowX: 'auto',
    overflowY: 'auto'
  }}
/>

@bvaughn bvaughn closed this as completed Jan 4, 2017
@bvaughn
Copy link
Owner

bvaughn commented Jan 4, 2017

I've captured this bug with a small Plnkr and reported it here: facebook/react/issues/8689

@bvaughn
Copy link
Owner

bvaughn commented Jan 5, 2017

The underlying bug actually impacts all shorthand CSS properties. Turns out that shorthand properties aren't [fully?] supported by React due to how the style updates are currently done. I was unaware of this.

Either way, it's been fixed in react-virtualized for now and the fix will roll out with the next release.

@garylesueur
Copy link
Author

Superb, thanks for that. Just been reading up and I don't feel so bad about not solving it myself now 👍

Build failing btw

@bvaughn
Copy link
Owner

bvaughn commented Jan 5, 2017

Build is failing in CI because PhantomJS is crashing in the middle of tests:
screen shot 2017-01-05 at 10 00 15 am

I haven't had the time to look into it yet. It doesn't crash for me locally. (All tests pass.) If you have the time to look into it, I'd love a hand. Otherwise I'll get to it eventually.

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