[Material] [CssVars] --AppBar-color is invalid when dark color scheme is deleted

[mwskwong]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Link to live example:

Current behavior 😯

Overriding the defaultBg renders no effect because the computed --AppBar-color is invalid after dark color scheme is deleted

const theme = extendTheme({
  colorSchemes: {
    light: {
      AppBar: {
        defaultBg: "#ff0000"
      }
    }
  }
});

Expected behavior 🤔

This should set the default AppBar color to red.

const theme = extendTheme({
  colorSchemes: {
    light: {
      AppBar: {
        defaultBg: "#ff0000"
      }
    }
  }
});

Context 🔦

Workaround
Applying the color in the MuiAppBar.styleOverrides.

Your environment 🌎

npx @mui/envinfo

  System:
    OS: Windows 10 10.0.19044
  Binaries:
    Node: 16.17.1 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 8.15.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 110.0.5481.77
    Edge: Spartan (44.19041.1266.0), Chromium (109.0.1518.78)
  npmPackages:
    @emotion/react: ^11.10.5 => 11.10.5
    @emotion/styled: ^11.10.5 => 11.10.5
    @mui/base:  5.0.0-alpha.117
    @mui/core-downloads-tracker:  5.11.8
    @mui/icons-material: ^5.11.0 => 5.11.0
    @mui/lab: 5.0.0-alpha.119 => 5.0.0-alpha.119
    @mui/material: ^5.11.8 => 5.11.8
    @mui/private-theming:  5.11.7
    @mui/styled-engine:  5.11.8
    @mui/system:  5.11.8
    @mui/types:  7.2.3
    @mui/utils:  5.11.7
    @mui/x-date-pickers: ^5.0.18 => 5.0.18
    @types/react: ^18.0.27 => 18.0.27
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ^4.9.5 => 4.9.5

[siriwatknp]

It does not work because dark palette is removed in your sandbox which causes theme.vars.palette.AppBar.darkBg to be undefined.

If anyone wants to contribute, here is one way to fix it:

diff --git a/packages/mui-material/src/AppBar/AppBar.js b/packages/mui-material/src/AppBar/AppBar.js
index f68b30d817..c554d23751 100644
--- a/packages/mui-material/src/AppBar/AppBar.js
+++ b/packages/mui-material/src/AppBar/AppBar.js
@@ -20,7 +20,7 @@ const useUtilityClasses = (ownerState) => {
 
 // var2 is the fallback.
 // Ex. var1: 'var(--a)', var2: 'var(--b)'; return: 'var(--a, var(--b))'
-const joinVars = (var1, var2) => `${var1?.replace(')', '')}, ${var2})`;
+const joinVars = (var1, var2) => (var1 ? `${var1?.replace(')', '')}, ${var2})` : var2);
 
 const AppBarRoot = styled(Paper, {
   name: 'MuiAppBar',

@mwskwong You will also need to have palette as a key:

const theme = extendTheme({
  colorSchemes: {
    light: {
      palette: { // <<< without `palette`, it will not work even with the above fix.
        AppBar: {
          defaultBg: "red"
        }
      }
    }
  }
});

[donaldnevermore]

I'd like to take on this one. Just to discuss: can we Object.seal() the theme before extendTheme() returning it? In TS, deleting a non-optional property you'll get an error, but not in JS.

[mwskwong]

It does not work because dark palette is removed in your sandbox which causes theme.vars.palette.AppBar.darkBg to be undefined.

If anyone wants to contribute, here is one way to fix it:

diff --git a/packages/mui-material/src/AppBar/AppBar.js b/packages/mui-material/src/AppBar/AppBar.js
index f68b30d817..c554d23751 100644
--- a/packages/mui-material/src/AppBar/AppBar.js
+++ b/packages/mui-material/src/AppBar/AppBar.js
@@ -20,7 +20,7 @@ const useUtilityClasses = (ownerState) => {
 
 // var2 is the fallback.
 // Ex. var1: 'var(--a)', var2: 'var(--b)'; return: 'var(--a, var(--b))'
-const joinVars = (var1, var2) => `${var1?.replace(')', '')}, ${var2})`;
+const joinVars = (var1, var2) => (var1 ? `${var1?.replace(')', '')}, ${var2})` : var2);
 
 const AppBarRoot = styled(Paper, {
   name: 'MuiAppBar',

@mwskwong You will also need to have palette as a key:

const theme = extendTheme({
  colorSchemes: {
    light: {
      palette: { // <<< without `palette`, it will not work even with the above fix.
        AppBar: {
          defaultBg: "red"
        }
      }
    }
  }
});

Yeah, this is exactly what I was trying to say. I should have stated that explicitly in the OP to save for all the trouble of re-investigation.
And sorry for missing the palette, it is meant to be there.

[oliviertassinari]

Regarding the path: we have theme.vars.palette.AppBar.darkBg in Material UI. On Joy UI, we have CSS variables like this: --Switch-thumb-size. I wonder if it wouldn't make sense in the future, to move closer to: theme.vars.component.AppBar.darkBg. Maybe we could group all CSS variables related to the same component in the same path prefix. But maybe not, because it's simpler right now with the color schema switch logic.

[donaldnevermore]

BTW, from my point of view as a user, omitting the palette is more intuitive and concise.