Skip to content

Commit 013533d

Browse files
committed
Moved ThemeProvider updates into effect
#1669
1 parent 6fa2204 commit 013533d

5 files changed

Lines changed: 23 additions & 41 deletions

File tree

packages/chart/src/ChartThemeProvider.tsx

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createContext, ReactNode, useEffect, useState } from 'react';
1+
import { createContext, ReactNode, useMemo } from 'react';
22
import { useTheme } from '@deephaven/components';
33
import defaultChartTheme, { ChartTheme } from './ChartTheme';
44

@@ -20,20 +20,10 @@ export function ChartThemeProvider({
2020
}: ChartThemeProviderProps): JSX.Element {
2121
const { activeThemes } = useTheme();
2222

23-
const [chartTheme, setChartTheme] = useState<ChartTheme | null>(null);
24-
25-
// The `ThemeProvider` that supplies `activeThemes` also provides the corresponding
26-
// CSS theme variables to the DOM by dynamically rendering <style> tags whenever
27-
// the `activeThemes` change. Painting the latest CSS variables to the DOM may
28-
// not happen until after `ChartThemeProvider` is rendered, but they should be
29-
// available by the time the effect runs. Therefore, it is important to derive
30-
// the chart theme in an effect instead of deriving in a `useMemo` to ensure
31-
// we have the latest CSS variables.
32-
useEffect(() => {
33-
if (activeThemes != null) {
34-
setChartTheme(defaultChartTheme());
35-
}
36-
}, [activeThemes]);
23+
const chartTheme = useMemo(
24+
() => (activeThemes == null ? null : defaultChartTheme()),
25+
[activeThemes]
26+
);
3727

3828
return (
3929
<ChartThemeContext.Provider value={chartTheme}>

packages/chart/src/MockChartModel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class MockChartModel extends ChartModel {
2020

2121
static smoothing = 1.5;
2222

23-
static _theme: ChartTheme;
23+
static _theme: ChartTheme | null;
2424

2525
static get theme(): ChartTheme {
2626
/* eslint-disable no-underscore-dangle */

packages/components/src/RandomAreaPlotAnimation.tsx

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable react-hooks/exhaustive-deps */
22
/* eslint-disable react/display-name */
33

4-
import React, { useEffect, useState, useRef } from 'react';
4+
import React, { useEffect, useState, useRef, useMemo } from 'react';
55
import debounce from 'lodash.debounce';
66
import { assertNotNull } from '@deephaven/utils';
77
import './RandomAreaPlotAnimation.scss';
@@ -42,14 +42,11 @@ function getRandomAreaPlotAnimationThemeColors(): ThemeColors {
4242
const RandomAreaPlotAnimation = React.memo(() => {
4343
const { activeThemes } = useTheme();
4444

45-
const [themeColors, setThemeColors] = useState<ThemeColors | null>(null);
46-
47-
// Resolving css variables has to run in `useEffect` or `useLayoutEffect` so
48-
// that we know React has updated the DOM with any styles set by the
49-
// ThemeProvider.
50-
useEffect(() => {
51-
setThemeColors(getRandomAreaPlotAnimationThemeColors());
52-
}, [activeThemes]);
45+
const themeColors = useMemo(
46+
() =>
47+
activeThemes == null ? null : getRandomAreaPlotAnimationThemeColors(),
48+
[activeThemes]
49+
);
5350

5451
const canvas = useRef<HTMLCanvasElement>(null);
5552
const container = useRef<HTMLDivElement>(null);

packages/components/src/theme/ThemeProvider.tsx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ export interface ThemeProviderProps {
3636
export function ThemeProvider({
3737
themes: customThemes,
3838
children,
39-
}: ThemeProviderProps): JSX.Element {
39+
}: ThemeProviderProps): JSX.Element | null {
4040
const baseThemes = useMemo(() => getDefaultBaseThemes(), []);
4141

42+
const [value, setValue] = useState<ThemeContextValue | null>(null);
43+
4244
const [selectedThemeKey, setSelectedThemeKey] = useState<string>(
4345
() => getThemePreloadData()?.themeKey ?? DEFAULT_DARK_THEME_KEY
4446
);
@@ -88,17 +90,16 @@ export function ThemeProvider({
8890
[activeThemes, selectedThemeKey, customThemes]
8991
);
9092

91-
const value = useMemo(
92-
() => ({
93+
useEffect(() => {
94+
setValue({
9395
activeThemes,
9496
selectedThemeKey,
9597
themes,
9698
setSelectedThemeKey,
97-
}),
98-
[activeThemes, selectedThemeKey, themes]
99-
);
99+
});
100+
}, [activeThemes, selectedThemeKey, themes]);
100101

101-
return (
102+
return value == null ? null : (
102103
<ThemeContext.Provider value={value}>
103104
{activeThemes == null ? null : (
104105
<>

packages/iris-grid/src/IrisGridThemeProvider.tsx

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useTheme } from '@deephaven/components';
2-
import { createContext, ReactNode, useEffect, useState } from 'react';
2+
import { createContext, ReactNode, useMemo } from 'react';
33
import { createDefaultIrisGridTheme, IrisGridThemeType } from './IrisGridTheme';
44

55
export type IrisGridThemeContextValue = Partial<IrisGridThemeType>;
@@ -16,14 +16,8 @@ export function IrisGridThemeProvider({
1616
}: IrisGridThemeProviderProps): JSX.Element {
1717
const { activeThemes } = useTheme();
1818

19-
const [gridTheme, setGridTheme] = useState<IrisGridThemeContextValue>({});
20-
21-
useEffect(
22-
function refreshIrisGridTheme() {
23-
if (activeThemes != null) {
24-
setGridTheme(createDefaultIrisGridTheme());
25-
}
26-
},
19+
const gridTheme = useMemo(
20+
() => (activeThemes == null ? null : createDefaultIrisGridTheme()),
2721
[activeThemes]
2822
);
2923

0 commit comments

Comments
 (0)