Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f

- Updated `MediaCard` to accept ReactNode as title and make `primaryAction` optional ([#3552](https://github.com/Shopify/polaris-react/pull/3552))

- Optimized `ThemeProvider` to only output its custom properties in nested `ThemeProvider`s when they differ from the parent context ([#3550](https://github.com/Shopify/polaris-react/pull/3550))

### Bug fixes

### Documentation
Expand Down
58 changes: 39 additions & 19 deletions src/components/ThemeProvider/ThemeProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ThemeConfig,
buildThemeContext,
buildCustomProperties,
toString,
Tokens,
} from '../../utilities/theme';
import {useFeatures} from '../../utilities/features';
Expand Down Expand Up @@ -38,15 +39,16 @@ export function ThemeProvider({

const parentContext = useContext(ThemeContext);
const isParentThemeProvider = parentContext === undefined;
const processedThemeConfig = useMemo(() => {
const parentColorScheme =
parentContext && parentContext.colorScheme && parentContext.colorScheme;
const parentColors =
parentContext && parentContext.colors && parentContext.colors;

const parentColorScheme =
parentContext && parentContext.colorScheme && parentContext.colorScheme;
const parentColors =
parentContext && parentContext.colors && parentContext.colors;

const [customProperties, theme] = useMemo(() => {
const {colors, colorScheme, ...rest} = themeConfig;

return {
const processedThemeConfig = {
...rest,
...{colorScheme: getColorScheme(colorScheme, parentColorScheme)},
colors: {
Expand All @@ -55,22 +57,29 @@ export function ThemeProvider({
...colors,
},
};
}, [parentContext, themeConfig, isParentThemeProvider]);

const customProperties = useMemo(
() =>
buildCustomProperties(processedThemeConfig, newDesignLanguage, Tokens),
[processedThemeConfig, newDesignLanguage],
);
const customProperties = buildCustomProperties(
processedThemeConfig,
newDesignLanguage,
Tokens,
);

const theme = useMemo(
() =>
buildThemeContext(
const theme = {
...buildThemeContext(
processedThemeConfig,
newDesignLanguage ? customProperties : undefined,
),
[customProperties, processedThemeConfig, newDesignLanguage],
);
textColor: customProperties['--p-text'] || '',
};

return [customProperties, theme];
}, [
isParentThemeProvider,
newDesignLanguage,
parentColorScheme,
parentColors,
themeConfig,
]);

// We want these values to be empty string instead of `undefined` when not set.
// Otherwise, setting a style property to `undefined` does not remove it from the DOM.
Expand All @@ -84,10 +93,21 @@ export function ThemeProvider({
}
}, [backgroundColor, color, isParentThemeProvider]);

const style = {...customProperties, ...(!isParentThemeProvider && {color})};
let style;

if (isParentThemeProvider) {
style = customProperties;
} else if (
!isParentThemeProvider &&
parentContext!.cssCustomProperties !== toString(customProperties)
) {
style = {...customProperties, ...{color}};
} else {
style = {color};
}

return (
<ThemeContext.Provider value={{...theme, textColor: color}}>
<ThemeContext.Provider value={theme}>
<div style={style}>{children}</div>
</ThemeContext.Provider>
);
Expand Down
6 changes: 3 additions & 3 deletions src/components/ThemeProvider/tests/ThemeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ describe('<ThemeProvider />', () => {
});

describe('when nested', () => {
it('sets a default theme', () => {
it('does not render custom properties if themes are identical', () => {
const themeProvider = mountWithNewDesignLanguage(
<ThemeProvider theme={{}}>
<ThemeProvider theme={{}}>
Expand All @@ -200,7 +200,7 @@ describe('<ThemeProvider />', () => {
);

expect(themeProvider.findAll('div')[1]).toHaveReactProps({
style: expect.objectContaining({
style: expect.not.objectContaining({
'--p-background': expect.any(String),
'--p-text': expect.any(String),
'--p-interactive': expect.any(String),
Expand Down Expand Up @@ -234,7 +234,7 @@ describe('<ThemeProvider />', () => {
);

expect(themeProvider.findAll('div')[1]).toHaveReactProps({
style: expect.objectContaining({
style: expect.not.objectContaining({
'--p-surface': 'rgba(255, 255, 255, 1)',
}),
});
Expand Down
1 change: 1 addition & 0 deletions src/utilities/theme/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {
buildCustomProperties,
buildThemeContext,
toCssCustomPropertySyntax,
toString,
} from './utils';

export * from './tokens';
2 changes: 1 addition & 1 deletion src/utilities/theme/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function buildThemeContext(
};
}

function toString(obj?: CustomPropertiesLike) {
export function toString(obj?: CustomPropertiesLike) {
if (obj) {
return Object.entries(obj)
.map((pair) => pair.join(':'))
Expand Down