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

[system] Fix dark mode flicker using useEnhancedEffect #44812

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Dec 19, 2024

This is a bug on client-side apps only.
For server-side apps, the mode is correctly set before the app render by InitColorSchemeScript component.

Root cause

The logic of updating the html's class attribute lives within useEffect instead of useLayoutEffect.

Even though, the first render has mode dark based on noSsr prop, the class="dark" applied to <html> happens after Browser has painted the first render.

The fix is to use useEnhancedEffect (a wrapper of useLayoutEffect for client-side) to attach class="dark" before browser paint.

Before:

The right button renders with default blue before turning red.

Screen.Recording.2567-12-19.at.17.03.21.mov

After:

Screen.Recording.2567-12-19.at.17.02.24.mov

@siriwatknp siriwatknp added customization: theme Centered around the theming features package: material-ui Specific to @mui/material package: system Specific to @mui/system labels Dec 19, 2024
@siriwatknp siriwatknp changed the title [material-ui] Fix dark mode flicker using useEnhancedEffect [system] Fix dark mode flicker using useEnhancedEffect Dec 19, 2024
@mui-bot
Copy link

mui-bot commented Dec 19, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 93d5653

@siriwatknp siriwatknp force-pushed the fix/flicker-client-side branch from 19316e2 to 93d5653 Compare December 20, 2024 03:40
@siriwatknp siriwatknp marked this pull request as ready for review January 2, 2025 04:18
@siriwatknp siriwatknp requested a review from DiegoAndai January 2, 2025 04:18
Copy link
Member

@aarongarciah aarongarciah left a comment

Choose a reason for hiding this comment

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

Nice improvement. Was the flicker always there or is it a "regression" in v6?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: theme Centered around the theming features package: material-ui Specific to @mui/material package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants