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

[material-ui] Spacing customization not working on 6.0.0-beta.6 when enabling CSS variables #43378

Closed
trungutt opened this issue Aug 20, 2024 · 5 comments · Fixed by #43389
Closed
Assignees
Labels
customization: theme Centered around the theming features regression A bug, but worse v6.x

Comments

@trungutt
Copy link

trungutt commented Aug 20, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/old-sunset-42dgnh?file=%2Fsrc%2FDemo.tsx%3A19%2C52

Steps:

  1. Go to Demo.tsx. This contains a little spacing customization for theme creation (so that we could call something like theme.spacing('xs') in downstream client).
  2. Comment/Uncomment cssVariables: true line. The goal is to switch theme creation between Non CSS Vars <> With CSS Vars theme
  3. Observe the change in height
Screen.Recording.2024-08-20.at.12.27.14.mov

Current behavior

With MUI version 6.0.0-beta.6, switching to CSS Vars theme doesn't take into account spacing customization.

I didn't try with MUI v5 as CSS Vars theming seems to be unstable there.

Expected behavior

With MUI version 6.0.0-beta.6, switching to CSS Vars theme should take into account spacing customization.

Context

We are migrating to MUI v6 (+ CSS Vars support) and our frontend infra flags this difference.

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: v6, customization, spacing

@trungutt trungutt added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 20, 2024
@trungutt trungutt changed the title [v6] [Customization] Spacing customization stops workingon v6 beta.6 [v6] [Customization] Spacing customization stops working on 6.0.0-beta.6 Aug 20, 2024
@trungutt
Copy link
Author

At first glance, maybe this comes from

const themeSpacing = getPath(theme, themeKey, true) ?? defaultValue;

somehow getPath behaves differently with CSS Vars theming

@zannager zannager added v6.x customization: theme Centered around the theming features labels Aug 20, 2024
@trungutt trungutt changed the title [v6] [Customization] Spacing customization stops working on 6.0.0-beta.6 [v6] [Customization] Spacing customization not working on 6.0.0-beta.6 Aug 20, 2024
@siriwatknp
Copy link
Member

siriwatknp commented Aug 21, 2024

With CSS variables enabled, what behavior of theme.spacing() do you expect?

@trungutt
Copy link
Author

We have that spacing customisation in Non Vars theme construction. That allows us to do something like theme.spacing('xs') that returns '100px' instead of doing theme.spacing(some_number), in that specific live example.

In reality we have more scales than just xs. The example works as expected without cssVariables: true, but switching to CSS Vars theme with cssVariables: true makes it stop working

@siriwatknp siriwatknp added regression A bug, but worse and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 21, 2024
@siriwatknp
Copy link
Member

We have that spacing customisation in Non Vars theme construction. That allows us to do something like theme.spacing('xs') that returns '100px' instead of doing theme.spacing(some_number), in that specific live example.

In reality we have more scales than just xs. The example works as expected without cssVariables: true, but switching to CSS Vars theme with cssVariables: true makes it stop working

Note that with this fix, when a custom spacing function is provided, a CSS variable for spacing will not be generated.

@siriwatknp siriwatknp changed the title [v6] [Customization] Spacing customization not working on 6.0.0-beta.6 [material-ui] Spacing customization not working on 6.0.0-beta.6 when enabling CSS variables Aug 21, 2024
@trungutt
Copy link
Author

@siriwatknp thanks for your quick reactivity 💯

@aarongarciah aarongarciah added this to the Material UI: v6 milestone Aug 21, 2024
@aarongarciah aarongarciah moved this to In progress in Material UI Aug 21, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Material UI Aug 22, 2024
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 regression A bug, but worse v6.x
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants