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

[FormLabel] Style overrides with variants do not work #44699

Closed
Gopikrishna19 opened this issue Dec 8, 2024 · 13 comments · Fixed by #44752
Closed

[FormLabel] Style overrides with variants do not work #44699

Gopikrishna19 opened this issue Dec 8, 2024 · 13 comments · Fixed by #44752
Assignees
Labels
bug 🐛 Something doesn't work component: FormControl The React component customization: theme Centered around the theming features

Comments

@Gopikrishna19
Copy link

Gopikrishna19 commented Dec 8, 2024

Sandbox

https://codesandbox.io/p/sandbox/https-github-com-mui-material-ui-issues-44699-xgdw5j

Steps to reproduce

Plug this in the createTheme component overrides:

export const formLabel: ThemeOptions['components'] = {
    MuiFormLabel: {
        styleOverrides: {
            root: {
                variants: [
                    {
                        style: ({theme}) => ({
                            color: '#000',
                            fontWeight: 'bold'
                        })
                    }
                ]
            }
        }
    }
};

Current behavior

It generates a stylesheet with a 1 in it.

  .css-x7vhj8-MuiFormLabel-root 1 { /* has 1 in it */
    color: #000;
    font-weight: bold;
  }

Expected behavior

it should not have 1 in it, just like other components.

  .css-x7vhj8-MuiFormLabel-root {
    color: #000;
    font-weight: bold;
  }

Context

Applying custom style to FormLabel component through createTheme. Using root as a function is the alternative that works. But I'd like to stay consistent with the variant override code style.

Your environment

npx @mui/envinfo
  System:
    OS: macOS 14.6.1
  Binaries:
    Node: 20.18.0 - ~/.nvm/versions/node/v20.18.0/bin/node
    npm: 10.9.1 - ~/Developer/fuel-react/node_modules/.bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 131.0.6778.86    # current testing env
    Edge: 131.0.2903.86
    Safari: 17.6
  npmPackages:
    @emotion/react:  11.13.5 
    @emotion/styled:  11.13.5 
    @mui/core-downloads-tracker:  6.1.9 
    @mui/icons-material:  6.1.9 
    @mui/material:  6.1.9 
    @mui/private-theming:  6.1.9 
    @mui/styled-engine:  6.1.9 
    @mui/system:  6.1.9 
    @mui/types:  7.2.19 
    @mui/utils:  6.1.9 
    @types/react: 18.3.12 => 18.3.12 
    react:  18.3.1 
    react-dom:  18.3.1 
    typescript: 5.7.2 => 5.7.2 

Search keywords: FormLabel, form-label, form label style

@Gopikrishna19 Gopikrishna19 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 8, 2024
@mj12albert
Copy link
Member

@Gopikrishna19 Would you mind providing a repro in CodeSandbox/stackblitz?

@mj12albert mj12albert added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 9, 2024
@Gopikrishna19
Copy link
Author

Gopikrishna19 commented Dec 9, 2024

@mj12albert sorry about the delay. Here you go :)
https://codesandbox.io/p/sandbox/9tdxml

Testing: comment/uncomment the ways of overriding the styles, and in the devTools > Elements tab, search for bah: to locate the stylesheet generated by the override

Two notes: The sandbox uses 6.1.10, and I used blue as opposed to the reported color black for the font to make it apparent.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Dec 9, 2024
@mj12albert mj12albert added bug 🐛 Something doesn't work customization: theme Centered around the theming features component: FormControl The React component and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 10, 2024
@mj12albert mj12albert changed the title FormLabel style overrides with variants does not work [FormLabel] Style overrides with variants do not work Dec 10, 2024
@mj12albert mj12albert self-assigned this Dec 10, 2024
@mj12albert mj12albert added status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it and removed bug 🐛 Something doesn't work labels Dec 10, 2024
@mj12albert
Copy link
Member

@Gopikrishna19 Hey thanks for the repro, FormLabel does not support the variant prop internally (I'm unsure if this is intentional yet), but InputLabel does and wraps the FormLabel component

I'm not sure what your exact use-case is, but does using InputLabel work for you?

@Gopikrishna19
Copy link
Author

I am trying to implement this example: https://mui.com/material-ui/react-checkbox/#formgroup.

I thought InputLabel was for TextField and Input components, while FormLabel was for form controls. Do you know if my understanding is incorrect? Overloading InputLabel seems like a hassle though, trying to identify where it is used.

As to why I want to use it, its just code style and code consistency. Every component we are overriding now is using the variants API, which is cleaner than the big blob of CSS on the root.

Either way, the variant API should be consistent, shouldn't it? I am finding out FormControl doesn't work as well. Has the same 1 in the generated stylesheet.

@Gopikrishna19
Copy link
Author

Gopikrishna19 commented Dec 11, 2024

Additional components with same issues:
MuiFormControl-root
MuiCardHeader-root (-title and -action works just fine)
MuiAutocomplete-popupIndicator (others work)

@DiegoAndai
Copy link
Member

Hey! This seems to be a bug caused by FormLabel's overridesResolver processing the styles incorrectly. See:

overridesResolver: ({ ownerState }, styles) => {
return {
...styles.root,

Here, styles.root is an array of objects, so this doesn't work.

For comparison, see the Button's implementation:

return [
styles.root,

This is also true for the other reported components:

These are the only ones that destructure styles.root like this.

Changing these to follow the Button's implementation fixes the issue.

@siriwatknp, you're more familiar with the implementation of variants. Does this make sense?

@DiegoAndai DiegoAndai added bug 🐛 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it labels Dec 12, 2024
@siddhantantil39
Copy link
Contributor

Hi @DiegoAndai I have started to work on it. I hope that's okay?

@Gopikrishna19
Copy link
Author

MuiAutocomplete-popupIndicator is also not working

@mj12albert mj12albert removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Dec 13, 2024
@mj12albert
Copy link
Member

@siddhantantil39 Thanks,assigned you ~

@Gopikrishna19
Copy link
Author

Looks like those are only finds? https://github.com/search?q=repo%3Amui%2Fmaterial-ui+...styles.root+path%3A%2F%5Epackages%5C%2Fmui-material%5C%2Fsrc%5C%2F%2F&type=code

I shall let you folks know if I find anything else. Thank you all for the quick resolution ❤️ ❤️ ❤️

@siddhantantil39
Copy link
Contributor

@mj12albert I have added the changes in my PR. Can anyone please review the PR.

@siriwatknp
Copy link
Member

you're more familiar with the implementation of variants. Does this make sense?

Yes, that makes sense. It should always be an array.

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@Gopikrishna19 How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

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: FormControl The React component customization: theme Centered around the theming features
Projects
None yet
5 participants