Split out getDefaultSelectedThemeKey (#2203)

deephaven · Aug 29, 2024 · 6dd4af7 · 6dd4af7
1 parent 837e5be
commit 6dd4af7
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 228 deletions.
diff --git a/packages/components/src/theme/ThemeProvider.test.tsx b/packages/components/src/theme/ThemeProvider.test.tsx
@@ -2,18 +2,12 @@ import React from 'react';
 import { act, render } from '@testing-library/react';
 import { assertNotNull, TestUtils } from '@deephaven/utils';
 import { ThemeContextValue, ThemeProvider } from './ThemeProvider';
-import {
- DEFAULT_DARK_THEME_KEY,
- DEFAULT_LIGHT_THEME_KEY,
- ThemeData,
- ThemePreloadData,
-} from './ThemeModel';
+import { DEFAULT_LIGHT_THEME_KEY, ThemeData } from './ThemeModel';
 import {
  calculatePreloadStyleContent,
  getActiveThemes,
  getDefaultBaseThemes,
- getThemeKeyOverride,
- getThemePreloadData,
+ getDefaultSelectedThemeKey,
  setThemePreloadData,
 } from './ThemeUtils';
 import { useTheme } from './useTheme';
@@ -25,17 +19,17 @@ jest.mock('./ThemeUtils', () => {
  return {
  ...actual,
  calculatePreloadStyleContent: jest.fn(),
+ getDefaultSelectedThemeKey: jest.fn(),
  getThemeKeyOverride: jest.fn(),
- getThemePreloadData: jest.fn(actual.getThemePreloadData),
  setThemePreloadData: jest.fn(),
  };
 });
 
 const customThemes = [
  { themeKey: 'themeA' },
- { themeKey: 'themeOverrideKey' },
+ { themeKey: 'mockDefaultSelectedThemeKey' },
 ] as ThemeData[];
-const preloadA: ThemePreloadData = { themeKey: 'themeA' };
+const defaultSelectedThemeKey = 'mockDefaultSelectedThemeKey';
 
 beforeEach(() => {
  jest.clearAllMocks();
@@ -46,11 +40,10 @@ beforeEach(() => {
  .mockName('calculatePreloadStyleContent')
  .mockReturnValue(':root{mock-preload-content}');
 
- asMock(getThemeKeyOverride)
- .mockName('getThemeKeyOverride')
- .mockReturnValue(null);
+ asMock(getDefaultSelectedThemeKey)
+ .mockName('getDefaultSelectedThemeKey')
+ .mockReturnValue(defaultSelectedThemeKey);
 
- asMock(getThemePreloadData).mockName('getThemePreloadData');
  asMock(setThemePreloadData).mockName('setThemePreloadData');
 });
 
@@ -66,30 +59,9 @@ describe('ThemeProvider', () => {
  themeContextValueRef.current = null;
  });
 
- it.each([
- [null, null, null],
- [null, preloadA, null],
- [customThemes, null, null],
- [customThemes, preloadA, null],
- [null, null, 'themeOverrideKey'],
- [null, preloadA, 'themeOverrideKey'],
- [customThemes, null, 'themeOverrideKey'],
- [customThemes, preloadA, 'themeOverrideKey'],
- ] as const)(
- 'should load themes based on override, preload data or default: %o, %s, %s',
- (themes, preloadData, overrideKey) => {
- asMock(getThemeKeyOverride).mockReturnValue(overrideKey);
-
- if (overrideKey == null) {
- asMock(getThemePreloadData).mockReturnValue(preloadData);
- } else {
- asMock(getThemePreloadData).mockImplementation(() => {
- throw new Error(
- 'getThemePreloadData should not be called when overrideKey is set'
- );
- });
- }
-
+ it.each([null, customThemes])(
+ 'should load themes based on default selected theme key: %o',
+ themes => {
  const component = render(
  <ThemeProvider themes={themes}>
  <MockChild />
@@ -102,34 +74,24 @@ describe('ThemeProvider', () => {
  expect(themeContextValueRef.current.activeThemes).toBeNull();
  } else {
  expect(themeContextValueRef.current.activeThemes).toEqual(
- getActiveThemes(
- overrideKey ?? preloadData?.themeKey ?? DEFAULT_DARK_THEME_KEY,
- {
- base: getDefaultBaseThemes(),
- custom: themes,
- }
- )
+ getActiveThemes(defaultSelectedThemeKey, {
+ base: getDefaultBaseThemes(),
+ custom: themes,
+ })
  );
 
  expect(themeContextValueRef.current.selectedThemeKey).toEqual(
- overrideKey ?? preloadData?.themeKey ?? DEFAULT_DARK_THEME_KEY
+ defaultSelectedThemeKey
  );
  }
 
  expect(component.baseElement).toMatchSnapshot();
  }
  );
 
- it.each([
- [null, null],
- [null, preloadA],
- [customThemes, null],
- [customThemes, preloadA],
- ] as const)(
- 'should set preload data when active themes change: %o, %s',
- (themes, preloadData) => {
- asMock(getThemePreloadData).mockReturnValue(preloadData);
-
+ it.each([null, customThemes] as const)(
+ 'should set preload data when active themes change: %o',
+ themes => {
  render(
  <ThemeProvider themes={themes}>
  <MockChild />
@@ -140,7 +102,7 @@ describe('ThemeProvider', () => {
  expect(setThemePreloadData).not.toHaveBeenCalled();
  } else {
  expect(setThemePreloadData).toHaveBeenCalledWith({
- themeKey: preloadData?.themeKey ?? DEFAULT_DARK_THEME_KEY,
+ themeKey: defaultSelectedThemeKey,
  preloadStyleContent: calculatePreloadStyleContent({}),
  });
  }

diff --git a/packages/components/src/theme/ThemeProvider.tsx b/packages/components/src/theme/ThemeProvider.tsx
@@ -1,18 +1,13 @@
 import { createContext, ReactNode, useEffect, useMemo, useState } from 'react';
 import Log from '@deephaven/log';
-import {
- DEFAULT_DARK_THEME_KEY,
- DEFAULT_PRELOAD_DATA_VARIABLES,
- ThemeData,
-} from './ThemeModel';
+import { DEFAULT_PRELOAD_DATA_VARIABLES, ThemeData } from './ThemeModel';
 import {
  calculatePreloadStyleContent,
  getActiveThemes,
  getDefaultBaseThemes,
- getThemePreloadData,
  setThemePreloadData,
  overrideSVGFillColors,
- getThemeKeyOverride,
+ getDefaultSelectedThemeKey,
 } from './ThemeUtils';
 import { SpectrumThemeProvider } from './SpectrumThemeProvider';
 import './theme-svg.scss';
@@ -48,13 +43,8 @@ export function ThemeProvider({
 
  const [value, setValue] = useState<ThemeContextValue | null>(null);
 
- const themeKeyOverride = useMemo(() => getThemeKeyOverride(), []);
-
  const [selectedThemeKey, setSelectedThemeKey] = useState<string>(
- () =>
- themeKeyOverride ??
- getThemePreloadData()?.themeKey ??
- DEFAULT_DARK_THEME_KEY
+ getDefaultSelectedThemeKey
  );
 
  // Calculate active themes once a non-null themes array is provided.

diff --git a/packages/components/src/theme/ThemeUtils.ts b/packages/components/src/theme/ThemeUtils.ts
@@ -179,6 +179,21 @@ export function getDefaultBaseThemes(): ThemeData[] {
  ];
 }
 
+/**
+ * Get the default selected theme key. Precedence is:
+ * 1. Theme key override query parameter
+ * 2. Theme key from preload data
+ * 3. Default dark theme key
+ * @returns The default selected theme key
+ */
+export function getDefaultSelectedThemeKey(): string {
+ return (
+ getThemeKeyOverride() ??
+ getThemePreloadData()?.themeKey ??
+ DEFAULT_DARK_THEME_KEY
+ );
+}
+
 /**
  * A theme key override can be set via a query parameter to force a specific
  * theme selection. Useful for embedded widget scenarios that don't expose the