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

Rows not scrolling in beta-26 #482

Closed
tgolbs opened this issue Sep 16, 2019 · 5 comments
Closed

Rows not scrolling in beta-26 #482

tgolbs opened this issue Sep 16, 2019 · 5 comments
Assignees
Labels
bug help wanted regression issue that was "fixed" but got reopened later on

Comments

@tgolbs
Copy link

tgolbs commented Sep 16, 2019

Hi there,
when we started developing our application we started using beta-8. We now wanted to upgrade to the newest beta but do have the strange behaviour that the table will not scroll its content. It shows the shadows in the correct way though.

Expected Behavior

We would expect that the rows will be rerendered on scroll.

Current Behavior

No rerender happens on scroll.

Steps to Reproduce (for bugs)

https://codesandbox.io/embed/distracted-vaughan-crqe6?fontsize=14

@tgolbs tgolbs changed the title Rows not scrolling in beta Rows not scrolling in beta-26 Sep 16, 2019
@pradeepnschrodinger
Copy link
Collaborator

We had a couple people report this on the latest versions of beta, but none had a reproducible example we could start looking into, and for some reason it worked just fine in our app which uses latest FDT.

Strangely this also passed the tests, and nothing seemed wrong in the examples. So not sure why this happens. Anyway I'll definitely start looking into this, now that there's a reproducible example.

Thanks for reporting with a sandbox link 😄

@tgolbs
Copy link
Author

tgolbs commented Sep 16, 2019

You're welcome. I am quite happy, that it is not some misconfiguration on my side, because there were times i strongly questioned myself :D
We tried switching to a newer beta version before (i think that this was beta-23) and the same behaviour was observed but we did not have time to look into it. So the change responsible for it must not be in the latest version update.

@pradeepnschrodinger
Copy link
Collaborator

pradeepnschrodinger commented Sep 16, 2019

Hey, thanks for the information. Yea, I feel it was introduced through #460, which went into v1.0.0-beta.21 (see this comment and continued discussion on the PR)

I do notice one thing. The latest beta works just fine with React 15.6.2, the one in FDT's package.json, which is also the one used in the site, and in our product. So that's probably why we could never reproduce the bug.

In the codepen, if you rollback react and react-dom version from 16.8.6 to 16.0.0-alpha.2 or less, it works well. I'm trying to figure out why it makes a difference though 😕

As for what happens:
It seems the render method for the rows are never being called after the initial table render.
Basically, all rows under FixedDataTableBufferedRows (which makes up the content) are never rendered more than once, no matter how much you scroll. FixedDataTableBufferedRows itself, however, is called properly during every scroll, so basically it's not rendering it's children, although it's clearly returned... Very weird.

We'll look more into this. @wcjordan, @apar03 , @mycrobe, @somavara, @miskreant , @quixotically , any of you know why this might have occurred? Is it some weird optimization introduced in later versions? Or are any of the lifecycle methods dead due to React version upgrade?

And most importantly(?), I'm trying to see why #460 introduced this issue.

@pradeepnschrodinger pradeepnschrodinger self-assigned this Sep 16, 2019
@pradeepnschrodinger pradeepnschrodinger added bug help wanted regression issue that was "fixed" but got reopened later on labels Sep 16, 2019
pradeepnschrodinger added a commit that referenced this issue Sep 18, 2019
Remove whitespace react element in `FixedDataBufferedRows`

See #483 on why this fixes scrolling in latest versions of React.
@pradeepnschrodinger
Copy link
Collaborator

@tgolbs , we released a fix for this. Please try latest beta v1.0.0-beta.28.

Again, thanks so much for the reproducible sandbox. Really helped in finding out the cause.

I'm closing this issue as it seems fixed, but feel free to continue the discussion if needed.

@tgolbs
Copy link
Author

tgolbs commented Sep 18, 2019

@pradeepnschrodinger Thank you very much! We upgraded to beta-28 and it now works like a charm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted regression issue that was "fixed" but got reopened later on
Projects
None yet
Development

No branches or pull requests

2 participants