-
-
Couldn't load subscription status.
- Fork 32.7k
[docs][Autocomplete] Update virtualization example to use react-window v2 #47054
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
Conversation
Netlify deploy previewhttps://deploy-preview-47054--material-ui.netlify.app/ Bundle size report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sai6855 Maybe we should migrate our docs demos from react-window and react-virtuoso to @tanstack/react-virtual, given its higher adoption. See: https://npmtrends.com/@tanstack/react-virtual-vs-react-virtualized-vs-react-virtuoso-vs-react-window. cc @siriwatknp
If you think it is not straightforward, I can review this PR. But first, what do you think of migrating to @tanstack/react-virtual?
I agree that @tanstack/react-virtual has higher adoption, but since react-window still has around 3M downloads in the past year and the community is expecting a v2 versions of demos, I think it’s not ideal to completely migrate away from react-window. Maybe we can support both, but removing react-window entirely doesn’t seem ideal to me. |
Agree.
It's extremely painful to review as well. 😓 |
|
@ZeeshanTamboli Cleaned up code and fixed all issues reported by you here 478696e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just these two comments. Rest looks good to me! Will give a final review later.
@ZeeshanTamboli Both are fixed now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sai6855 When I am opening the popup of the Material UI Autocomplete Virtualization demo locally, I am getting the following error:
NaNis an invalid value for thetopcss style property.
@ZeeshanTamboli can you check now? The error occurred because the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sai6855 Sorry for not finding it earlier, but I found two issues in Joy UI's Autocomplete Virtualization demo:
- Keyboard Navigation is not working by scrolling to the new options on ArrowDown, PageDown, Home, End key presses.
- Same console error as #47054 (review)
Actually Joy UI isn't maintained anymore. Can we avoid upgrading react-window in it somehow?
You can do aliasing like:
{
"dependencies": {
"react-window": "^2.2.1",
"react-window-v1": "npm:react-window@^1.8.11"
}
}But I think it would be better to fix the issues.
As joy-ui isn't matained anymore, i haven't spent time fixing it. Additionally i tried not upgrading joy-ui version but it's not possible as both material-ui and joy-ui are using same package. Do you think we should introduce aliasing Given that all this code will be removed anyway? |
Aliasing would just complicate dependency maintenance by adding two versions of |
|
@ZeeshanTamboli keyboard navigation issue is fixed in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Nice work!

closes #46850
address https://mui-org.slack.com/archives/C0757QYLK7V/p1759503478232929
Initially i fixed this issue in #46900 but CI in that PR is failing for some reason i'm not aware of, moving changes to new PR fixed the CI. Additionally this PR also have fix reported by caspardue here #46900 (comment)
Migration to
react-windowv2 is extremely painful there is no official migration guide. I went through v2 docs and v1 docs to figure out what all changed.these are all the resources i could find.
Previews of all demos that effected by version upgrade
Regarding argos failure, I'm not really sure why it's failing. Looking at DOM of both v1 and v2, react-window has removed
outerElementsupport andinnerElementsupport (source), I suspect it is what causing argos issue. I think diff shown in argos is extremly minimal, since it's causing by external library and there is not enough guidance on migrartion, can we approve the change?