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

[data grid] Demo LazyLoadingGrid crashes in docs #11865

Closed
1 task
PontusAxelsson opened this issue Jan 30, 2024 · 14 comments · Fixed by #12080
Closed
1 task

[data grid] Demo LazyLoadingGrid crashes in docs #11865

PontusAxelsson opened this issue Jan 30, 2024 · 14 comments · Fixed by #12080
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation feature: Row loading Related to the data grid Row loading features plan: Pro Impact at least one Pro user regression A bug, but worse v7.x

Comments

@PontusAxelsson
Copy link

PontusAxelsson commented Jan 30, 2024

Steps to reproduce

  1. Visit https://next.mui.com/x/react-data-grid/row-updates/#lazy-loading
  2. ??
  3. demo LazyLoadingGrid crashes

Your environment

Tech Version
Version v7.0.0-beta.0
Netlify deploy https://65b76b6a52f53800081426d7--material-ui-x.netlify.app
Browser Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36

Search keywords:

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 30, 2024
@jyash97
Copy link

jyash97 commented Jan 30, 2024

I was looking into these issue, and the easy way to fix it is to add conditional operator here but it seems something is off with the visibleRows at first its empty which I suppose is correct considering the debounce.

But on closer inspection I found when we keep scrolling the visibleRows count goes to 10000 and we can see a lag in the scroll.

At first it shows no Rows shouldnt it be a loading flag ?

After adding conditional operator this is how it looks: @MBilalShafi is this correct behavior?

Screen.Recording.2024-01-30.at.8.38.08.PM.mov

@zannager zannager added docs Improvements or additions to the documentation component: data grid This is the name of the generic UI component, not the React module! labels Jan 30, 2024
@michelengelen michelengelen added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 31, 2024
@michelengelen michelengelen changed the title [docs] Demo LazyLoadingGrid crashes [data grid] Demo LazyLoadingGrid crashes in docs Jan 31, 2024
@michelengelen
Copy link
Member

Hey @jyash97 and thanks for reporting this. We will have a look into this asap! Thanks again! 🙇🏼

@gitstart
Copy link
Contributor

gitstart commented Feb 1, 2024

@michelengelen we would like to pick this up

@jyash97
Copy link

jyash97 commented Feb 1, 2024

@michelengelen we would like to pick this up

Hey 👋🏻

I already started looking into it 2 days back just waiting for confirmation from @michelengelen and team on correct behaviour

@michelengelen
Copy link
Member

Hey @jyash97 you can find the correct behavior in the docs for v6: Layout

If you want I can assign this to you to fix it.

@jyash97
Copy link

jyash97 commented Feb 1, 2024

@michelengelen sure thing

will take a look at the issue more, any way I can get help from team if needed?

@michelengelen
Copy link
Member

definitely ... You can either comment here (or on the PR that you will open) or via our Community Discord

@romgrk
Copy link
Contributor

romgrk commented Feb 1, 2024

Not sure if we already have an issue for this but this started after the sticky PR (#10059), so it's on my tasklist.

@jyash97 If you want to take this you can open a PR. For the correct behavior compare with the v6 version of the docs.

@jyash97
Copy link

jyash97 commented Feb 1, 2024

Not sure if we already have an issue for this but this started after the sticky PR (#10059), so it's on my tasklist

yes I was able to find the PR which caused the regression, I havent started fixing it yet to correct behaviour, any suggestions to start with or look for?

@romgrk
Copy link
Contributor

romgrk commented Feb 1, 2024

No I haven't touched that code yet, use the v6 equivalent example as the reference: https://mui.com/x/react-data-grid/row-updates/#lazy-loading

Note that this code is part of the commercial plans, so you'll need to sign the CLA if you open a PR.

@oliviertassinari

This comment was marked as resolved.

@romgrk
Copy link
Contributor

romgrk commented Feb 11, 2024

Yes it's a duplicate.

@jyash97 I'll pick this one up on Monday unless you're still interested in submitting a PR.

@jyash97
Copy link

jyash97 commented Feb 11, 2024

Hey @romgrk

Sorry for the delay, but you can go ahead and work on this one, don’t want to block the issue right now as I may not be able to look into the issue this week

@cherniavskii cherniavskii added v7.x bug 🐛 Something doesn't work feature: Row loading Related to the data grid Row loading features and removed bug 🐛 Something doesn't work labels Feb 27, 2024
Copy link

⚠️ This issue has been closed.
If you have a similar problem, please open a new issue and provide details about your specific problem.
If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @PontusAxelsson?
Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.

@cherniavskii cherniavskii added the plan: Pro Impact at least one Pro user label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation feature: Row loading Related to the data grid Row loading features plan: Pro Impact at least one Pro user regression A bug, but worse v7.x
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants