Theme provider cleanup

UNRELEASED.md

@@ -22,6 +22,17 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Code quality
 
+- Improved code quality for the theme provider component ([#2225](https://github.com/Shopify/polaris-react/pull/2225)):
+
+  - updated type for `theme` prop to `ThemeConfig` to distinguish from the type `Theme` which is shared over context. A `Theme` contains only the logo properties, while `ThemeConfig` can contain a `colors` property.
+  - converted `ThemeProvider` to use hooks
+  - created symmetry in context between app provider and test provider
+  - added better tests for default topBar colors
+  - fixed an issue where `colorToHsla` returned HSLA strings instead of HSLA objects when given HSL or HSLA strings
+  - fixed an issue with `colorToHsla` where RGB colors with no saturation could result in a divide by zero error
+  - fixed an issue where `colorToHsla` inconsistently returned an alpha value
+  - fixed an issue where `lightenColor` and `darkenColor` would lighten or darken absolute lightness vales (0, 100)
+
 ### Deprecations
 
 ### Development workflow

src/components/AppProvider/AppProvider.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import {Theme} from '../../utilities/theme';
+import {ThemeConfig} from '../../utilities/theme';
 import {ThemeProvider} from '../ThemeProvider';
 import {MediaQueryProvider} from '../MediaQueryProvider';
 import {I18n, I18nContext, TranslationDictionary} from '../../utilities/i18n';
@@ -36,7 +36,7 @@ export interface AppProviderProps extends AppBridgeOptions {
   /** A custom component to use for all links used by Polaris components */
   linkComponent?: LinkLikeComponent;
   /** Custom logos and colors provided to select components */
-  theme?: Theme;
+  theme?: ThemeConfig;
   /** For toggling features */
   features?: Features;
   /** Inner content of the application */
@@ -98,7 +98,7 @@ export class AppProvider extends React.Component<AppProviderProps, State> {
   }
 
   render() {
-    const {theme = {logo: null}, features = {}, children} = this.props;
+    const {theme = {}, features = {}, children} = this.props;
     const {intl, appBridge, link} = this.state;
 
     return (

src/components/PolarisTestProvider/PolarisTestProvider.tsx

@@ -1,7 +1,11 @@
 import React from 'react';
 import {merge} from '../../utilities/merge';
 import {FrameContext} from '../../utilities/frame';
-import {Theme, ThemeContext} from '../../utilities/theme';
+import {
+  ThemeContext,
+  ThemeConfig,
+  buildThemeContext,
+} from '../../utilities/theme';
 import {MediaQueryContext} from '../../utilities/media-query';
 import {
   ScrollLockManager,
@@ -36,7 +40,7 @@ export type WithPolarisTestProviderOptions = {
   i18n?: TranslationDictionary | TranslationDictionary[];
   appBridge?: AppBridgeOptions;
   link?: LinkLikeComponent;
-  theme?: Partial<Theme>;
+  theme?: ThemeConfig;
   mediaQuery?: Partial<MediaQueryContextType>;
   features?: Features;
   // Contexts provided by Frame
@@ -59,7 +63,7 @@ export function PolarisTestProvider({
   i18n,
   appBridge,
   link,
-  theme,
+  theme = {},
   mediaQuery,
   features = {},
   frame,
@@ -78,7 +82,7 @@ export function PolarisTestProvider({
   // I'm not that worried about it
   const appBridgeApp = appBridge as React.ContextType<typeof AppBridgeContext>;
 
-  const mergedTheme = createThemeContext(theme);
+  const mergedTheme = buildThemeContext(theme);
 
   const mergedFrame = createFrameContext(frame);
 
@@ -113,11 +117,6 @@ export function PolarisTestProvider({
 
 function noop() {}
 
-function createThemeContext(theme: Partial<Theme> = {}): Theme {
-  const {logo = null} = theme;
-  return {logo};
-}
-
 function createFrameContext({
   showToast = noop,
   hideToast = noop,

src/components/ThemeProvider/ThemeProvider.tsx

@@ -1,77 +1,33 @@
-import React from 'react';
-import isEqual from 'lodash/isEqual';
-import {ThemeContext} from '../../utilities/theme';
-import {Theme} from '../../utilities/theme/types';
-import {setColors} from '../../utilities/theme/utils';
+import React, {useMemo} from 'react';
+import {
+  ThemeContext,
+  ThemeConfig,
+  buildThemeContext,
+  buildCustomProperties,
+} from '../../utilities/theme';
 import {themeProvider} from '../shared';
 
-interface State {
-  theme: Theme;
-  colors: string[][] | undefined;
-}
-
 interface ThemeProviderProps {
   /** Custom logos and colors provided to select components */
-  theme: Theme;
+  theme: ThemeConfig;
   /** The content to display */
   children?: React.ReactNode;
 }
 
-const defaultTheme = {
-  '--top-bar-background': '#00848e',
-  '--top-bar-color': '#f9fafb',
-  '--top-bar-background-lighter': '#1d9ba4',
-};
-
-export class ThemeProvider extends React.Component<ThemeProviderProps, State> {
-  state: State = {
-    theme: setThemeContext(this.props.theme),
-    colors: setColors(this.props.theme),
-  };
-
-  componentDidUpdate({theme: prevTheme}: ThemeProviderProps) {
-    const {theme} = this.props;
-    if (isEqual(prevTheme, theme)) {
-      return;
-    }
-
-    // eslint-disable-next-line react/no-did-update-set-state
-    this.setState({
-      theme: setThemeContext(theme),
-      colors: setColors(theme),
-    });
-  }
-
-  render() {
-    const {
-      theme: {logo = null, ...rest},
-    } = this.state;
-    const {children} = this.props;
-    const styles = this.createStyles() || defaultTheme;
-
-    const theme = {
-      ...rest,
-      logo,
-    };
-
-    return (
-      <ThemeContext.Provider value={theme}>
-        <div style={styles} {...themeProvider.props}>
-          {children}
-        </div>
-      </ThemeContext.Provider>
-    );
-  }
-
-  createStyles() {
-    const {colors} = this.state;
-    return colors
-      ? colors.reduce((state, [key, value]) => ({...state, [key]: value}), {})
-      : null;
-  }
-}
-
-function setThemeContext(ctx: Theme): Theme {
-  const {colors, ...theme} = ctx;
-  return {...theme};
+export function ThemeProvider({
+  theme: themeConfig,
+  children,
+}: ThemeProviderProps) {
+  const theme = useMemo(() => buildThemeContext(themeConfig), [themeConfig]);
+  const customProperties = useMemo(() => buildCustomProperties(themeConfig), [
+    themeConfig,
+  ]);
+
+  return (
+    <ThemeContext.Provider value={theme}>
+      <div style={customProperties} {...themeProvider.props}>
+        {children}
+      </div>
+    </ThemeContext.Provider>
+  );
 }

src/components/ThemeProvider/tests/ThemeProvider.test.tsx

@@ -6,7 +6,7 @@ import {ThemeContext} from '../../../utilities/theme';
 describe('<ThemeProvider />', () => {
   it('mounts', () => {
     const themeProvider = mountWithAppProvider(
-      <ThemeProvider theme={{logo: null}}>
+      <ThemeProvider theme={{logo: {}}}>
         <p>Hello</p>
       </ThemeProvider>,
     );
@@ -41,10 +41,7 @@ describe('<ThemeProvider />', () => {
       </ThemeProvider>,
     );
 
-    const div = wrapper
-      .find(Child)
-      .find('div')
-      .first();
+    const div = wrapper.find(Child).find('div');
 
     expect(div.exists()).toBe(true);
   });
@@ -56,7 +53,11 @@ describe('<ThemeProvider />', () => {
       </ThemeProvider>,
     );
 
-    expect(wrapper.find('div').props().style).toBeDefined();
+    expect(wrapper.find('div').props().style).toStrictEqual({
+      '--top-bar-background': '#00848e',
+      '--top-bar-background-lighter': '#f9fafb',
+      '--top-bar-color': '#1d9ba4',
+    });
   });
 
   it('sets a provided theme', () => {

src/utilities/color-manipulation.ts

@@ -1,3 +1,4 @@
+import {clamp} from '@shopify/javascript-utilities/math';
 import {HSLColor, HSBColor} from './color-types';
 
 export function lightenColor(color: HSLColor | string, lighten = 0) {
@@ -8,7 +9,7 @@ export function lightenColor(color: HSLColor | string, lighten = 0) {
   const {lightness} = color;
   const nextLightness = lightness + lighten;
 
-  return {...color, lightness: nextLightness};
+  return {...color, lightness: clamp(nextLightness, 0, 100)};
 }
 
 export function darkenColor(color: HSLColor | string, lighten = 0) {
@@ -19,7 +20,7 @@ export function darkenColor(color: HSLColor | string, lighten = 0) {
   const {lightness} = color;
   const nextLightness = lightness - lighten;
 
-  return {...color, lightness: nextLightness};
+  return {...color, lightness: clamp(nextLightness, 0, 100)};
 }
 
 export function saturateColor(

src/utilities/color-transformers.ts

@@ -147,10 +147,11 @@ function rgbToHsbl(color: RGBAColor, type: 'b' | 'l' = 'b'): HSBLAColor {
   } else if (type === 'b') {
     saturation = delta / largestComponent;
   } else if (type === 'l') {
-    saturation =
+    const baseSaturation =
       lightness > 0.5
         ? delta / (2 - largestComponent - smallestComponent)
         : delta / (largestComponent + smallestComponent);
+    saturation = isNaN(baseSaturation) ? 0 : baseSaturation;
   }
 
   let huePercentage = 0;
@@ -177,20 +178,18 @@ function rgbToHsbl(color: RGBAColor, type: 'b' | 'l' = 'b'): HSBLAColor {
 }
 
 export function rgbToHsb(color: RGBColor): HSBColor;
-export function rgbToHsb(color: RGBAColor): HSBAColor;
 export function rgbToHsb(color: RGBAColor): HSBAColor {
-  const {hue, saturation, brightness, alpha} = rgbToHsbl(color, 'b');
+  const {hue, saturation, brightness, alpha = 1} = rgbToHsbl(color, 'b');
   return {hue, saturation, brightness, alpha};
 }
 
-export function rgbToHsl(color: RGBColor): HSLColor;
-export function rgbToHsl(color: RGBAColor): HSLAColor;
+export function rgbToHsl(color: RGBColor): HSLAColor;
 export function rgbToHsl(color: RGBAColor): HSLAColor {
   const {
     hue,
     saturation: rawSaturation,
     lightness: rawLightness,
-    alpha,
+    alpha = 1,
   } = rgbToHsbl(color, 'l');
   const saturation = rawSaturation * 100;
   const lightness = rawLightness * 100;
@@ -253,7 +252,7 @@ export function hslToString(hslColor: HSLAColor | string) {
   return `hsl(${hue}, ${saturation}%, ${lightness}%, ${alpha})`;
 }
 
-function rgbToObject(color: string) {
+function rgbToObject(color: string): RGBAColor {
   const colorMatch = color.match(/\(([^)]+)\)/);
 
   if (!colorMatch) {
@@ -270,27 +269,48 @@ function rgbToObject(color: string) {
   return objColor;
 }
 
-const hexToHsl: (color: string) => HSLColor | HSLAColor = compose(
+const hexToHsla: (color: string) => HSLAColor = compose(
   rgbToHsl,
   hexToRgb,
 );
 
-const rbgStringToHsl: (color: string) => HSLColor | HSLAColor = compose(
+const rbgStringToHsla: (color: string) => HSLAColor = compose(
   rgbToHsl,
   rgbToObject,
 );
 
-export function colorToHsla(color: string) {
+function hslToObject(color: string): HSLAColor {
+  const colorMatch = color.match(/\(([^)]+)\)/);
+
+  if (!colorMatch) {
+    return {hue: 0, saturation: 0, lightness: 0, alpha: 0};
+  }
+
+  const [hue, saturation, lightness, alpha] = colorMatch[1].split(',');
+  const objColor = {
+    hue: parseInt(hue, 10),
+    saturation: parseInt(saturation, 10),
+    lightness: parseInt(lightness, 10),
+    alpha: parseFloat(alpha) || 1,
+  };
+  return objColor;
+}
+
+export function colorToHsla(color: string): HSLAColor {
   const type: ColorType = getColorType(color);
   switch (type) {
     case ColorType.Hex:
-      return hexToHsl(color);
+      return hexToHsla(color);
     case ColorType.Rgb:
     case ColorType.Rgba:
-      return rbgStringToHsl(color);
-    case ColorType.Hsl:
+      return rbgStringToHsla(color);
     case ColorType.Hsla:
+    case ColorType.Hsl:
+      return hslToObject(color);
+    case ColorType.Default:
     default:
-      return color;
+      throw new Error(
+        'Accepted color formats are: hex, rgb, rgba, hsl and hsla',
+      );
   }
 }

src/utilities/tests/color-manipulation.test.ts

@@ -20,6 +20,11 @@ describe('lightenColor', () => {
     );
     expect((lightColor as HSLColor).lightness).toBeGreaterThan(50);
   });
+
+  it('returns a valid color when an input is at maximum lightness', () => {
+    const lightColor = lightenColor({hue: 0, saturation: 0, lightness: 100}, 5);
+    expect((lightColor as HSLColor).lightness).toBe(100);
+  });
 });
 
 describe('darkenColor', () => {
@@ -32,6 +37,11 @@ describe('darkenColor', () => {
     const darkColor = darkenColor({hue: 50, saturation: 50, lightness: 50}, 5);
     expect((darkColor as HSLColor).lightness).toBeLessThan(50);
   });
+
+  it('returns a valid color when an input is at maximum lightness', () => {
+    const darkColor = darkenColor({hue: 0, saturation: 0, lightness: 0}, 5);
+    expect((darkColor as HSLColor).lightness).toBe(0);
+  });
 });
 
 describe('saturateColor', () => {