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][Slider][CssVarsProvider] Custom SliderThumb with CssVarsProvider is broken after upgrade @mui/material to 5.15.14 #41753

Closed
vimutti77 opened this issue Apr 3, 2024 · 7 comments · Fixed by #42370
Assignees
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material package: pigment-css Specific to @pigment-css/* priority: important This change can make a difference regression A bug, but worse

Comments

@vimutti77
Copy link
Contributor

vimutti77 commented Apr 3, 2024

Steps to reproduce

Link to live example: (required)

Steps:

  1. Go to https://stackblitz.com/edit/react-ceuj3r?file=Demo.tsx
  2. You will see the position of SliderThumb is broken
  3. Hover anywhere in the demo screen
  4. You will see that '&:hover': { background: 'red' } has been applied to every elements

Current behavior

Position of the SliderThumb is broken
image

'&:hover': { background: 'red' } has been applied to every elements
image

Expected behavior

Position of the SliderThumb is correct
image

'&:hover': { background: 'red' } is only applied to SliderThumb
image

Context

I have a project that use CssVarsProvider and have a custom Slider. After I upgrade @mui/material to 5.15.14, the style is broken.

Your environment

npx @mui/envinfo
  System:
    OS: Windows 11 10.0.22631
  Binaries:
    Node: 20.10.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.3 - C:\Program Files\nodejs\npm.CMD
    pnpm: Not Found
  Browsers:
    Chrome: Not Found
    Edge: Chromium (123.0.2420.65)
  npmPackages:
    @emotion/react: ^11.11.4 => 11.11.4
    @emotion/styled: ^11.11.5 => 11.11.5
    @mui/base: 5.0.0-beta.40 => 5.0.0-beta.40
    @mui/core-downloads-tracker:  5.15.14
    @mui/icons-material: ^5.15.14 => 5.15.14
    @mui/material: ^5.15.14 => 5.15.14
    @mui/private-theming:  5.15.14
    @mui/styled-engine:  5.15.14
    @mui/system: ^5.15.14 => 5.15.14
    @mui/types:  7.2.14
    @mui/utils:  5.15.14
    @mui/x-charts: ^7.1.0 => 7.1.0
    @mui/x-data-grid:  6.19.8
    @mui/x-data-grid-premium: ^6.19.8 => 6.19.8
    @mui/x-data-grid-pro:  6.19.8
    @mui/x-date-pickers:  5.0.20
    @mui/x-date-pickers-pro: ^5.0.20 => 5.0.20
    @mui/x-license-pro:  6.10.2
    @types/react: ^18.2.74 => 18.2.74
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ^5.4.3 => 5.4.3

Search keywords: CssVarsProvider, Slider, SliderThumb

@vimutti77 vimutti77 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 3, 2024
@vimutti77 vimutti77 changed the title Custom SliderThumb with CssVarsProvider is broken after upgrade @mui/material to 5.15.14 [material-ui][Slider][CssVarsProvider] Custom SliderThumb with CssVarsProvider is broken after upgrade @mui/material to 5.15.14 Apr 3, 2024
@unaisshemim
Copy link

can you assign this to me

@danilo-leal danilo-leal added component: slider This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Apr 3, 2024
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work regression A bug, but worse priority: important This change can make a difference package: pigment-css Specific to @pigment-css/* and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 13, 2024
@vimutti77
Copy link
Contributor Author

After upgrading to 5.15.17 the position has been fixed, but '&:hover': { background: 'red' } still applies to every elements.

@ZeeshanTamboli
Copy link
Member

It's interesting that it works well with ThemeProvider: https://stackblitz.com/edit/react-ceuj3r-qlzp31?file=Demo.tsx or without any theme provider: https://stackblitz.com/edit/react-ceuj3r-ybpmz3?file=Demo.tsx. It seems to be an issue specifically with CssVarsProvider.

@vimutti77
Copy link
Contributor Author

It's interesting that it works well with ThemeProvider: https://stackblitz.com/edit/react-ceuj3r-qlzp31?file=Demo.tsx or without any theme provider: https://stackblitz.com/edit/react-ceuj3r-ybpmz3?file=Demo.tsx. It seems to be an issue specifically with CssVarsProvider.

What is the current plan for CssVarsProvider?
My code currently uses a lot of theme.vars, so I cannot change back to ThemeProvider easily.
I cannot upgrade @mui/x-data-grid-premium to version 7 because it requires @mui/material at least v5.15.14.

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented May 16, 2024

I tried to debug it and noticed that the :hover effect is being applied globally. Here's a screenshot for reference:

cssvarsprovider

However, I couldn't determine why this is happening. @siriwatknp, could you take a look?

As a temporary workaround to unblock your upgrade (if it's crucial to upgrade @mui/x-data-grid-premium to version 7), you could try using the slider's thumb class:

const StyledSlider = styled(Slider)(() => {
  return {
    [`& .${sliderClasses.thumb}`]: {
      '&:hover': {
        background: 'red',
      },
    },
  };
});

You can find the StackBlitz example here.

@mnajdova
Copy link
Member

mnajdova commented May 23, 2024

It may be a regression from #41201, I can't see any other change that is related to this component.

@mnajdova
Copy link
Member

I found the issue, it was a wrong CSS value, that ended with "}" which broke the next CSS selector. I will cherry pick to master too, so we can release it next week.

To the original question, @vimutti77 it's safe to use the CssVarsProvider, we are stabilizing the API in v6. This issue was not related to it not working, it was a wrong CSS value in the component itself. Honestly, took me two hours to dissect the wrong line that produced this 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material package: pigment-css Specific to @pigment-css/* priority: important This change can make a difference regression A bug, but worse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants