diff --git a/packages/chart/src/ChartTheme.ts b/packages/chart/src/ChartTheme.ts index be0cc2423f..816d599bb3 100644 --- a/packages/chart/src/ChartTheme.ts +++ b/packages/chart/src/ChartTheme.ts @@ -1,5 +1,9 @@ -import { resolveCssVariablesInRecord } from '@deephaven/components'; +import { + getExpressionRanges, + resolveCssVariablesInRecord, +} from '@deephaven/components'; import Log from '@deephaven/log'; +import { ColorUtils } from '@deephaven/utils'; import chartThemeRaw from './ChartTheme.module.scss'; const log = Log.module('ChartTheme'); @@ -26,6 +30,17 @@ export interface ChartTheme { export function defaultChartTheme(): Readonly { const chartTheme = resolveCssVariablesInRecord(chartThemeRaw); + // The color normalization in `resolveCssVariablesInRecord` won't work for + // colorway since it is an array of colors. We need to explicitly normalize + // each color expression + chartTheme.colorway = getExpressionRanges(chartTheme.colorway ?? '') + .map(([start, end]) => + ColorUtils.normalizeCssColor( + chartTheme.colorway.substring(start, end + 1) + ) + ) + .join(' '); + log.debug2('Chart theme:', chartThemeRaw); log.debug2('Chart theme derived:', chartTheme); diff --git a/packages/components/src/theme/ThemeUtils.test.ts b/packages/components/src/theme/ThemeUtils.test.ts index 5df01d5fc7..80e5234e8d 100644 --- a/packages/components/src/theme/ThemeUtils.test.ts +++ b/packages/components/src/theme/ThemeUtils.test.ts @@ -10,8 +10,8 @@ import { calculatePreloadStyleContent, extractDistinctCssVariableExpressions, getActiveThemes, - getCssVariableRanges, getDefaultBaseThemes, + getExpressionRanges, getThemeKey, getThemePreloadData, preloadTheme, @@ -138,11 +138,20 @@ describe('getActiveThemes', () => { }); }); -describe('getCssVariableRanges', () => { - const t = [ - ['Single var', 'var(--aaa-aa)', [[0, 12]]], +describe('getExpressionRanges', () => { + const testCases = [ + ['Single expression', '#ffffff', [[0, 6]]], + ['Single var expression', 'var(--aaa-aa)', [[0, 12]]], [ - 'Multiple vars', + 'Multiple expressions', + '#ffffff #ffffff', + [ + [0, 6], + [8, 14], + ], + ], + [ + 'Multiple var expressions', 'var(--aaa-aa) var(--bbb-bb)', [ [0, 12], @@ -150,20 +159,33 @@ describe('getCssVariableRanges', () => { ], ], [ - 'Nested vars - level 2', + 'Mixed expressions', + ' var(--aaa-aa)\n #fff \n\n var(--bbb-bb) \n ', + [ + [1, 13], + [18, 21], + [26, 38], + ], + ], + [ + 'Nested expressions - level 2', 'var(--ccc-cc, var(--aaa-aa, green)) var(--bbb-bb)', [ [0, 34], [36, 48], ], ], - ['Nested vars - level 3', 'var(--a, var(--b, var(--c, red)))', [[0, 32]]], [ - 'Nested vars - level 4', - 'var(--a, var(--b, var(--c, var(--d, red)))) var(--e, var(--f, var(--g, var(--h, red))))', + 'Nested expressions - level 3', + 'var(--a, var(--b, var(--c, red)))', + [[0, 32]], + ], + [ + 'Nested expressions - level 4', + 'var(--a, var(--b, \n var(--c, var(--d, red)))) \n\t\n var(--e, var(--f, var(--g, var(--h, red)))) \n \t', [ - [0, 42], - [44, 86], + [0, 44], + [50, 92], ], ], ['Nested calc - level 3', 'var(--a, calc(calc(1px + 2px)))', [[0, 30]]], @@ -175,11 +197,10 @@ describe('getCssVariableRanges', () => { ['Unbalanced', 'var(--a', []], ] as const; - it.each(t)( - 'should return the css variable ranges - %s: %s, %s', - (_label, given, expected) => { - const actual = getCssVariableRanges(given); - expect(actual).toEqual(expected); + it.each(testCases)( + 'should return top-level expression ranges: %s, %s', + (_, given, expected) => { + expect(getExpressionRanges(given)).toEqual(expected); } ); }); @@ -411,7 +432,7 @@ describe('resolveCssVariablesInString', () => { [ 'Non top-level var', 'calc(var(--a, calc(calc(calc(1px + 2px) + 3px)))) var(--b)', - 'calc(R[var(--a, calc(calc(calc(1px + 2px) + 3px)))]) R[var(--b)]', + 'R[calc(var(--a, calc(calc(calc(1px + 2px) + 3px))))] R[var(--b)]', ], ])('should replace css variables - %s: %s, %s', (_label, given, expected) => { const actual = resolveCssVariablesInString(mockResolver, given); diff --git a/packages/components/src/theme/ThemeUtils.ts b/packages/components/src/theme/ThemeUtils.ts index f1bb6fd6b2..85eec63020 100644 --- a/packages/components/src/theme/ThemeUtils.ts +++ b/packages/components/src/theme/ThemeUtils.ts @@ -27,7 +27,10 @@ import { const log = Log.module('ThemeUtils'); +export const CSS_VAR_EXPRESSION_PREFIX = 'var(--'; export const TMP_CSS_PROP_PREFIX = 'dh-tmp'; +export const NON_WHITESPACE_REGEX = /\S/; +export const WHITESPACE_REGEX = /\s/; export type VarExpressionResolver = (varExpression: string) => string; @@ -60,8 +63,12 @@ export function extractDistinctCssVariableExpressions( const set = new Set(); Object.values(record).forEach(value => { - getCssVariableRanges(value).forEach(([start, end]) => { - set.add(value.substring(start, end + 1)); + getExpressionRanges(value).forEach(([start, end]) => { + const expression = value.substring(start, end + 1); + + if (expression.includes(CSS_VAR_EXPRESSION_PREFIX)) { + set.add(expression); + } }); }); @@ -143,59 +150,61 @@ export function getThemePreloadData(): ThemePreloadData | null { } /** - * Identifies start and end indices of any css variable expressions in the given + * Identifies start and end indices of any top-level expressions in the given * string. * * e.g. - * getCssVariableRanges('var(--aaa-aa) var(--bbb-bb)') + * getExpressionRanges('var(--aaa-aa) #fff var(--bbb-bb)') * yields: * [ - * [0, 12], - * [14, 26], + * [0, 12], // 'var(--aaa-aa)' + * [14, 17] // '#fff' + * [19, 31], // 'var(--bbb-bb)' * ] * * In cases where there are nested expressions, only the indices of the outermost * expression will be included. * * e.g. - * getCssVariableRanges('var(--ccc-cc, var(--aaa-aa, green)) var(--bbb-bb)') + * getExpressionRanges('var(--ccc-cc, var(--aaa-aa, green)) var(--bbb-bb)') * yields: * [ - * [0, 34], // range for --ccc-cc expression - * [36, 48], // range for --bbb-bb expression + * [0, 34], // 'var(--ccc-cc, var(--aaa-aa, green))' + * [36, 48], // 'var(--bbb-bb)' * ] - * @param value The string to search for css variable expressions - * @returns An array of [start, end] index pairs for each css variable expression + * @param value The string to search for expressions + * @returns An array of [start, end] index pairs for each expression */ -export function getCssVariableRanges(value: string): [number, number][] { +export function getExpressionRanges(value: string): [number, number][] { const ranges: [number, number][] = []; - const cssVarPrefix = 'var(--'; - let start = value.indexOf(cssVarPrefix); + let start = NON_WHITESPACE_REGEX.exec(value)?.index ?? 0; let parenLevel = 0; - while (start > -1) { - parenLevel = 1; - let i = start + cssVarPrefix.length; - for (; i < value.length; i += 1) { - if (value[i] === '(') { - parenLevel += 1; - } else if (value[i] === ')') { - parenLevel -= 1; - } + for (let i = 0; i < value.length; i += 1) { + if (value[i] === '(') { + parenLevel += 1; + } else if (value[i] === ')') { + parenLevel -= 1; + } + + if ( + i === value.length - 1 || + (WHITESPACE_REGEX.test(value[i + 1]) && parenLevel === 0) + ) { + ranges.push([start, i]); - if (parenLevel === 0) { - ranges.push([start, i]); - break; + while (i < value.length - 1 && WHITESPACE_REGEX.test(value[i + 1])) { + i += 1; } - } - if (parenLevel !== 0) { - log.error('Unbalanced parentheses in css var expression', value); - return []; + start = i + 1; } + } - start = value.indexOf(cssVarPrefix, i + 1); + if (parenLevel !== 0) { + log.error('Unbalanced parentheses in css var expression', value); + return []; } return ranges; @@ -280,13 +289,19 @@ export function resolveCssVariablesInString( ): string { const result: string[] = []; let i = 0; - getCssVariableRanges(value).forEach(([start, end]) => { + getExpressionRanges(value).forEach(([start, end]) => { if (i < start) { result.push(value.substring(i, start)); i += start - i; } - result.push(resolver(value.substring(start, end + 1))); + const expression = value.substring(start, end + 1); + + result.push( + expression.includes(CSS_VAR_EXPRESSION_PREFIX) + ? resolver(expression) + : expression + ); i += end - start + 1; });