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.

Sign up for GitHub

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

Jump to bottom

[material-ui] Fix createTheme with just color schemes #43518

Merged
merged 6 commits into from
Sep 2, 2024
Merged

[material-ui] Fix createTheme with just color schemes #43518

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cbe9a14
wip
siriwatknp Aug 29, 2024
c56577b
fix color schemes without palette
siriwatknp Aug 29, 2024
3480b49
Merge branch 'master' of https://github.com/mui/material-ui into fix/…
siriwatknp Aug 29, 2024
414a82a
Update packages/mui-material/src/styles/createTheme.ts
siriwatknp Aug 30, 2024
e61a0bd
add more tests
siriwatknp Aug 30, 2024
3df87c5
add ts-ignore
siriwatknp Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion packages/mui-material/src/styles/createTheme.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { createRenderer } from '@mui/internal-test-utils';
import Button from '@mui/material/Button';
import Box from '@mui/material/Box';
import { ThemeProvider, createTheme, styled } from '@mui/material/styles';
import { deepOrange, green } from '@mui/material/colors';
import { deepOrange, green, grey } from '@mui/material/colors';
import createPalette from './createPalette';

const lightPalette = createPalette({ mode: 'light' });
Expand Down Expand Up @@ -56,6 +56,68 @@ describe('createTheme', () => {
expect(theme.palette.secondary.main).to.equal(green.A400);
});

it('should be customizable through `colorSchemes` node', () => {
const theme = createTheme({
colorSchemes: {
dark: {
palette: {
background: {
default: grey[900],
},
},
},
light: {
palette: {
background: {
default: grey[50],
},
bg: {
main: grey[800],
dark: grey[700],
},
},
},
},
});
expect(theme.colorSchemes.dark.palette.background.default).to.equal(grey[900]);
expect(theme.colorSchemes.light.palette.background.default).to.equal(grey[50]);
expect(theme.colorSchemes.light.palette.bg.main).to.equal(grey[800]);
expect(theme.colorSchemes.light.palette.bg.dark).to.equal(grey[700]);
expect(theme.palette.mode).to.equal('light');
expect(theme.palette.background.default).to.equal(grey[50]);
});

it('should be customizable through `colorSchemes` node with non-existing fields', () => {
const theme = createTheme({
colorSchemes: {
dark: {
opacity: {
disabled: 0.38,
},
palette: {
gradient: 'linear-gradient(90deg, #000000 0%, #ffffff 100%)',
},
},
light: {
opacity: {
disabled: 0.5,
},
palette: {
gradient: 'linear-gradient(90deg, #ffffff 0%, #000000 100%)',
},
},
},
});
expect(theme.colorSchemes.dark.opacity.disabled).to.equal(0.38);
expect(theme.colorSchemes.light.opacity.disabled).to.equal(0.5);
expect(theme.colorSchemes.dark.palette.gradient).to.equal(
'linear-gradient(90deg, #000000 0%, #ffffff 100%)',
);
expect(theme.colorSchemes.light.palette.gradient).to.equal(
'linear-gradient(90deg, #ffffff 0%, #000000 100%)',
);
});

describe('CSS variables', () => {
it('should have default light with media selector if no `palette` and colorSchemes.dark is provided ', () => {
const theme = createTheme({
Expand Down
37 changes: 29 additions & 8 deletions packages/mui-material/src/styles/createTheme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function attachColorScheme(
theme.colorSchemes[scheme] = {
...(colorScheme !== true && colorScheme),
palette: createPalette({
...(colorScheme === true ? {} : colorScheme),
...(colorScheme === true ? {} : colorScheme.palette),
mode: scheme,
} as any), // cast type to skip module augmentation test
};
Expand Down Expand Up @@ -72,24 +72,45 @@ export default function createTheme(
};

if (cssVariables === false) {
const theme = createThemeNoVars(options as ThemeOptions, ...args) as unknown as Theme & {
if (!('colorSchemes' in options)) {
// v5 signature
siriwatknp marked this conversation as resolved.
Show resolved Hide resolved
return createThemeNoVars(options as ThemeOptions, ...args);
}

let paletteOptions;
if (!('palette' in options)) {
if (colorSchemesInput[defaultColorSchemeInput]) {
if (colorSchemesInput[defaultColorSchemeInput] !== true) {
paletteOptions = colorSchemesInput[defaultColorSchemeInput].palette;
} else if (defaultColorSchemeInput === 'dark') {
paletteOptions = { mode: 'dark' };
}
}
}

const theme = createThemeNoVars(
{ ...options, palette: paletteOptions } as ThemeOptions,
...args,
) as unknown as Theme & {
defaultColorScheme?: 'light' | 'dark';
colorSchemes?: Partial<Record<string, any>>;
};

if (!('colorSchemes' in options)) {
return theme;
}

theme.defaultColorScheme = defaultColorSchemeInput;
theme.colorSchemes = colorSchemesInput as Record<string, ColorSystem>;

if (theme.palette.mode === 'light') {
theme.colorSchemes.light = { palette: theme.palette } as ColorSystem;
theme.colorSchemes.light = {
...(colorSchemesInput.light !== true && colorSchemesInput.light),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this covered by line 100 above?

Copy link
Member Author

@siriwatknp siriwatknp Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this line, the other fields that are not palette will be overridden because of the reassigning theme.colorSchemes.light.

palette: theme.palette,
} as ColorSystem;
attachColorScheme(theme, 'dark', colorSchemesInput.dark);
}
if (theme.palette.mode === 'dark') {
theme.colorSchemes.dark = { palette: theme.palette } as ColorSystem;
theme.colorSchemes.dark = {
...(colorSchemesInput.dark !== true && colorSchemesInput.dark),
palette: theme.palette,
} as ColorSystem;
attachColorScheme(theme, 'light', colorSchemesInput.light);
}

Expand Down
Loading