From ad10ce0473a7ece076c5065caaab53a28c9742ee Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Thu, 13 Aug 2026 10:30:11 -0600 Subject: [PATCH 1/6] fix: Ensure Sana theme is portaled --- .storybook/set-data-theme.js | 10 +- .../react/common/lib/theming/brandScope.ts | 5 + modules/react/common/lib/theming/sanaTheme.ts | 173 ++++++++++++------ modules/react/common/lib/theming/types.ts | 25 ++- modules/react/common/spec/sanaTheme.spec.ts | 70 ++++++- modules/react/common/stories/mdx/Theming.mdx | 5 + .../mdx/examples/SimplifiedSanaSetup.tsx | 6 +- 7 files changed, 220 insertions(+), 74 deletions(-) diff --git a/.storybook/set-data-theme.js b/.storybook/set-data-theme.js index 23635aa0c3..df27df28b9 100644 --- a/.storybook/set-data-theme.js +++ b/.storybook/set-data-theme.js @@ -1,8 +1,8 @@ // ?theme=canvas` URL param for switching off the default Sana Canvas theme, for dev and QA use only. // This must be set on (not a nested element) since the sana token CSS only defines // `[data-theme="sana-canvas"]` overrides - there's no `[data-theme="canvas"]` rule to undo it. -var themeParam = new URLSearchParams(window.location.search).get('theme'); -document.documentElement.setAttribute( - 'data-theme', - themeParam === 'canvas' ? 'canvas' : 'sana-canvas' -); +// var themeParam = new URLSearchParams(window.location.search).get('theme'); +// document.documentElement.setAttribute( +// 'data-theme', +// themeParam === 'canvas' ? 'canvas' : 'sana-canvas' +// ); diff --git a/modules/react/common/lib/theming/brandScope.ts b/modules/react/common/lib/theming/brandScope.ts index f52da0a5b7..491b83a52e 100644 --- a/modules/react/common/lib/theming/brandScope.ts +++ b/modules/react/common/lib/theming/brandScope.ts @@ -73,6 +73,11 @@ const EXTENDED_BRAND_TOKEN_MAP: Record = { neutral150: '--cnvs-brand-neutral-150', neutral850: '--cnvs-brand-neutral-850', neutralA150: '--cnvs-brand-neutral-a150', + neutralA850: '--cnvs-brand-neutral-a850', + primaryA300: '--cnvs-brand-primary-a300', + criticalA300: '--cnvs-brand-critical-a300', + cautionA300: '--cnvs-brand-caution-a300', + positiveA300: '--cnvs-brand-positive-a300', }; const setStyleVar = (style: React.CSSProperties, token: string, value: string) => { diff --git a/modules/react/common/lib/theming/sanaTheme.ts b/modules/react/common/lib/theming/sanaTheme.ts index 2f96d9a4aa..71023a2812 100644 --- a/modules/react/common/lib/theming/sanaTheme.ts +++ b/modules/react/common/lib/theming/sanaTheme.ts @@ -23,85 +23,137 @@ import type {CanvasNumericalBrandTheme} from './types'; const varRef = (token: string) => `var(${token})`; /** - * Sana extends the neutral ramp with steps not yet exported from canvas-tokens-web JS. - * Defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. + * Sana extends the neutral ramp with base-palette steps not yet exported from + * canvas-tokens-web JS. Defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. */ -const sanaBrandNeutral = { - '150': '--cnvs-brand-neutral-150', - '850': '--cnvs-brand-neutral-850', - A150: '--cnvs-brand-neutral-a150', +const sanaBaseNeutral = { + '150': '--cnvs-base-palette-neutral-150', + '850': '--cnvs-base-palette-neutral-850', + A150: '--cnvs-base-palette-neutral-a150', + A850: '--cnvs-base-palette-neutral-a850', +} as const; + +/** + * Sana's only distinct step for `primary`/`critical`/`caution`/`positive` — a stronger alpha + * wash (`A300`) on top of the matching base-palette hue. Not yet exported from canvas-tokens-web + * JS; defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. + */ +const sanaBaseAccentA300 = { + primary: '--cnvs-base-palette-blue-a300', + critical: '--cnvs-base-palette-red-a300', + caution: '--cnvs-base-palette-amber-a300', + positive: '--cnvs-base-palette-green-a300', } as const; /** * Sana Canvas brand tokens for scoped `CanvasProvider` / popup forwarding. * Values are `var()` references to Sana brand variables — not merged from `defaultCanvasTheme`. + * + * `action` and `neutral` are fully populated: Sana's palette is neutral/monochrome-driven, so + * `action.*` reads directly from the `neutral` ramp (see + * `@workday/canvas-tokens-web/css/sana/_variables.css`). `primary`/`critical`/`caution`/`positive` + * only set `A300` — the one step Sana actually redefines for those families (a stronger alpha + * wash on the matching hue) — every other key is intentionally omitted: Sana does not define a + * distinct value for it, so writing it here would only reference the very variable being written + * (a `var()` cycle that resolves to invalid, leaking the classic-theme fallback color instead of + * Sana's). + * + * The `selected.fg`/`selected.surface` shortcuts (`system.color.brand.fg`/`surface.selected`, + * aliases for `primary.700`/`primary.A50`) are likewise omitted — Sana doesn't redefine those + * ramp steps either, so selected `Menu.Item`/`Menu.Option` state stays on the classic values. + * + * `system.color.brand.accent.primary`/`.action` and `.fg.primary.default`/`.strong` **are** + * forwarded (via the `system.color.brand.*` escape hatch — see + * {@link CanvasNumericalBrandTheme.system}) — unlike `selected`, Sana's stylesheet does + * redefine these four, to `brand.neutral.975` / `.A900` / `.A950`, so portaled popups need the + * override too for parity with in-document Sana styling. + * + * Ramp values must reference `base.*` (the underlying palette), never `brand.*` of the same + * name — CanvasProvider writes each entry onto the identically-named `--cnvs-brand-*` CSS + * variable, so referencing `brand.*` here would create that same self-reference cycle. The + * `system.color.brand.*` overrides above are the exception: they target *different* CSS + * variables (`--cnvs-sys-color-brand-*`) than the `brand.*` values they reference, so no cycle. */ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { // Explicit brand vars only — multi-key ramps write 1:1; no system shortcut bundles run. themeScope: 'brand', brand: { + primary: { + A300: varRef(sanaBaseAccentA300.primary), + }, + critical: { + A300: varRef(sanaBaseAccentA300.critical), + }, + caution: { + A300: varRef(sanaBaseAccentA300.caution), + }, + positive: { + A300: varRef(sanaBaseAccentA300.positive), + }, action: { base: varRef(brand.neutral975), dark: varRef(brand.neutral950), darkest: varRef(brand.neutral900), + darker: varRef(brand.neutral975), accent: varRef(base.neutral0), lightest: varRef(brand.neutral25), lighter: varRef(brand.neutral50), light: varRef(brand.neutral200), }, neutral: { - '25': varRef(brand.neutral25), - '50': varRef(brand.neutral50), - '100': varRef(brand.neutral100), - '150': varRef(sanaBrandNeutral['150']), - '200': varRef(brand.neutral200), - '300': varRef(brand.neutral300), - '400': varRef(brand.neutral400), - '500': varRef(brand.neutral500), - '600': varRef(brand.neutral600), - '700': varRef(brand.neutral700), - '800': varRef(brand.neutral800), - '850': varRef(sanaBrandNeutral['850']), - '900': varRef(brand.neutral900), - '950': varRef(brand.neutral950), - '975': varRef(brand.neutral975), - A25: varRef(brand.neutralA25), - A50: varRef(brand.neutralA50), - A100: varRef(brand.neutralA100), - A150: varRef(sanaBrandNeutral.A150), - A200: varRef(brand.neutralA200), - }, - primary: { - '500': varRef(brand.primary500), - '600': varRef(brand.primary600), - '700': varRef(brand.primary700), - A25: varRef(brand.primaryA25), - A50: varRef(brand.primaryA50), - A100: varRef(brand.primaryA100), - }, - critical: { - '500': varRef(brand.critical500), - '600': varRef(brand.critical600), - '700': varRef(brand.critical700), - A25: varRef(brand.criticalA25), - A50: varRef(brand.criticalA50), - }, - caution: { - '400': varRef(brand.caution400), - '500': varRef(brand.caution500), - A25: varRef(brand.cautionA25), - A50: varRef(brand.cautionA50), - }, - positive: { - '600': varRef(brand.positive600), - '800': varRef(brand.positive800), - A25: varRef(brand.positiveA25), - A50: varRef(brand.positiveA50), + '25': varRef(base.neutral25), + '50': varRef(base.neutral50), + '100': varRef(base.neutral100), + '150': varRef(sanaBaseNeutral['150']), + '200': varRef(base.neutral200), + '300': varRef(base.neutral300), + '400': varRef(base.neutral400), + '500': varRef(base.neutral500), + '600': varRef(base.neutral600), + '700': varRef(base.neutral700), + '800': varRef(base.neutral800), + '850': varRef(sanaBaseNeutral['850']), + '900': varRef(base.neutral900), + '950': varRef(base.neutral950), + '975': varRef(base.neutral975), + A25: varRef(base.neutralA25), + A50: varRef(base.neutralA50), + A100: varRef(base.neutralA100), + A150: varRef(sanaBaseNeutral.A150), + A200: varRef(base.neutralA200), + A300: varRef(base.neutralA300), + A400: varRef(base.neutralA400), + A500: varRef(base.neutralA500), + A600: varRef(base.neutralA600), + A700: varRef(base.neutralA700), + A800: varRef(base.neutralA800), + A850: varRef(sanaBaseNeutral.A850), + A900: varRef(base.neutralA900), + A950: varRef(base.neutralA950), + A975: varRef(base.neutralA975), }, }, - selected: { - fg: varRef(brand.neutralA900), - surface: varRef(brand.neutralA100), + // `selected` is intentionally omitted — see the `primary`/`critical`/`caution`/`positive` note + // above. Selected Menu.Item/Menu.Option state falls through to the classic + // `brand.primary.700` / `brand.primary.A50` values, unchanged by Sana. + // + // Unlike `selected`, Sana's stylesheet *does* redefine these four `system.color.brand.*` + // tokens, so forward them for portal parity (see the doc comment above). + system: { + color: { + brand: { + accent: { + primary: varRef(brand.neutral975), + action: varRef(brand.neutral975), + }, + fg: { + primary: { + default: varRef(brand.neutralA900), + strong: varRef(brand.neutralA950), + }, + }, + }, + }, }, }; @@ -119,6 +171,15 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { * - Prefer setting `data-theme="sana-canvas"` on `` with Sana CSS imported. Popups then * inherit brand variables from the document and no `theme` prop is needed. * + * Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / + * `-surface-selected`) is unaffected either way — Sana doesn't redefine `brand.primary`, so + * those resolve to the classic `brand.primary.700` / `.A50` values with or without this preset. + * + * `--cnvs-sys-color-brand-accent-primary`/`-accent-action`/`-fg-primary-default`/ + * `-fg-primary-strong` are different: Sana's stylesheet *does* redefine them (to Sana neutral + * tones), so without this preset a popup outside `data-theme="sana-canvas"`'s reach falls back + * to classic primary-derived colors for those four, out of step with the rest of a Sana UI. + * * @example * ```tsx * // Preferred — control diff --git a/modules/react/common/lib/theming/types.ts b/modules/react/common/lib/theming/types.ts index a88e1f552c..d87e28fe06 100644 --- a/modules/react/common/lib/theming/types.ts +++ b/modules/react/common/lib/theming/types.ts @@ -312,17 +312,34 @@ export type CanvasBrandRamp = Partial< | 'A25' | 'A50' | 'A100' - | 'A200', + | 'A200' + | 'A300', string > >; /** - * Neutral brand ramp — includes Sana-only steps (`150` / `850` / `A150`) that are not - * exported for primary/critical/caution/positive families. + * Neutral brand ramp — includes Sana-only steps (`150` / `850` / `A150` / `A850`), plus the + * extended alpha steps (`A400`–`A975`) that only the neutral family exposes. */ export type CanvasNeutralBrandRamp = CanvasBrandRamp & - Partial>; + Partial< + Record< + | '150' + | '850' + | 'A150' + | 'A400' + | 'A500' + | 'A600' + | 'A700' + | 'A800' + | 'A850' + | 'A900' + | 'A950' + | 'A975', + string + > + >; /** Semantic keys for `brand.action.*` CSS variables (PrimaryButton, etc.). */ export type CanvasActionBrandRamp = Partial< diff --git a/modules/react/common/spec/sanaTheme.spec.ts b/modules/react/common/spec/sanaTheme.spec.ts index 7166c5ccf5..5a030044b8 100644 --- a/modules/react/common/spec/sanaTheme.spec.ts +++ b/modules/react/common/spec/sanaTheme.spec.ts @@ -5,8 +5,8 @@ import {defaultCanvasTheme} from '../lib/theming'; import {sanaCanvasNumericalTheme, sanaCanvasProviderTheme} from '../lib/theming/sanaTheme'; describe('sanaCanvasNumericalTheme', () => { - it('references Sana brand CSS variables instead of defaultCanvasTheme literals', () => { - expect(sanaCanvasNumericalTheme.brand?.neutral?.['600']).toBe(`var(${brand.neutral600})`); + it('references Sana base-palette CSS variables instead of defaultCanvasTheme literals', () => { + expect(sanaCanvasNumericalTheme.brand?.neutral?.['600']).toBe(`var(${base.neutral600})`); expect(sanaCanvasNumericalTheme.brand?.action?.base).toBe(`var(${brand.neutral975})`); expect(sanaCanvasNumericalTheme.brand?.action?.accent).toBe(`var(${base.neutral0})`); expect(sanaCanvasNumericalTheme.brand?.neutral?.['600']).not.toBe( @@ -14,17 +14,73 @@ describe('sanaCanvasNumericalTheme', () => { ); }); + it('never references the same CSS variable name it writes to (would create a var() cycle)', () => { + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + Object.entries(style).forEach(([key, value]) => { + if (key.startsWith('--') && typeof value === 'string') { + expect(value).not.toBe(`var(${key})`); + } + }); + }); + + it("only writes the A300 step of primary/critical/caution/positive (Sana's one distinct value)", () => { + expect(Object.keys(sanaCanvasNumericalTheme.brand?.primary ?? {})).toEqual(['A300']); + expect(Object.keys(sanaCanvasNumericalTheme.brand?.critical ?? {})).toEqual(['A300']); + expect(Object.keys(sanaCanvasNumericalTheme.brand?.caution ?? {})).toEqual(['A300']); + expect(Object.keys(sanaCanvasNumericalTheme.brand?.positive ?? {})).toEqual(['A300']); + + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + expect(style['--cnvs-brand-primary-a300' as any]).toBe('var(--cnvs-base-palette-blue-a300)'); + expect(style['--cnvs-brand-critical-a300' as any]).toBe('var(--cnvs-base-palette-red-a300)'); + expect(style['--cnvs-brand-caution-a300' as any]).toBe('var(--cnvs-base-palette-amber-a300)'); + expect(style['--cnvs-brand-positive-a300' as any]).toBe('var(--cnvs-base-palette-green-a300)'); + }); + it('writes brand tokens when passed to canvasThemeToCssVars', () => { const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); expect(Object.keys(style).length).toBeGreaterThan(0); - expect(style[brand.neutral600 as any]).toBe(`var(${brand.neutral600})`); + expect(style[brand.neutral600 as any]).toBe(`var(${base.neutral600})`); expect(style[brand.action.base as any]).toBe(`var(${brand.neutral975})`); }); - it('writes Sana extended neutral ramp keys', () => { + it('writes Sana extended neutral ramp keys from the base palette', () => { const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style['--cnvs-brand-neutral-150' as any]).toBe('var(--cnvs-brand-neutral-150)'); - expect(style['--cnvs-brand-neutral-850' as any]).toBe('var(--cnvs-brand-neutral-850)'); - expect(style['--cnvs-brand-neutral-a150' as any]).toBe('var(--cnvs-brand-neutral-a150)'); + expect(style['--cnvs-brand-neutral-150' as any]).toBe('var(--cnvs-base-palette-neutral-150)'); + expect(style['--cnvs-brand-neutral-850' as any]).toBe('var(--cnvs-base-palette-neutral-850)'); + expect(style['--cnvs-brand-neutral-a150' as any]).toBe('var(--cnvs-base-palette-neutral-a150)'); + expect(style['--cnvs-brand-neutral-a850' as any]).toBe('var(--cnvs-base-palette-neutral-a850)'); + }); + + it('writes the full neutral alpha ramp through A975 (not just up to A200)', () => { + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + expect(style[brand.neutralA300 as any]).toBe(`var(${base.neutralA300})`); + expect(style[brand.neutralA900 as any]).toBe(`var(${base.neutralA900})`); + expect(style[brand.neutralA975 as any]).toBe(`var(${base.neutralA975})`); + }); + + it('writes action.darker alongside the rest of the action bundle', () => { + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + expect(style[brand.action.darker as any]).toBe(`var(${brand.neutral975})`); + }); + + it('does not write selected.fg/selected.surface shortcuts (no distinct Sana primary values)', () => { + expect(sanaCanvasNumericalTheme.selected).toBeUndefined(); + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + expect(style['--cnvs-sys-color-brand-fg-selected' as any]).toBeUndefined(); + expect(style['--cnvs-sys-color-brand-surface-selected' as any]).toBeUndefined(); + }); + + it('forwards accent.primary/action and fg.primary.default/strong for portal parity', () => { + // Unlike `selected`, Sana's stylesheet redefines these four `system.color.brand.*` + // tokens, so popups outside `[data-theme="sana-canvas"]`'s reach need the override too. + const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); + expect(style['--cnvs-sys-color-brand-accent-primary' as any]).toBe(`var(${brand.neutral975})`); + expect(style['--cnvs-sys-color-brand-accent-action' as any]).toBe(`var(${brand.neutral975})`); + expect(style['--cnvs-sys-color-brand-fg-primary-default' as any]).toBe( + `var(${brand.neutralA900})` + ); + expect(style['--cnvs-sys-color-brand-fg-primary-strong' as any]).toBe( + `var(${brand.neutralA950})` + ); }); }); diff --git a/modules/react/common/stories/mdx/Theming.mdx b/modules/react/common/stories/mdx/Theming.mdx index e3106e3c8c..9b7fd8c586 100644 --- a/modules/react/common/stories/mdx/Theming.mdx +++ b/modules/react/common/stories/mdx/Theming.mdx @@ -118,6 +118,11 @@ import {CanvasProvider} from '@workday/canvas-kit-react/common'; ``` +Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / +`-surface-selected`) is unaffected by Sana either way: Sana doesn't redefine `brand.primary`, so +those variables keep resolving to the classic `brand.primary.700` / `.A50` values whether or not +`data-theme="sana-canvas"` is set. + **Scoped / no document-root control:** if you cannot set `data-theme` on `` (embedded apps, microfrontends, third-party shells), a nested `data-theme` alone does **not** reach portaled popups. Pass both `data-theme="sana-canvas"` (for in-tree UI) and `sanaCanvasProviderTheme` (so diff --git a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx index 11641a13ad..d19ea609ef 100644 --- a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx +++ b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx @@ -20,8 +20,10 @@ export const SimplifiedSetup = () => { - - Option 1 + + + Option 1 + Option 2 Option 3 From e9e5e2184fcdf82d453201b7507934b1f863bdc3 Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Thu, 13 Aug 2026 11:00:57 -0600 Subject: [PATCH 2/6] fix: Clean up code --- modules/react/common/lib/theming/sanaTheme.ts | 96 +++++++++---------- .../react/common/spec/CanvasProvider.spec.tsx | 10 ++ .../mdx/examples/SimplifiedSanaSetup.tsx | 2 +- 3 files changed, 58 insertions(+), 50 deletions(-) diff --git a/modules/react/common/lib/theming/sanaTheme.ts b/modules/react/common/lib/theming/sanaTheme.ts index 71023a2812..8fbde31e3b 100644 --- a/modules/react/common/lib/theming/sanaTheme.ts +++ b/modules/react/common/lib/theming/sanaTheme.ts @@ -15,13 +15,11 @@ * | `sanaCanvasNumericalTheme` | Numerical `brand` shape for popup forwarding | * | `sanaCanvasProviderTheme` | Same — pass to root `CanvasProvider` when `` is unavailable | */ +import {cssVar} from '@workday/canvas-kit-styling'; import {base, brand} from '@workday/canvas-tokens-web'; import type {CanvasNumericalBrandTheme} from './types'; -/** Reference a canvas-tokens CSS variable (resolves under `[data-theme="sana-canvas"]`). */ -const varRef = (token: string) => `var(${token})`; - /** * Sana extends the neutral ramp with base-palette steps not yet exported from * canvas-tokens-web JS. Defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. @@ -79,58 +77,58 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { themeScope: 'brand', brand: { primary: { - A300: varRef(sanaBaseAccentA300.primary), + A300: cssVar(sanaBaseAccentA300.primary), }, critical: { - A300: varRef(sanaBaseAccentA300.critical), + A300: cssVar(sanaBaseAccentA300.critical), }, caution: { - A300: varRef(sanaBaseAccentA300.caution), + A300: cssVar(sanaBaseAccentA300.caution), }, positive: { - A300: varRef(sanaBaseAccentA300.positive), + A300: cssVar(sanaBaseAccentA300.positive), }, action: { - base: varRef(brand.neutral975), - dark: varRef(brand.neutral950), - darkest: varRef(brand.neutral900), - darker: varRef(brand.neutral975), - accent: varRef(base.neutral0), - lightest: varRef(brand.neutral25), - lighter: varRef(brand.neutral50), - light: varRef(brand.neutral200), + base: cssVar(brand.neutral975), + dark: cssVar(brand.neutral950), + darkest: cssVar(brand.neutral900), + darker: cssVar(brand.neutral975), + accent: cssVar(base.neutral0), + lightest: cssVar(brand.neutral25), + lighter: cssVar(brand.neutral50), + light: cssVar(brand.neutral200), }, neutral: { - '25': varRef(base.neutral25), - '50': varRef(base.neutral50), - '100': varRef(base.neutral100), - '150': varRef(sanaBaseNeutral['150']), - '200': varRef(base.neutral200), - '300': varRef(base.neutral300), - '400': varRef(base.neutral400), - '500': varRef(base.neutral500), - '600': varRef(base.neutral600), - '700': varRef(base.neutral700), - '800': varRef(base.neutral800), - '850': varRef(sanaBaseNeutral['850']), - '900': varRef(base.neutral900), - '950': varRef(base.neutral950), - '975': varRef(base.neutral975), - A25: varRef(base.neutralA25), - A50: varRef(base.neutralA50), - A100: varRef(base.neutralA100), - A150: varRef(sanaBaseNeutral.A150), - A200: varRef(base.neutralA200), - A300: varRef(base.neutralA300), - A400: varRef(base.neutralA400), - A500: varRef(base.neutralA500), - A600: varRef(base.neutralA600), - A700: varRef(base.neutralA700), - A800: varRef(base.neutralA800), - A850: varRef(sanaBaseNeutral.A850), - A900: varRef(base.neutralA900), - A950: varRef(base.neutralA950), - A975: varRef(base.neutralA975), + '25': cssVar(base.neutral25), + '50': cssVar(base.neutral50), + '100': cssVar(base.neutral100), + '150': cssVar(sanaBaseNeutral['150']), + '200': cssVar(base.neutral200), + '300': cssVar(base.neutral300), + '400': cssVar(base.neutral400), + '500': cssVar(base.neutral500), + '600': cssVar(base.neutral600), + '700': cssVar(base.neutral700), + '800': cssVar(base.neutral800), + '850': cssVar(sanaBaseNeutral['850']), + '900': cssVar(base.neutral900), + '950': cssVar(base.neutral950), + '975': cssVar(base.neutral975), + A25: cssVar(base.neutralA25), + A50: cssVar(base.neutralA50), + A100: cssVar(base.neutralA100), + A150: cssVar(sanaBaseNeutral.A150), + A200: cssVar(base.neutralA200), + A300: cssVar(base.neutralA300), + A400: cssVar(base.neutralA400), + A500: cssVar(base.neutralA500), + A600: cssVar(base.neutralA600), + A700: cssVar(base.neutralA700), + A800: cssVar(base.neutralA800), + A850: cssVar(sanaBaseNeutral.A850), + A900: cssVar(base.neutralA900), + A950: cssVar(base.neutralA950), + A975: cssVar(base.neutralA975), }, }, // `selected` is intentionally omitted — see the `primary`/`critical`/`caution`/`positive` note @@ -143,13 +141,13 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { color: { brand: { accent: { - primary: varRef(brand.neutral975), - action: varRef(brand.neutral975), + primary: cssVar(brand.neutral975), + action: cssVar(brand.neutral975), }, fg: { primary: { - default: varRef(brand.neutralA900), - strong: varRef(brand.neutralA950), + default: cssVar(brand.neutralA900), + strong: cssVar(brand.neutralA950), }, }, }, diff --git a/modules/react/common/spec/CanvasProvider.spec.tsx b/modules/react/common/spec/CanvasProvider.spec.tsx index 487feeb115..2829ecea55 100644 --- a/modules/react/common/spec/CanvasProvider.spec.tsx +++ b/modules/react/common/spec/CanvasProvider.spec.tsx @@ -5,6 +5,16 @@ import {CanvasProvider} from '../lib/CanvasProvider'; import {sanaCanvasProviderTheme} from '../lib/theming/sanaTheme'; describe('CanvasProvider', () => { + it('forwards data-theme onto the wrapper div', () => { + const {container} = render( + +
Test
+ + ); + + expect(container.firstElementChild?.getAttribute('data-theme')).toBe('sana-canvas'); + }); + describe('console warnings', () => { it('should warn when sanaCanvasProviderTheme is used with global Sana theme', () => { const consoleSpy = vi.spyOn(global.console, 'warn').mockImplementation(() => {}); diff --git a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx index d19ea609ef..76b01a1229 100644 --- a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx +++ b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx @@ -16,7 +16,7 @@ export const SimplifiedSetup = () => { return ( - Open Menu + Open Menu From 0f8415a08e2914e7a591cf0c479cd45d46c337fc Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Thu, 13 Aug 2026 11:14:47 -0600 Subject: [PATCH 3/6] fix: Undo change --- .storybook/set-data-theme.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.storybook/set-data-theme.js b/.storybook/set-data-theme.js index df27df28b9..23635aa0c3 100644 --- a/.storybook/set-data-theme.js +++ b/.storybook/set-data-theme.js @@ -1,8 +1,8 @@ // ?theme=canvas` URL param for switching off the default Sana Canvas theme, for dev and QA use only. // This must be set on (not a nested element) since the sana token CSS only defines // `[data-theme="sana-canvas"]` overrides - there's no `[data-theme="canvas"]` rule to undo it. -// var themeParam = new URLSearchParams(window.location.search).get('theme'); -// document.documentElement.setAttribute( -// 'data-theme', -// themeParam === 'canvas' ? 'canvas' : 'sana-canvas' -// ); +var themeParam = new URLSearchParams(window.location.search).get('theme'); +document.documentElement.setAttribute( + 'data-theme', + themeParam === 'canvas' ? 'canvas' : 'sana-canvas' +); From 40fb41651d89df1419685171db40558cbf23decf Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Thu, 13 Aug 2026 13:06:47 -0600 Subject: [PATCH 4/6] test: Update popup test --- .../react/popup/spec/usePopupStack.spec.tsx | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/modules/react/popup/spec/usePopupStack.spec.tsx b/modules/react/popup/spec/usePopupStack.spec.tsx index 864277ee35..94a3168f11 100644 --- a/modules/react/popup/spec/usePopupStack.spec.tsx +++ b/modules/react/popup/spec/usePopupStack.spec.tsx @@ -58,14 +58,24 @@ describe('usePopupStack', () => { // Wait for the effect to apply styles await waitFor(() => { - // Check that the container has the correct CSS variables const styles = container?.style; if (styles) { - // The Sana theme should set brand variables - // Sana theme uses CSS variable references (var(--cnvs-brand-primary-600)) - const primary600 = styles.getPropertyValue('--cnvs-brand-primary-600'); - // Check that the value is a CSS variable reference - expect(primary600).toContain('var(--cnvs-brand-primary-600)'); + // Sana only redefines primary A300 (not 600) — writing primary-600 would be a + // var() self-reference cycle. Neutral/action ramps and system.color.brand.* + // overrides are forwarded as var() references to the underlying palette. + expect(styles.getPropertyValue('--cnvs-brand-primary-600')).toBe(''); + expect(styles.getPropertyValue('--cnvs-brand-primary-a300')).toBe( + 'var(--cnvs-base-palette-blue-a300)' + ); + expect(styles.getPropertyValue('--cnvs-brand-neutral-600')).toBe( + 'var(--cnvs-base-palette-neutral-600)' + ); + expect(styles.getPropertyValue('--cnvs-brand-action-base')).toBe( + 'var(--cnvs-brand-neutral-975)' + ); + expect(styles.getPropertyValue('--cnvs-sys-color-brand-accent-primary')).toBe( + 'var(--cnvs-brand-neutral-975)' + ); } }); }); From e9929d3dac378b27fe4b74506123119d5e846ba9 Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Mon, 17 Aug 2026 09:31:13 -0600 Subject: [PATCH 5/6] fix: Restore Sana selected-state colors and fix broken Menu.Option example Sana's selected Menu.Item/Menu.Option fg/surface colors were dropped when reworking the theme to avoid var() self-reference cycles, silently regressing portaled popups back to classic blue. Restore them via the neutral ramp (no cycle risk, since they target different CSS variables). Also fix the SimplifiedSanaSetup story's Menu.Option, which used `id` instead of `data-id` so initialSelectedIds never matched, masked by a hardcoded aria-selected prop. Additionally, replace hand-typed Sana CSS variable name strings with canvas-tokens-web's own `base.sana` export to avoid drift. Co-Authored-By: Claude Sonnet 5 --- modules/react/common/lib/theming/sanaTheme.ts | 70 ++++++++----------- modules/react/common/spec/sanaTheme.spec.ts | 25 +++---- modules/react/common/stories/mdx/Theming.mdx | 4 +- .../mdx/examples/SimplifiedSanaSetup.tsx | 6 +- .../react/popup/spec/usePopupStack.spec.tsx | 15 ++-- 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/modules/react/common/lib/theming/sanaTheme.ts b/modules/react/common/lib/theming/sanaTheme.ts index 8fbde31e3b..cdcda53cce 100644 --- a/modules/react/common/lib/theming/sanaTheme.ts +++ b/modules/react/common/lib/theming/sanaTheme.ts @@ -21,27 +21,15 @@ import {base, brand} from '@workday/canvas-tokens-web'; import type {CanvasNumericalBrandTheme} from './types'; /** - * Sana extends the neutral ramp with base-palette steps not yet exported from - * canvas-tokens-web JS. Defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. + * `base.sana` is a pre-built map of `var(--token, fallback)` strings exported by + * canvas-tokens-web specifically for these Sana-only base-palette steps — use them directly, + * not `cssVar()` (which expects a bare variable name, not an already-wrapped `var()` string). + * Referencing `base.*` (not `brand.*` of the same name) avoids a `var()` self-reference cycle: + * CanvasProvider writes each ramp entry onto the identically-named `--cnvs-brand-*` variable, so + * `brand.sana.neutral150` (itself `var(--cnvs-brand-neutral-150, ...)`) would point right back at + * the variable being written. */ -const sanaBaseNeutral = { - '150': '--cnvs-base-palette-neutral-150', - '850': '--cnvs-base-palette-neutral-850', - A150: '--cnvs-base-palette-neutral-a150', - A850: '--cnvs-base-palette-neutral-a850', -} as const; - -/** - * Sana's only distinct step for `primary`/`critical`/`caution`/`positive` — a stronger alpha - * wash (`A300`) on top of the matching base-palette hue. Not yet exported from canvas-tokens-web - * JS; defined in `@workday/canvas-tokens-web/css/sana/_variables.css`. - */ -const sanaBaseAccentA300 = { - primary: '--cnvs-base-palette-blue-a300', - critical: '--cnvs-base-palette-red-a300', - caution: '--cnvs-base-palette-amber-a300', - positive: '--cnvs-base-palette-green-a300', -} as const; +const sanaBaseNeutral = base.sana; /** * Sana Canvas brand tokens for scoped `CanvasProvider` / popup forwarding. @@ -56,9 +44,11 @@ const sanaBaseAccentA300 = { * (a `var()` cycle that resolves to invalid, leaking the classic-theme fallback color instead of * Sana's). * - * The `selected.fg`/`selected.surface` shortcuts (`system.color.brand.fg`/`surface.selected`, - * aliases for `primary.700`/`primary.A50`) are likewise omitted — Sana doesn't redefine those - * ramp steps either, so selected `Menu.Item`/`Menu.Option` state stays on the classic values. + * `selected.fg`/`selected.surface` are forwarded too — they write directly onto + * `--cnvs-sys-color-brand-fg-selected`/`-surface-selected` (independent of the `primary` ramp), + * and Sana keeps selected `Menu.Item`/`Menu.Option` state on its neutral ramp + * (`neutralA900`/`neutralA100`), same as classic. Forwarding them keeps portaled popups in sync + * with in-document Sana styling and matches classic's own behavior — no color change intended. * * `system.color.brand.accent.primary`/`.action` and `.fg.primary.default`/`.strong` **are** * forwarded (via the `system.color.brand.*` escape hatch — see @@ -77,16 +67,16 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { themeScope: 'brand', brand: { primary: { - A300: cssVar(sanaBaseAccentA300.primary), + A300: sanaBaseNeutral.blueA300, }, critical: { - A300: cssVar(sanaBaseAccentA300.critical), + A300: sanaBaseNeutral.redA300, }, caution: { - A300: cssVar(sanaBaseAccentA300.caution), + A300: sanaBaseNeutral.amberA300, }, positive: { - A300: cssVar(sanaBaseAccentA300.positive), + A300: sanaBaseNeutral.greenA300, }, action: { base: cssVar(brand.neutral975), @@ -102,7 +92,7 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { '25': cssVar(base.neutral25), '50': cssVar(base.neutral50), '100': cssVar(base.neutral100), - '150': cssVar(sanaBaseNeutral['150']), + '150': sanaBaseNeutral.neutral150, '200': cssVar(base.neutral200), '300': cssVar(base.neutral300), '400': cssVar(base.neutral400), @@ -110,14 +100,14 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { '600': cssVar(base.neutral600), '700': cssVar(base.neutral700), '800': cssVar(base.neutral800), - '850': cssVar(sanaBaseNeutral['850']), + '850': sanaBaseNeutral.neutral850, '900': cssVar(base.neutral900), '950': cssVar(base.neutral950), '975': cssVar(base.neutral975), A25: cssVar(base.neutralA25), A50: cssVar(base.neutralA50), A100: cssVar(base.neutralA100), - A150: cssVar(sanaBaseNeutral.A150), + A150: sanaBaseNeutral.neutralA150, A200: cssVar(base.neutralA200), A300: cssVar(base.neutralA300), A400: cssVar(base.neutralA400), @@ -125,18 +115,20 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { A600: cssVar(base.neutralA600), A700: cssVar(base.neutralA700), A800: cssVar(base.neutralA800), - A850: cssVar(sanaBaseNeutral.A850), + A850: sanaBaseNeutral.neutralA850, A900: cssVar(base.neutralA900), A950: cssVar(base.neutralA950), A975: cssVar(base.neutralA975), }, }, - // `selected` is intentionally omitted — see the `primary`/`critical`/`caution`/`positive` note - // above. Selected Menu.Item/Menu.Option state falls through to the classic - // `brand.primary.700` / `brand.primary.A50` values, unchanged by Sana. - // - // Unlike `selected`, Sana's stylesheet *does* redefine these four `system.color.brand.*` - // tokens, so forward them for portal parity (see the doc comment above). + // Selected Menu.Item/Menu.Option state — writes directly onto + // `--cnvs-sys-color-brand-fg-selected` / `-surface-selected` (not derived from + // `primary.700`/`primary.A50`, so no var() cycle risk referencing `brand.*` here). + // Sana keeps this on its neutral ramp rather than primary; forwarded for portal parity. + selected: { + fg: cssVar(brand.neutralA900), + surface: cssVar(brand.neutralA100), + }, system: { color: { brand: { @@ -170,8 +162,8 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { * inherit brand variables from the document and no `theme` prop is needed. * * Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / - * `-surface-selected`) is unaffected either way — Sana doesn't redefine `brand.primary`, so - * those resolve to the classic `brand.primary.700` / `.A50` values with or without this preset. + * `-surface-selected`) is forwarded to Sana's neutral values (`neutralA900`/`neutralA100`, + * same as classic) so portaled popups match in-document Sana styling — no color change either way. * * `--cnvs-sys-color-brand-accent-primary`/`-accent-action`/`-fg-primary-default`/ * `-fg-primary-strong` are different: Sana's stylesheet *does* redefine them (to Sana neutral diff --git a/modules/react/common/spec/sanaTheme.spec.ts b/modules/react/common/spec/sanaTheme.spec.ts index 5a030044b8..c5c062ca94 100644 --- a/modules/react/common/spec/sanaTheme.spec.ts +++ b/modules/react/common/spec/sanaTheme.spec.ts @@ -30,10 +30,10 @@ describe('sanaCanvasNumericalTheme', () => { expect(Object.keys(sanaCanvasNumericalTheme.brand?.positive ?? {})).toEqual(['A300']); const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style['--cnvs-brand-primary-a300' as any]).toBe('var(--cnvs-base-palette-blue-a300)'); - expect(style['--cnvs-brand-critical-a300' as any]).toBe('var(--cnvs-base-palette-red-a300)'); - expect(style['--cnvs-brand-caution-a300' as any]).toBe('var(--cnvs-base-palette-amber-a300)'); - expect(style['--cnvs-brand-positive-a300' as any]).toBe('var(--cnvs-base-palette-green-a300)'); + expect(style['--cnvs-brand-primary-a300' as any]).toBe(base.sana.blueA300); + expect(style['--cnvs-brand-critical-a300' as any]).toBe(base.sana.redA300); + expect(style['--cnvs-brand-caution-a300' as any]).toBe(base.sana.amberA300); + expect(style['--cnvs-brand-positive-a300' as any]).toBe(base.sana.greenA300); }); it('writes brand tokens when passed to canvasThemeToCssVars', () => { @@ -45,10 +45,10 @@ describe('sanaCanvasNumericalTheme', () => { it('writes Sana extended neutral ramp keys from the base palette', () => { const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style['--cnvs-brand-neutral-150' as any]).toBe('var(--cnvs-base-palette-neutral-150)'); - expect(style['--cnvs-brand-neutral-850' as any]).toBe('var(--cnvs-base-palette-neutral-850)'); - expect(style['--cnvs-brand-neutral-a150' as any]).toBe('var(--cnvs-base-palette-neutral-a150)'); - expect(style['--cnvs-brand-neutral-a850' as any]).toBe('var(--cnvs-base-palette-neutral-a850)'); + expect(style['--cnvs-brand-neutral-150' as any]).toBe(base.sana.neutral150); + expect(style['--cnvs-brand-neutral-850' as any]).toBe(base.sana.neutral850); + expect(style['--cnvs-brand-neutral-a150' as any]).toBe(base.sana.neutralA150); + expect(style['--cnvs-brand-neutral-a850' as any]).toBe(base.sana.neutralA850); }); it('writes the full neutral alpha ramp through A975 (not just up to A200)', () => { @@ -63,11 +63,12 @@ describe('sanaCanvasNumericalTheme', () => { expect(style[brand.action.darker as any]).toBe(`var(${brand.neutral975})`); }); - it('does not write selected.fg/selected.surface shortcuts (no distinct Sana primary values)', () => { - expect(sanaCanvasNumericalTheme.selected).toBeUndefined(); + it('forwards selected.fg/selected.surface using Sana neutral values (no color change from classic)', () => { const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style['--cnvs-sys-color-brand-fg-selected' as any]).toBeUndefined(); - expect(style['--cnvs-sys-color-brand-surface-selected' as any]).toBeUndefined(); + expect(style['--cnvs-sys-color-brand-fg-selected' as any]).toBe(`var(${brand.neutralA900})`); + expect(style['--cnvs-sys-color-brand-surface-selected' as any]).toBe( + `var(${brand.neutralA100})` + ); }); it('forwards accent.primary/action and fg.primary.default/strong for portal parity', () => { diff --git a/modules/react/common/stories/mdx/Theming.mdx b/modules/react/common/stories/mdx/Theming.mdx index 9b7fd8c586..ad6411bfea 100644 --- a/modules/react/common/stories/mdx/Theming.mdx +++ b/modules/react/common/stories/mdx/Theming.mdx @@ -119,8 +119,8 @@ import {CanvasProvider} from '@workday/canvas-kit-react/common'; ``` Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / -`-surface-selected`) is unaffected by Sana either way: Sana doesn't redefine `brand.primary`, so -those variables keep resolving to the classic `brand.primary.700` / `.A50` values whether or not +`-surface-selected`) uses Sana's neutral ramp (`neutralA900`/`neutralA100`), the same values as +classic Canvas — selection styling doesn't change under Sana, whether or not `data-theme="sana-canvas"` is set. **Scoped / no document-root control:** if you cannot set `data-theme` on `` (embedded apps, diff --git a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx index 76b01a1229..d0cd130983 100644 --- a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx +++ b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import {PrimaryButton, SecondaryButton} from '@workday/canvas-kit-react/button'; +import {PrimaryButton} from '@workday/canvas-kit-react/button'; import {CanvasProvider, sanaCanvasProviderTheme} from '@workday/canvas-kit-react/common'; import {Menu} from '@workday/canvas-kit-react/menu'; import {Popup, useCloseOnOutsideClick, usePopupModel} from '@workday/canvas-kit-react/popup'; @@ -21,9 +21,7 @@ export const SimplifiedSetup = () => { - - Option 1 - + Option 1 Option 2 Option 3 diff --git a/modules/react/popup/spec/usePopupStack.spec.tsx b/modules/react/popup/spec/usePopupStack.spec.tsx index 94a3168f11..e3aba06bdd 100644 --- a/modules/react/popup/spec/usePopupStack.spec.tsx +++ b/modules/react/popup/spec/usePopupStack.spec.tsx @@ -2,6 +2,7 @@ import {renderHook, waitFor} from '@testing-library/react'; import React from 'react'; import {CanvasProvider, sanaCanvasProviderTheme} from '@workday/canvas-kit-react/common'; +import {base, brand} from '@workday/canvas-tokens-web'; import {usePopupStack} from '../lib/hooks/usePopupStack'; @@ -64,11 +65,9 @@ describe('usePopupStack', () => { // var() self-reference cycle. Neutral/action ramps and system.color.brand.* // overrides are forwarded as var() references to the underlying palette. expect(styles.getPropertyValue('--cnvs-brand-primary-600')).toBe(''); - expect(styles.getPropertyValue('--cnvs-brand-primary-a300')).toBe( - 'var(--cnvs-base-palette-blue-a300)' - ); + expect(styles.getPropertyValue('--cnvs-brand-primary-a300')).toBe(base.sana.blueA300); expect(styles.getPropertyValue('--cnvs-brand-neutral-600')).toBe( - 'var(--cnvs-base-palette-neutral-600)' + `var(${base.neutral600})` ); expect(styles.getPropertyValue('--cnvs-brand-action-base')).toBe( 'var(--cnvs-brand-neutral-975)' @@ -76,6 +75,14 @@ describe('usePopupStack', () => { expect(styles.getPropertyValue('--cnvs-sys-color-brand-accent-primary')).toBe( 'var(--cnvs-brand-neutral-975)' ); + // Selected Menu.Item/Menu.Option state doesn't change under Sana — same neutral + // values as classic, forwarded for portal parity. + expect(styles.getPropertyValue('--cnvs-sys-color-brand-fg-selected')).toBe( + `var(${brand.neutralA900})` + ); + expect(styles.getPropertyValue('--cnvs-sys-color-brand-surface-selected')).toBe( + `var(${brand.neutralA100})` + ); } }); }); From 3d6275858e66c28635f9702aa41d4f374684adea Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Mon, 17 Aug 2026 13:36:21 -0600 Subject: [PATCH 6/6] fix: Update theme --- modules/react/common/lib/CanvasProvider.tsx | 27 ++++++- modules/react/common/lib/theming/sanaTheme.ts | 75 +++++++------------ modules/react/common/spec/sanaTheme.spec.ts | 23 +++--- modules/react/common/stories/mdx/Theming.mdx | 13 ++-- .../mdx/examples/SimplifiedSanaSetup.tsx | 2 + .../react/popup/lib/hooks/usePopupStack.ts | 24 +++++- .../react/popup/spec/usePopupStack.spec.tsx | 65 ++++++++++++---- 7 files changed, 143 insertions(+), 86 deletions(-) diff --git a/modules/react/common/lib/CanvasProvider.tsx b/modules/react/common/lib/CanvasProvider.tsx index 4d53d6df26..d3dbc1b6f4 100644 --- a/modules/react/common/lib/CanvasProvider.tsx +++ b/modules/react/common/lib/CanvasProvider.tsx @@ -31,6 +31,18 @@ import {sanaCanvasProviderTheme} from './theming/sanaTheme'; */ export const CanvasBrandStyleContext = React.createContext({}); +/** + * Context for providing the `data-theme` value to popup containers. + * + * Token stylesheets scope their variables to an attribute selector (e.g. Sana's + * `[data-theme="sana-canvas"]` block defines ~300 variables — palette, shape, depth, type). + * Portaled popups render under `document.body`, outside the `CanvasProvider` wrapper, so they + * never inherit a nested `data-theme` and none of those variables resolve. Forwarding the + * attribute itself lets the popup container match the same selector, so the whole theme applies + * through normal cascade — rather than trying to mirror every variable as an inline style. + */ +export const CanvasThemeAttributeContext = React.createContext(undefined); + export interface CanvasProviderProps { /** * ⚠️ Only use this prop if you intent to to theme a part of your application that is different from global theming. @@ -222,6 +234,13 @@ export const CanvasProvider = ({ // Read parent context to support nested scoped providers const parentBrandStyle = React.useContext(CanvasBrandStyleContext); + const parentThemeAttribute = React.useContext(CanvasThemeAttributeContext); + + // `data-theme` on this provider wins; otherwise inherit from a parent provider so nested + // providers keep forwarding the outer theme attribute to popups. + const themeAttribute = + (props as React.HTMLAttributes & {'data-theme'?: string})['data-theme'] ?? + parentThemeAttribute; // Popup forwarding only needs CSS custom properties (not consumer layout styles). const mergedBrandStyle = React.useMemo(() => { @@ -256,9 +275,11 @@ export const CanvasProvider = ({ ); const wrappedContent = ( - - {content} - + + + {content} + + ); return ( diff --git a/modules/react/common/lib/theming/sanaTheme.ts b/modules/react/common/lib/theming/sanaTheme.ts index cdcda53cce..b7ecf961ce 100644 --- a/modules/react/common/lib/theming/sanaTheme.ts +++ b/modules/react/common/lib/theming/sanaTheme.ts @@ -35,32 +35,28 @@ const sanaBaseNeutral = base.sana; * Sana Canvas brand tokens for scoped `CanvasProvider` / popup forwarding. * Values are `var()` references to Sana brand variables — not merged from `defaultCanvasTheme`. * - * `action` and `neutral` are fully populated: Sana's palette is neutral/monochrome-driven, so - * `action.*` reads directly from the `neutral` ramp (see - * `@workday/canvas-tokens-web/css/sana/_variables.css`). `primary`/`critical`/`caution`/`positive` - * only set `A300` — the one step Sana actually redefines for those families (a stronger alpha - * wash on the matching hue) — every other key is intentionally omitted: Sana does not define a - * distinct value for it, so writing it here would only reference the very variable being written - * (a `var()` cycle that resolves to invalid, leaking the classic-theme fallback color instead of - * Sana's). + * Mirrors only what Sana's stylesheet itself defines: the `neutral` ramp, the `A300` step of + * `primary`/`critical`/`caution`/`positive` (the one step Sana redefines for those families — a + * stronger alpha wash on the matching hue), and the four `system.color.brand.*` tokens Sana + * overrides. Every other key is intentionally omitted — Sana does not define a distinct value + * for it, so writing it here would only reference the very variable being written (a `var()` + * cycle that resolves to invalid, leaking the classic-theme fallback color instead of Sana's). * - * `selected.fg`/`selected.surface` are forwarded too — they write directly onto - * `--cnvs-sys-color-brand-fg-selected`/`-surface-selected` (independent of the `primary` ramp), - * and Sana keeps selected `Menu.Item`/`Menu.Option` state on its neutral ramp - * (`neutralA900`/`neutralA100`), same as classic. Forwarding them keeps portaled popups in sync - * with in-document Sana styling and matches classic's own behavior — no color change intended. - * - * `system.color.brand.accent.primary`/`.action` and `.fg.primary.default`/`.strong` **are** - * forwarded (via the `system.color.brand.*` escape hatch — see - * {@link CanvasNumericalBrandTheme.system}) — unlike `selected`, Sana's stylesheet does - * redefine these four, to `brand.neutral.975` / `.A900` / `.A950`, so portaled popups need the - * override too for parity with in-document Sana styling. + * `action.*` and `selected.*` are deliberately **not** set. Sana's stylesheet does not define + * `--cnvs-brand-action-*` or `--cnvs-sys-color-brand-fg-selected`/`-surface-selected`; those are + * derived downstream from the root theme. Forcing them here would pin values the root is meant + * to own. * * Ramp values must reference `base.*` (the underlying palette), never `brand.*` of the same * name — CanvasProvider writes each entry onto the identically-named `--cnvs-brand-*` CSS * variable, so referencing `brand.*` here would create that same self-reference cycle. The - * `system.color.brand.*` overrides above are the exception: they target *different* CSS - * variables (`--cnvs-sys-color-brand-*`) than the `brand.*` values they reference, so no cycle. + * `system.color.brand.*` overrides are the exception: they target *different* CSS variables + * (`--cnvs-sys-color-brand-*`) than the `brand.*` values they reference, so no cycle. + * + * Note this preset only covers brand tokens. The rest of Sana (shape, depth, type, non-brand + * system colors) comes from the stylesheet's `[data-theme="sana-canvas"]` block — pass + * `data-theme` to `CanvasProvider` alongside this preset and it is forwarded to portaled popups + * so the full theme applies there too. */ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { // Explicit brand vars only — multi-key ramps write 1:1; no system shortcut bundles run. @@ -78,16 +74,6 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { positive: { A300: sanaBaseNeutral.greenA300, }, - action: { - base: cssVar(brand.neutral975), - dark: cssVar(brand.neutral950), - darkest: cssVar(brand.neutral900), - darker: cssVar(brand.neutral975), - accent: cssVar(base.neutral0), - lightest: cssVar(brand.neutral25), - lighter: cssVar(brand.neutral50), - light: cssVar(brand.neutral200), - }, neutral: { '25': cssVar(base.neutral25), '50': cssVar(base.neutral50), @@ -121,14 +107,6 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { A975: cssVar(base.neutralA975), }, }, - // Selected Menu.Item/Menu.Option state — writes directly onto - // `--cnvs-sys-color-brand-fg-selected` / `-surface-selected` (not derived from - // `primary.700`/`primary.A50`, so no var() cycle risk referencing `brand.*` here). - // Sana keeps this on its neutral ramp rather than primary; forwarded for portal parity. - selected: { - fg: cssVar(brand.neutralA900), - surface: cssVar(brand.neutralA100), - }, system: { color: { brand: { @@ -161,14 +139,13 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { * - Prefer setting `data-theme="sana-canvas"` on `` with Sana CSS imported. Popups then * inherit brand variables from the document and no `theme` prop is needed. * - * Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / - * `-surface-selected`) is forwarded to Sana's neutral values (`neutralA900`/`neutralA100`, - * same as classic) so portaled popups match in-document Sana styling — no color change either way. + * Pass `data-theme="sana-canvas"` alongside it. That attribute is forwarded to popup stack + * containers, so the rest of Sana (shape, depth, type, non-brand system colors) applies to + * portaled content through the stylesheet's own `[data-theme="sana-canvas"]` block — this preset + * covers brand tokens, the attribute covers everything else. * - * `--cnvs-sys-color-brand-accent-primary`/`-accent-action`/`-fg-primary-default`/ - * `-fg-primary-strong` are different: Sana's stylesheet *does* redefine them (to Sana neutral - * tones), so without this preset a popup outside `data-theme="sana-canvas"`'s reach falls back - * to classic primary-derived colors for those four, out of step with the rest of a Sana UI. + * `action.*` and selected-state tokens are not set by this preset — Sana does not define them, + * and they resolve from the root theme. * * @example * ```tsx @@ -177,8 +154,10 @@ export const sanaCanvasNumericalTheme: CanvasNumericalBrandTheme = { * // * * - * // No access to — required for popup parity - * + * // No access to — pass both for full parity, including portaled popups + * + * + * * ``` */ export const sanaCanvasProviderTheme = sanaCanvasNumericalTheme; diff --git a/modules/react/common/spec/sanaTheme.spec.ts b/modules/react/common/spec/sanaTheme.spec.ts index c5c062ca94..ea501a2402 100644 --- a/modules/react/common/spec/sanaTheme.spec.ts +++ b/modules/react/common/spec/sanaTheme.spec.ts @@ -7,8 +7,6 @@ import {sanaCanvasNumericalTheme, sanaCanvasProviderTheme} from '../lib/theming/ describe('sanaCanvasNumericalTheme', () => { it('references Sana base-palette CSS variables instead of defaultCanvasTheme literals', () => { expect(sanaCanvasNumericalTheme.brand?.neutral?.['600']).toBe(`var(${base.neutral600})`); - expect(sanaCanvasNumericalTheme.brand?.action?.base).toBe(`var(${brand.neutral975})`); - expect(sanaCanvasNumericalTheme.brand?.action?.accent).toBe(`var(${base.neutral0})`); expect(sanaCanvasNumericalTheme.brand?.neutral?.['600']).not.toBe( defaultCanvasTheme.palette.neutral.main ); @@ -40,7 +38,6 @@ describe('sanaCanvasNumericalTheme', () => { const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); expect(Object.keys(style).length).toBeGreaterThan(0); expect(style[brand.neutral600 as any]).toBe(`var(${base.neutral600})`); - expect(style[brand.action.base as any]).toBe(`var(${brand.neutral975})`); }); it('writes Sana extended neutral ramp keys from the base palette', () => { @@ -58,22 +55,24 @@ describe('sanaCanvasNumericalTheme', () => { expect(style[brand.neutralA975 as any]).toBe(`var(${base.neutralA975})`); }); - it('writes action.darker alongside the rest of the action bundle', () => { + it('does not write brand.action.* — Sana does not define it; it derives from the root theme', () => { + expect(sanaCanvasNumericalTheme.brand?.action).toBeUndefined(); const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style[brand.action.darker as any]).toBe(`var(${brand.neutral975})`); + expect(style[brand.action.base as any]).toBeUndefined(); + expect(style[brand.action.darker as any]).toBeUndefined(); + expect(style[brand.action.accent as any]).toBeUndefined(); }); - it('forwards selected.fg/selected.surface using Sana neutral values (no color change from classic)', () => { + it('does not write selected.* — Sana does not define it; it derives from the root theme', () => { + expect(sanaCanvasNumericalTheme.selected).toBeUndefined(); const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); - expect(style['--cnvs-sys-color-brand-fg-selected' as any]).toBe(`var(${brand.neutralA900})`); - expect(style['--cnvs-sys-color-brand-surface-selected' as any]).toBe( - `var(${brand.neutralA100})` - ); + expect(style['--cnvs-sys-color-brand-fg-selected' as any]).toBeUndefined(); + expect(style['--cnvs-sys-color-brand-surface-selected' as any]).toBeUndefined(); }); it('forwards accent.primary/action and fg.primary.default/strong for portal parity', () => { - // Unlike `selected`, Sana's stylesheet redefines these four `system.color.brand.*` - // tokens, so popups outside `[data-theme="sana-canvas"]`'s reach need the override too. + // Sana's stylesheet redefines these four `system.color.brand.*` tokens, so popups outside + // `[data-theme="sana-canvas"]`'s reach need the override too. const {style} = canvasThemeToCssVars(sanaCanvasProviderTheme, {}); expect(style['--cnvs-sys-color-brand-accent-primary' as any]).toBe(`var(${brand.neutral975})`); expect(style['--cnvs-sys-color-brand-accent-action' as any]).toBe(`var(${brand.neutral975})`); diff --git a/modules/react/common/stories/mdx/Theming.mdx b/modules/react/common/stories/mdx/Theming.mdx index ad6411bfea..4a0d882b98 100644 --- a/modules/react/common/stories/mdx/Theming.mdx +++ b/modules/react/common/stories/mdx/Theming.mdx @@ -118,15 +118,12 @@ import {CanvasProvider} from '@workday/canvas-kit-react/common'; ``` -Selected `Menu.Item`/`Menu.Option` state (`--cnvs-sys-color-brand-fg-selected` / -`-surface-selected`) uses Sana's neutral ramp (`neutralA900`/`neutralA100`), the same values as -classic Canvas — selection styling doesn't change under Sana, whether or not -`data-theme="sana-canvas"` is set. - **Scoped / no document-root control:** if you cannot set `data-theme` on `` (embedded apps, -microfrontends, third-party shells), a nested `data-theme` alone does **not** reach portaled -popups. Pass both `data-theme="sana-canvas"` (for in-tree UI) and `sanaCanvasProviderTheme` (so -Canvas Kit forwards Sana brand variables onto the popup stack container): +microfrontends, third-party shells), pass both `data-theme="sana-canvas"` and +`sanaCanvasProviderTheme` to `CanvasProvider`. The preset supplies Sana's brand variables, and +Canvas Kit forwards the `data-theme` attribute onto the popup stack container — so portaled +menus, modals, and dialogs match the same `[data-theme="sana-canvas"]` selector and pick up the +rest of the theme (shape, depth, type, non-brand system colors) through normal cascade: ```tsx import {CanvasProvider, sanaCanvasProviderTheme} from '@workday/canvas-kit-react/common'; diff --git a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx index d0cd130983..8b78bf32e8 100644 --- a/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx +++ b/modules/react/common/stories/mdx/examples/SimplifiedSanaSetup.tsx @@ -4,6 +4,7 @@ import {PrimaryButton} from '@workday/canvas-kit-react/button'; import {CanvasProvider, sanaCanvasProviderTheme} from '@workday/canvas-kit-react/common'; import {Menu} from '@workday/canvas-kit-react/menu'; import {Popup, useCloseOnOutsideClick, usePopupModel} from '@workday/canvas-kit-react/popup'; +import {TextInput} from '@workday/canvas-kit-react/text-input'; /** * Scoped Sana setup for popup parity: `data-theme` themes the in-tree UI, and @@ -27,6 +28,7 @@ export const SimplifiedSetup = () => { Hello World + diff --git a/modules/react/popup/lib/hooks/usePopupStack.ts b/modules/react/popup/lib/hooks/usePopupStack.ts index 5a8efbfebd..e8d4458708 100644 --- a/modules/react/popup/lib/hooks/usePopupStack.ts +++ b/modules/react/popup/lib/hooks/usePopupStack.ts @@ -1,7 +1,12 @@ import React from 'react'; import {PopupStack} from '@workday/canvas-kit-popup-stack'; -import {CanvasBrandStyleContext, isElementRTL, useLocalRef} from '@workday/canvas-kit-react/common'; +import { + CanvasBrandStyleContext, + CanvasThemeAttributeContext, + isElementRTL, + useLocalRef, +} from '@workday/canvas-kit-react/common'; /** * **Note:** If you're using {@link Popper}, you do not need to use this hook directly. @@ -53,6 +58,7 @@ export const usePopupStack = ( // Read brand style from the context provided by CanvasProvider const style = React.useContext(CanvasBrandStyleContext); + const themeAttribute = React.useContext(CanvasThemeAttributeContext); const firstLoadRef = React.useRef(true); // React 19 can call a useState more than once, so we need to track if we've already created a container // useState function input ensures we only create a container once. @@ -90,6 +96,22 @@ export const usePopupStack = ( return undefined; }, [localRef, style]); + // Forward `data-theme` so the popup container matches the same attribute selector the token + // stylesheet scopes its variables to (e.g. `[data-theme="sana-canvas"]`). Portals render under + // `document.body`, outside the CanvasProvider wrapper, so without this none of those variables + // resolve — and unlike the inline styles above, this covers the *whole* theme (palette, shape, + // depth, type), not just the brand vars the theme object enumerates. Runs before PopupStack.add + // to avoid a theme flash. + React.useLayoutEffect(() => { + const element = localRef.current; + if (!element || !themeAttribute) { + return undefined; + } + element.setAttribute('data-theme', themeAttribute); + // No cleanup: leave theme on container so reopening doesn't flash + return undefined; + }, [localRef, themeAttribute]); + // We useLayoutEffect to ensure proper timing of registration of the element to the popup stack. // Without this, the timing is unpredictable when mixed with other frameworks. Other frameworks // should also register as soon as the element is available diff --git a/modules/react/popup/spec/usePopupStack.spec.tsx b/modules/react/popup/spec/usePopupStack.spec.tsx index e3aba06bdd..eebb8ff76b 100644 --- a/modules/react/popup/spec/usePopupStack.spec.tsx +++ b/modules/react/popup/spec/usePopupStack.spec.tsx @@ -2,7 +2,7 @@ import {renderHook, waitFor} from '@testing-library/react'; import React from 'react'; import {CanvasProvider, sanaCanvasProviderTheme} from '@workday/canvas-kit-react/common'; -import {base, brand} from '@workday/canvas-tokens-web'; +import {base} from '@workday/canvas-tokens-web'; import {usePopupStack} from '../lib/hooks/usePopupStack'; @@ -62,31 +62,68 @@ describe('usePopupStack', () => { const styles = container?.style; if (styles) { // Sana only redefines primary A300 (not 600) — writing primary-600 would be a - // var() self-reference cycle. Neutral/action ramps and system.color.brand.* - // overrides are forwarded as var() references to the underlying palette. + // var() self-reference cycle. The neutral ramp and system.color.brand.* overrides + // are forwarded as var() references to the underlying palette. expect(styles.getPropertyValue('--cnvs-brand-primary-600')).toBe(''); expect(styles.getPropertyValue('--cnvs-brand-primary-a300')).toBe(base.sana.blueA300); expect(styles.getPropertyValue('--cnvs-brand-neutral-600')).toBe( `var(${base.neutral600})` ); - expect(styles.getPropertyValue('--cnvs-brand-action-base')).toBe( - 'var(--cnvs-brand-neutral-975)' - ); expect(styles.getPropertyValue('--cnvs-sys-color-brand-accent-primary')).toBe( 'var(--cnvs-brand-neutral-975)' ); - // Selected Menu.Item/Menu.Option state doesn't change under Sana — same neutral - // values as classic, forwarded for portal parity. - expect(styles.getPropertyValue('--cnvs-sys-color-brand-fg-selected')).toBe( - `var(${brand.neutralA900})` - ); - expect(styles.getPropertyValue('--cnvs-sys-color-brand-surface-selected')).toBe( - `var(${brand.neutralA100})` - ); + // `action` and selected-state tokens are not set by the preset — Sana doesn't define + // them, so they resolve from the root theme rather than being pinned here. + expect(styles.getPropertyValue('--cnvs-brand-action-base')).toBe(''); + expect(styles.getPropertyValue('--cnvs-sys-color-brand-fg-selected')).toBe(''); + expect(styles.getPropertyValue('--cnvs-sys-color-brand-surface-selected')).toBe(''); } }); }); + it('should forward data-theme to the popup container so scoped token CSS applies', async () => { + // Token stylesheets scope variables to `[data-theme="..."]`. Portals render outside the + // CanvasProvider wrapper, so the attribute must be copied for the whole theme (palette, + // shape, depth, type) to resolve — inline brand vars alone don't cover it. + const wrapper = ({children}: {children: React.ReactNode}) => ( + + {children} + + ); + + const {result} = renderHook(() => usePopupStack(), {wrapper}); + + await waitFor(() => { + expect(result.current.current?.getAttribute('data-theme')).toBe('sana-canvas'); + }); + }); + + it('should not set data-theme on the popup container when the provider has none', async () => { + const wrapper = ({children}: {children: React.ReactNode}) => ( + {children} + ); + + const {result} = renderHook(() => usePopupStack(), {wrapper}); + + await waitFor(() => { + expect(result.current.current?.hasAttribute('data-theme')).toBe(false); + }); + }); + + it('should inherit data-theme from an outer provider through nested providers', async () => { + const wrapper = ({children}: {children: React.ReactNode}) => ( + + {children} + + ); + + const {result} = renderHook(() => usePopupStack(), {wrapper}); + + await waitFor(() => { + expect(result.current.current?.getAttribute('data-theme')).toBe('sana-canvas'); + }); + }); + it('should forward legacy palette theme CSS variables to popup container', async () => { const legacyTheme = { canvas: {