The "getSelector" API feels strange

[oliviertassinari]

Steps to reproduce

I'm confused by this getSelector API:

1. What does the name mean? The name feels obscure. How about we rename it to something more explicit? Looking at the source, it seems to be close to a getColorSchemeSelector so maybe the name should be merged. Now, I imagine we shouldn't confuse it with the helper developer would use to write conditional logic, e.g. theme.applyStyles('dark', {}).

https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/packages/pigment-css-react/src/utils/extendTheme.ts#L20-L35

https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/packages/pigment-css-react/src/utils/extendTheme.ts#L150-L155

2. I don't resonate with the API shape. I would expect 90% of the people not to use the media query approach. I mean, I would personally always make this selectors-based, media query is for the reallllllly quick and dirty websites, but has no purpose otherwise IMHO. If this is how people behave, it means that most people will need to reimplement their own custom selectors like getSelector: (colorScheme) => colorScheme ? `.theme-${colorScheme}` : ':root',. So, wouldn't it be better to match Tailwind CSS API? https://tailwindcss.com/docs/dark-mode#toggling-dark-mode-manually. I would propose something like this:

interface PigmentConfig {
  /**
   * @default media
   */
  colorScheme: 'selector' | 'media';
  /**
   * @default `getSelector: (colorScheme) => colorScheme ? `:where(.theme-${colorScheme})` : ':root',`
   */
  getColorSchemeSelector: (colorScheme: string) => string;
}

Context

As a side note:

• [ ] a. It looks like this whole function is dead code, to remove, no?

https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/packages/pigment-css-react/src/utils/extendTheme.ts#L104

• [ ] b. The apps are wrong, no? The return type is an object:

https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/apps/pigment-css-next-app/next.config.js#L84

but the expected type is a string: https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/packages/pigment-css-react/src/utils/extendTheme.ts#L154

• [ ] c. The docs look wrong to me: it should be :where() otherwise, style will override :hover, :focus-visible, etc styles.

https://github.com/mui/pigment-css/blob/e672e4b4ee36bb135d6d50fd805d15376f13d1cb/README.md?plain=1#L667

This makes the case for why we need 1.

Your environment

npx @mui/envinfo

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

Search keywords: -

[siriwatknp]

Absolutely, it can follow Tailwind API. Will change the default selector to use class approach.

As a note: Tailwind default behavior is media.

[oliviertassinari]

Tailwind default behavior is media.

I think this is great, I would advocate to do the same so it works in the configuration-less mode.