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

[question] Issue with jumping x-grid starting from alpha.37 #2508

Closed
2 tasks done
bartvde opened this issue Aug 31, 2021 · 11 comments · Fixed by #2558
Closed
2 tasks done

[question] Issue with jumping x-grid starting from alpha.37 #2508

bartvde opened this issue Aug 31, 2021 · 11 comments · Fixed by #2558
Assignees
Labels
bug 🐛 Something doesn't work plan: Pro Impact at least one Pro user regression A bug, but worse

Comments

@bartvde
Copy link

bartvde commented Aug 31, 2021

Order id 💳

25229

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

The grid jumps, starting with alpha.37 (alpha.36 is fine). When clicking a row and then scrolling until this row is not visible anymore, the grid jumps.

The problem in depth 🔍

Video of what happens in alpha.36:
Peek 2021-08-31 12-20

Video of what happens in alpha.37 and later:
Peek 2021-08-31 12-20a

Context 🔦

No response

Your Environment 🌎

Chrome: Version 92.0.4515.159 (Official Build) (64-bit)

System:
OS: Linux 5.11 Ubuntu 21.04 (Hirsute Hippo)
Binaries:
Node: 16.3.0 - ~/.nvm/versions/node/v16.3.0/bin/node
Yarn: 1.22.10 - ~/.nvm/versions/node/v16.3.0/bin/yarn
npm: 7.20.5 - ~/.nvm/versions/node/v16.3.0/bin/npm
Browsers:
Chrome: 92.0.4515.159
Firefox: 91.0.2
npmPackages:
@material-ui/codemod: ^4.5.0 => 4.5.0
@material-ui/core: ^4.12.3 => 4.12.3
@material-ui/icons: ^4.11.2 => 4.11.2
@material-ui/lab: 4.0.0-alpha.60 => 4.0.0-alpha.60
@material-ui/pickers: ^3.3.10 => 3.3.10
@material-ui/styles: 4.11.4
@material-ui/system: 4.12.1
@material-ui/types: 5.1.0
@material-ui/utils: 4.11.2
@material-ui/x-grid: ^4.0.0-alpha.37 => 4.0.0-alpha.37
@material-ui/x-license: 4.0.0-alpha.37
@types/react: ^17.0.19 => 17.0.19
react: ^17.0.2 => 17.0.2
react-dom: ^17.0.2 => 17.0.2
typescript: ^4.3.5 => 4.3.5

@bartvde bartvde added plan: Pro Impact at least one Pro user support: question Community support but can be turned into an improvement status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 31, 2021
@bartvde bartvde changed the title [question] [question] Issue with jumping grid starting from alpha.37 Aug 31, 2021
@bartvde bartvde changed the title [question] Issue with jumping grid starting from alpha.37 [question] Issue with jumping x-grid starting from alpha.37 Aug 31, 2021
@m4theushw
Copy link
Member

Please provide a minimal reproduction test case with the latest version. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

@m4theushw m4theushw added the status: waiting for author Issue with insufficient information label Aug 31, 2021
@bartvde
Copy link
Author

bartvde commented Sep 1, 2021

will do today

@bartvde
Copy link
Author

bartvde commented Sep 1, 2021

@m4theushw you can reproduce it here: https://stackblitz.com/edit/eqrt4n?file=demo.js

Peek 2021-09-01 09-20

Edit Olivier: Updated reproduction:

@zzossig
Copy link
Contributor

zzossig commented Sep 3, 2021

Jumping issue still exist.
This is just happened sometimes.
In my case, when I click checkbox or when cell contains image, jumping happened.
This bug is very difficult to reproduce so thank you @bartvde for taking time.
I don't want my users experience this bug.

@bartvde
Copy link
Author

bartvde commented Sep 6, 2021

@m4theushw any update on this one? TIA

@Primajin
Copy link

Primajin commented Sep 6, 2021

This prevents us from upgrading beyond alpha.36 to 4.0.0 as our customers rely on the steady position of the rows

@m4theushw
Copy link
Member

We have a race condition between the selection and the focus management introduced in #2239. When a cell is clicked, the internal state is updated to store which cell received focus. The problem is that the selection is also listing to the click event. That way, whenever a cell is selected, the grid rerenders, but still with the old focused cell in the state. When the old focused cell is rendered it calls .focus(), which makes the browser to try to scroll to it, so the jump.

As a workaround, you can disable the selection on click with:

<DataGridPro disableSelectionOnClick />

You can use the checkboxSelection to still be able to select rows.

@m4theushw m4theushw added bug 🐛 Something doesn't work regression A bug, but worse and removed support: question Community support but can be turned into an improvement status: waiting for maintainer These issues haven't been looked at yet by a maintainer status: waiting for author Issue with insufficient information labels Sep 6, 2021
@m4theushw m4theushw self-assigned this Sep 6, 2021
@Primajin
Copy link

Primajin commented Sep 9, 2021

Great thanks - what's the anticipated release cycle for these fixes? Roughly - doesn't need to be super exact. Just for us to know when to plan for upgrading from alpha.36 to 4.0.1.

@m4theushw
Copy link
Member

@Primajin This bug fix will be included in the next release (probably next week), which will be v5.0.0-beta.0. However, v4 is not dead. Some of the bug fixes added to v5 will be cherry-picked to the older version. This includes this fix. We plan to release 4.0.1 once there're enough fixes to do a big release, maybe at the end of this month.

@bartvde
Copy link
Author

bartvde commented Oct 5, 2021

@m4theushw can you confirm if this made it into 4.0.1?

@m4theushw
Copy link
Member

@m4theushw can you confirm if this made it into 4.0.1?

It was not included. You can use MUI X v5 with MUI Core v4 by changing the CSS prefixes used. Here's an example of how to do it: https://codesandbox.io/s/datagriddemo-material-demo-forked-8myv0?file=/src/demo.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work plan: Pro Impact at least one Pro user regression A bug, but worse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants