Skip to content

Commit

Permalink
feat: Theming - Moved ThemeProvider updates into effect (#1682)
Browse files Browse the repository at this point in the history
Moved ThemeProvider updates into effect
resolves #1669
  • Loading branch information
bmingles authored Dec 13, 2023
1 parent b2972f0 commit a09bdca
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 62 deletions.
2 changes: 1 addition & 1 deletion packages/auth-plugins/src/LoginForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function LoginForm({
>
{isLoggingIn && (
<span>
<LoadingSpinner className="loading-spinner-vertical-align" />
<LoadingSpinner className="mr-2 loading-spinner-vertical-align" />
<span className="btn-normal-content">Logging in</span>
<span className="btn-hover-content">Cancel</span>
</span>
Expand Down
17 changes: 2 additions & 15 deletions packages/chart/src/ChartThemeProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createContext, ReactNode, useEffect, useState } from 'react';
import { createContext, ReactNode, useMemo } from 'react';
import { useTheme } from '@deephaven/components';
import defaultChartTheme, { ChartTheme } from './ChartTheme';

Expand All @@ -20,20 +20,7 @@ export function ChartThemeProvider({
}: ChartThemeProviderProps): JSX.Element {
const { activeThemes } = useTheme();

const [chartTheme, setChartTheme] = useState<ChartTheme | null>(null);

// The `ThemeProvider` that supplies `activeThemes` also provides the corresponding
// CSS theme variables to the DOM by dynamically rendering <style> tags whenever
// the `activeThemes` change. Painting the latest CSS variables to the DOM may
// not happen until after `ChartThemeProvider` is rendered, but they should be
// available by the time the effect runs. Therefore, it is important to derive
// the chart theme in an effect instead of deriving in a `useMemo` to ensure
// we have the latest CSS variables.
useEffect(() => {
if (activeThemes != null) {
setChartTheme(defaultChartTheme());
}
}, [activeThemes]);
const chartTheme = useMemo(defaultChartTheme, [activeThemes]);

return (
<ChartThemeContext.Provider value={chartTheme}>
Expand Down
2 changes: 1 addition & 1 deletion packages/chart/src/MockChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class MockChartModel extends ChartModel {

static smoothing = 1.5;

static _theme: ChartTheme;
static _theme: ChartTheme | null;

static get theme(): ChartTheme {
/* eslint-disable no-underscore-dangle */
Expand Down
29 changes: 7 additions & 22 deletions packages/components/src/RandomAreaPlotAnimation.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable react-hooks/exhaustive-deps */
/* eslint-disable react/display-name */

import React, { useEffect, useState, useRef } from 'react';
import React, { useEffect, useState, useRef, useMemo } from 'react';
import debounce from 'lodash.debounce';
import { assertNotNull } from '@deephaven/utils';
import './RandomAreaPlotAnimation.scss';
Expand Down Expand Up @@ -42,14 +42,9 @@ function getRandomAreaPlotAnimationThemeColors(): ThemeColors {
const RandomAreaPlotAnimation = React.memo(() => {
const { activeThemes } = useTheme();

const [themeColors, setThemeColors] = useState<ThemeColors | null>(null);

// Resolving css variables has to run in `useEffect` or `useLayoutEffect` so
// that we know React has updated the DOM with any styles set by the
// ThemeProvider.
useEffect(() => {
setThemeColors(getRandomAreaPlotAnimationThemeColors());
}, [activeThemes]);
const themeColors = useMemo(getRandomAreaPlotAnimationThemeColors, [
activeThemes,
]);

const canvas = useRef<HTMLCanvasElement>(null);
const container = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -85,10 +80,6 @@ const RandomAreaPlotAnimation = React.memo(() => {

// Returns the background fill create offscreen as pattern
function createPatternFill(): CanvasPattern | null | undefined {
if (themeColors == null) {
return null;
}

const { foregroundFill, foregroundStroke } = themeColors;

// create the off-screen canvas
Expand Down Expand Up @@ -189,10 +180,6 @@ const RandomAreaPlotAnimation = React.memo(() => {
* @param timestamp passed in callback from requestAnimationFrame
*/
function drawCanvas(timestamp?: DOMHighResTimeStamp): void {
if (themeColors == null) {
return;
}

lastTimestamp = lastTimestamp ?? timestamp;

const { background, foregroundStroke, gridColor } = themeColors;
Expand Down Expand Up @@ -343,11 +330,9 @@ const RandomAreaPlotAnimation = React.memo(() => {
}, [themeColors]);

return (
themeColors && (
<div className="random-area-plot-animation-container" ref={container}>
<canvas ref={canvas} className={shade ? 'shade' : ''} />
</div>
)
<div className="random-area-plot-animation-container" ref={container}>
<canvas ref={canvas} className={shade ? 'shade' : ''} />
</div>
);
});

Expand Down
23 changes: 14 additions & 9 deletions packages/components/src/theme/ThemeProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ export interface ThemeProviderProps {
export function ThemeProvider({
themes: customThemes,
children,
}: ThemeProviderProps): JSX.Element {
}: ThemeProviderProps): JSX.Element | null {
const baseThemes = useMemo(() => getDefaultBaseThemes(), []);

const [value, setValue] = useState<ThemeContextValue | null>(null);

const [selectedThemeKey, setSelectedThemeKey] = useState<string>(
() => getThemePreloadData()?.themeKey ?? DEFAULT_DARK_THEME_KEY
);
Expand Down Expand Up @@ -88,18 +90,17 @@ export function ThemeProvider({
[activeThemes, selectedThemeKey, customThemes]
);

const value = useMemo(
() => ({
useEffect(() => {
setValue({
activeThemes,
selectedThemeKey,
themes,
setSelectedThemeKey,
}),
[activeThemes, selectedThemeKey, themes]
);
});
}, [activeThemes, selectedThemeKey, themes]);

return (
<ThemeContext.Provider value={value}>
<>
{activeThemes == null ? null : (
<>
{activeThemes.map(theme => (
Expand All @@ -109,8 +110,12 @@ export function ThemeProvider({
))}
</>
)}
<SpectrumThemeProvider>{children}</SpectrumThemeProvider>
</ThemeContext.Provider>
{value == null ? null : (
<ThemeContext.Provider value={value}>
<SpectrumThemeProvider>{children}</SpectrumThemeProvider>
</ThemeContext.Provider>
)}
</>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class ConsoleHistoryResultInProgress extends Component<
})}
>
<span className="badge">
<LoadingSpinner />
<LoadingSpinner className="ml-2" />
Running... {TimeUtils.formatElapsedTime(elapsed)}&nbsp;
<Button
className="console-history-result-in-progress-cancel"
Expand Down
13 changes: 2 additions & 11 deletions packages/iris-grid/src/IrisGridThemeProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useTheme } from '@deephaven/components';
import { createContext, ReactNode, useEffect, useState } from 'react';
import { createContext, ReactNode, useMemo } from 'react';
import { createDefaultIrisGridTheme, IrisGridThemeType } from './IrisGridTheme';

export type IrisGridThemeContextValue = Partial<IrisGridThemeType>;
Expand All @@ -16,16 +16,7 @@ export function IrisGridThemeProvider({
}: IrisGridThemeProviderProps): JSX.Element {
const { activeThemes } = useTheme();

const [gridTheme, setGridTheme] = useState<IrisGridThemeContextValue>({});

useEffect(
function refreshIrisGridTheme() {
if (activeThemes != null) {
setGridTheme(createDefaultIrisGridTheme());
}
},
[activeThemes]
);
const gridTheme = useMemo(createDefaultIrisGridTheme, [activeThemes]);

return (
<IrisGridThemeContext.Provider value={gridTheme}>
Expand Down
2 changes: 1 addition & 1 deletion packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ class CustomColumnBuilder extends Component<
>
{isCustomColumnApplying && (
<span>
<LoadingSpinner className="loading-spinner-vertical-align" />
<LoadingSpinner className="mr-2 loading-spinner-vertical-align" />
<span className="btn-normal-content">Applying</span>
</span>
)}
Expand Down
2 changes: 1 addition & 1 deletion packages/iris-grid/src/sidebar/TableCsvExporter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ class TableCsvExporter extends Component<
>
{isDownloading && (
<span>
<LoadingSpinner className="loading-spinner-vertical-align" />
<LoadingSpinner className="mr-2 loading-spinner-vertical-align" />
<span className="btn-normal-content">Downloading</span>
<span className="btn-hover-content">Cancel</span>
</span>
Expand Down

0 comments on commit a09bdca

Please sign in to comment.