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

[Select] Recent removal of .MuiSelect-root class prevents specifically targeting root for only Select #30225

Open
2 tasks done
rathpc opened this issue Dec 15, 2021 · 11 comments
Assignees
Labels
component: select This is the name of the generic UI component, not the React module! status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it waiting for 👍 Waiting for upvotes

Comments

@rathpc
Copy link

rathpc commented Dec 15, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Following the merge of this PR into version 5.1.1, we can no longer specifically target the root element of a Select explicitly without also targeting all Inputs. Having this root class for example allowed you to be able to add different padding for a Select component at the root but not for other Inputs.

I understand it was removed as a result of also removing the root override slot because that usage was unclear - however I think we should still keep the class as that is useful for specificity purposes.

Expected behavior 🤔

.MuiSelect-root is still available as a class on the root element but the slot is not available to apply styleOverrides to.

Steps to reproduce 🕹

Before Select root was removed (v5.1.0): codesanbox

After Select root was removed (v5.2.4): codesandbox

If you compare the two you will see that erroneous padding is present on the right side of the select in the "After" option because it is no longer able to target the Select root to override this.

Context 🔦

Without this root class style overrides get very complicated, very quickly if you are trying to apply varying styles to selects vs inputs vs multiline inputs and there are some styles you may not want just on the input element itself, but rather the wrapper (root). This has also broken our Select styles and prevented us from upgrading beyond 5.1.0 unless we do quite a bit of unconventional style hacking to get our theme back to where it was.

Your environment 🌎

`npx @mui/envinfo`
  System:
    OS: macOS 11.6
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 169.88 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
    Yarn: 1.22.15 - /usr/local/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.17.6/bin/npm
  Managers:
    Homebrew: 3.3.7 - /usr/local/bin/brew
    pip3: 21.2.4 - /usr/local/bin/pip3
    RubyGems: 3.0.3 - /usr/bin/gem
  Utilities:
    Make: 3.81 - /usr/bin/make
    GCC: 4.2.1 - /usr/bin/gcc
    Git: 2.30.1 - /usr/bin/git
    Clang: 13.0.0 - /usr/bin/clang
  Servers:
    Apache: 2.4.48 - /usr/sbin/apachectl
  Virtualization:
    Docker: 20.10.11 - /usr/local/bin/docker
  IDEs:
    IntelliJ: 2021.2.2
    Nano: 2.0.6 - /usr/bin/nano
    VSCode: 1.62.3 - /usr/local/bin/code
    Vim: 8.2 - /usr/bin/vim
    Xcode: /undefined - /usr/bin/xcodebuild
  Languages:
    Bash: 3.2.57 - /bin/bash
    Java: 1.8.0_302 - /Users/steven.rathbauer/.sdkman/candidates/java/current/bin/javac
    Perl: 5.30.2 - /usr/bin/perl
    PHP: 7.3.29 - /usr/bin/php
    Python: 2.7.16 - /usr/bin/python
    Python3: 3.9.7 - /usr/local/bin/python3
    Ruby: 2.6.3 - /usr/bin/ruby
  Databases:
    SQLite: 3.32.3 - /usr/bin/sqlite3
  Browsers:
    Firefox Developer Edition: 95.0
    Safari: 15.1
  Monorepos:
    Yarn Workspaces: 1.22.15
    Lerna: 4.0.0
@rathpc rathpc added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 15, 2021
@oliviertassinari oliviertassinari added the component: select This is the name of the generic UI component, not the React module! label Dec 21, 2021
@hbjORbj hbjORbj changed the title Recent removal of ".MuiSelect-root" class causing inability to specifically target root for only Select's [Select] Recent removal of .MuiSelect-root class prevents specifically targeting root for only Select Jan 3, 2022
@hbjORbj hbjORbj 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 status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 3, 2022
@hbjORbj
Copy link
Member

hbjORbj commented Jan 3, 2022

Hi. I apologize for late reply.

We are aware of this constraint. As you can see here, we think that this is rather a healthy constraint.

I am against reverting the change we made in #29593 because providing the root class of Select but not supporting it is a bug. More details about the issue: #29593 (comment)
Supporting it is also complex because as @oliviertassinari said in here, "It could have been a footgun to be able to customize the Select root without the TextField root."

@rathpc
Copy link
Author

rathpc commented Jan 3, 2022

Yes, I understand that but I would also reference this comment and argue that the hypothesis mentioned is not valid and there are in fact developers wanting to style them differently.

I am not seeing the issue of allowing someone to style the Select root without styling the TextField root. Why would that be a concern?

@israelKusayev
Copy link

I strongly think that it's a very common use case to style them differently

@hbjORbj are there any updates on it?

@hbjORbj
Copy link
Member

hbjORbj commented Apr 29, 2022

I will add "waiting for upvotes" label to see the popularity of the opinion.

@hbjORbj hbjORbj added the waiting for 👍 Waiting for upvotes label Apr 29, 2022
@maciej-baruch
Copy link

maciej-baruch commented Nov 9, 2022

@rathpc The root styleOverride still works in MuiSelect if you need to use it.

@hobadams
Copy link

Hey @hbjORbj, any update on this. I'd really love to just target select elements.

@shinitakunai
Copy link

shinitakunai commented Feb 9, 2023

In order to style the border, you need to target root and not just the select class and many other cases, so I agree with others' opinions to allow targeting the root

@david-skimli
Copy link

In Typescript you only has to set the type of styleOverrides to any:

    MuiSelect: {
      styleOverrides: {
        root: {
          fontSize: '15px',
          color: '#555555',
          borderRadius: '8px',
          borderColor: '#CCCCCC',
        },
        select: {
          padding: '8px 32px 6px 12px'
        }
      } as any
    }

@alexerisov
Copy link

alexerisov commented Apr 5, 2023

In Typescript you only has to set the type of styleOverrides to any:

    MuiSelect: {
      styleOverrides: {
        root: {
          fontSize: '15px',
          color: '#555555',
          borderRadius: '8px',
          borderColor: '#CCCCCC',
        },
        select: {
          padding: '8px 32px 6px 12px'
        }
      } as any
    }

"any" leads to losing type completion in styleOverrides block. You can use this boilerplate:

import type { Components, ComponentsOverrides, Theme } from "@mui/material";
import { OverridesStyleRules } from "@mui/material/styles/overrides";

type OverridesWithRoot = ComponentsOverrides<Theme>["MuiSelect"] & {
  root: OverridesStyleRules<"root", "MuiSelect", Theme>["root"];
};

export const MuiSelect: Components<Theme>["MuiSelect"] = {
  defaultProps: {
    onClose: () => {
      setTimeout(() => {
        (document.activeElement as HTMLElement).blur();
      }, 0);
    }
  },
  styleOverrides: {
    select: ({ ownerState, theme }) =>
      theme.unstable_sx({
        py: 2,
        fontSize: 14,
        border: "none !important",
        borderRadius: 2.5
      }),
    root: ({ ownerState, theme }) =>
      theme.unstable_sx({
        ".MuiOutlinedInput-notchedOutline": { border: 0 }
      })
  } as OverridesWithRoot
};

@DiegoAndai
Copy link
Member

DiegoAndai commented Jan 8, 2025

I stumbled upon this while going through our most upvoted issues.

Turns out we fixed this (#44928) without knowing about #29593. I suspected something weird was going on: #42975 (review).

We either:

  1. Keep the root class and close this issue
  2. Revert [material-ui][Select] Add missing root class #44928, remove it from the docs, add a test explaining why it shouldn't be added

We should go with option 1, IMO. I disagree with the following:

I suspect we shouldn't [...] allow developers to customize the Select independently of the Input

(From #29342 (comment))

29 upvotes on this issue also disagree with this. This is on page 2 of our most upvoted issues.

As additional info, the Autocomplete component does have a root class, but it's not the same as its input root.

As a post-mortem, the reason we fixed it without noticing it was removed on purpose was:

What option should we follow @aarongarciah?

cc: @oliviertassinari

@aarongarciah
Copy link
Member

@DiegoAndai I don't have a strong opinion and I'm not fully familiarized with how we handle this in all components, but looks like option 1 is what users want. If this is consistent with how we handle it in other components, no objections from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests