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

[docs] Revert icon search virtualization #43569

Merged
merged 13 commits into from
Sep 16, 2024
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Sep 2, 2024

@mui-bot
Copy link

mui-bot commented Sep 2, 2024

Netlify deploy preview

https://deploy-preview-43569--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against a49d647

@Janpot Janpot added the docs Improvements or additions to the documentation label Sep 2, 2024
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I don't understand, I would expect this to be smooth like in https://react.dev/reference/react/useDeferredValue#examples

but instead, once the full list of icons starts rendering, it never gets interrupted:

Screen.Recording.2024-09-03.at.00.49.07.mov

@Janpot
Copy link
Member Author

Janpot commented Sep 3, 2024

You can compare the input responsiveness with https://deploy-preview-43582--material-ui.netlify.app/material-ui/material-icons/ where I removed the deferred values. You don't even need to throttle CPU to notice the difference. Just type "a" and then backspace

but instead, once the full list of icons starts rendering, it never gets interrupted:

I'm not sure I follow. How do you know it never gets interrupted just by looking at the UI?

@oliviertassinari oliviertassinari added the package: icons Specific to @mui/icons label Sep 3, 2024
@Janpot Janpot changed the title [icons] Revert icons virtualization in the docs [docs] Revert icon search virtualization Sep 4, 2024
@Janpot Janpot marked this pull request as ready for review September 4, 2024 02:47
@Janpot Janpot marked this pull request as draft September 4, 2024 02:49
@Janpot Janpot marked this pull request as ready for review September 4, 2024 02:49
@Janpot Janpot requested a review from DiegoAndai September 9, 2024 11:28
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 13, 2024

@Janpot Oh, ok, strange. I see it behaving as expected on the PR preview (prod mode) but not in dev mode.

All the feedback that I left seems handled here. This looks like a clear step forward, I think it's much better 👍.

I used the opportunity to remove a lot of <div>: a7418a1.
Before: https://pagespeed.web.dev/analysis/https-deploy-preview-43168--material-ui-netlify-app-material-ui-material-icons/zrq6w27l77?form_factor=desktop
SCR-20240913-dcpa

After: https://pagespeed.web.dev/analysis/https-deploy-preview-43569--material-ui-netlify-app-material-ui-material-icons/ffwxi8dwji?form_factor=desktop
SCR-20240913-dbwb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: icons Specific to @mui/icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][docs] Material Icon search lags
4 participants