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] Smooth performance animation #8986

Merged
merged 1 commit into from
May 20, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented May 14, 2023

Since #7846, we have a great performance demo, however, I noticed that we could improve the design of the animation, so I did for fun (overkill PR):

Screen.Recording.2023-05-14.at.19.19.12.mov

https://mui.com/x/react-data-grid/performance/#memoize-inner-components-with-react-memo. It smoothly appears (feels a bit laggy) and then instantly disappears (feels a bit brutal). I think that it should be reversed like Chrome whose repaints feel great:

Screen.Recording.2023-05-14.at.18.12.27.mov

For the colors, we could consider picking the ones of React Dev Tools' UX, but it feels a bit cheap:

Screen.Recording.2023-05-14.at.18.10.37.mov

Here is how it looks now: https://deploy-preview-8986--material-ui-x.netlify.app/x/react-data-grid/performance/#memoize-inner-components-with-react-memo

Screen.Recording.2023-05-14.at.19.26.13.mov

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation performance component: data grid This is the name of the generic UI component, not the React module! labels May 14, 2023
@mui-bot
Copy link

mui-bot commented May 14, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-8986--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 591.3 1,081.4 662.5 814.14 174.94
Sort 100k rows ms 583.3 1,283.8 583.3 974.64 264.944
Select 100k rows ms 206.1 364.4 302.1 292.58 61.214
Deselect 100k rows ms 163.7 282.1 214.3 223.84 45.694

Generated by 🚫 dangerJS against d3deb48

@noraleonte
Copy link
Contributor

I think the animation switch was a very good idea! I definitely agree that it looks better if the start is more abrupt, and it disappears more slowly. 🎉

However, I feel like I would still experiment with a smoother appearance (not as smooth as before, so we avoid making it feel super slow, but a 50ms should go a long way) - at least something to explore 💡

About the colors 🎨 I think we can improve a little bit.
I think we need a color that draws attention, but it should not steal the thunder from the other colorful elements such as the progress bars or the badges. It should also work well with the dark theme too. I would suggest a color that sits between the previous blue-ish tone and the current green for the following reasons:

I feel like there is a bit of a color clash between the green, yellow, red and blue colors we are using for the other elements, and the background.
green clash
We could try something like this:
green suggestion

I feel like the interaction with the dark mode would look/feel better with a cooler tone of green:
Current:
green dark theme interaction
Suggestion:
green 2 dark interaction

Slightly off-topic: I noticed that the yellow progress bar has a lower opacity, and it automatically changes based on the background - this might be something we could look into in the future and choose a color shade that works well on multiple background with 100% opacity.
Screenshot 2023-05-16 154200

The colors I used for my examples:
background-color: rgba(135 245 199/ 25%); outline: 1px solid rgb(135 245 199 / 35%);

@romgrk
Copy link
Contributor

romgrk commented May 16, 2023

This section of the docs would go away if we go through with the row memoization by default, I wouldn't spend too much time on this.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 16, 2023

This section of the docs would go away if we go through with the row memoization by default, I wouldn't spend too much time on this.

Agree, sorry I should have clarified what I had in mind with the "(overkill PR)" in the PR description 😁

Slightly off-topic: I noticed that the yellow progress bar has a lower opacity

@noraleonte Yeah, depending on what we want to prioritize, I feel that we could indeed improve these demo progress bars, it's not very welcoming, e.g. on https://mui.com/x/

Screenshot 2023-05-16 at 15 22 08

The custom theme demo is already better:

Screenshot 2023-05-16 at 15 23 07

@noraleonte
Copy link
Contributor

noraleonte commented May 16, 2023

The custom theme demo is already better

@oliviertassinari I agree. The smooth corners, bolder font and slightly smoother colors make a huge difference. It's a cool improvement opportunity I think 🎉

@oliviertassinari oliviertassinari merged commit 658e8be into mui:master May 20, 2023
@oliviertassinari oliviertassinari deleted the improve-animation branch May 20, 2023 11:06
@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 20, 2023

I'm merging for now, we could continue to improve it, but since we are working on fixing the root problem of these rerenders, this demo might soon be removed from the docs. Thanks for the input.

Two points I think that can make as general rules:

  1. spending extra time to iron the look & feel is most often worth it (here because we will remove the demo, not this much)
  2. instant-appearing animations are better than ones that don't feel great. So when we struggle to make it feel great with an animation, it's better to make it instant. This applies to the date picker popup, etc.

@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ui labels Aug 18, 2023
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! design This is about UI or UX design, please involve a designer docs Improvements or additions to the documentation performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants