From 8f4c166701c81d37550edb9d96397438e79ebc49 Mon Sep 17 00:00:00 2001 From: AK <144495202+AKnassa@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:17:04 -0400 Subject: [PATCH 1/2] feat(ComplexSelector): popup surface theme target The popup surface had no stable class: contentXstyle is StyleX-typed and the surface element carried only hashed atomic classes, so non-StyleX consumers could not style the popup's background, border, radius, or width at all (route A of the RFC). The popup content container now paints the surface itself (usePopover hasSurface: false + the exported popoverSurfaceStyles, same tokens) and carries themeProps('complex-selector-popup'), so defineTheme components and plain stylesheets reach the element that owns the surface paint, and contentXstyle can override it too. Rendered defaults unchanged. Refs #4804 --- .../complex-selector-popup-theme-target.md | 7 + .../stories/ComplexSelector.stories.tsx | 68 +++++++ .../ComplexSelector/ComplexSelector.doc.mjs | 4 +- .../ComplexSelector/ComplexSelector.test.tsx | 171 ++++++++++++++++++ .../src/ComplexSelector/ComplexSelector.tsx | 26 ++- packages/core/src/Popover/usePopover.tsx | 17 +- 6 files changed, 284 insertions(+), 9 deletions(-) create mode 100644 .changeset/complex-selector-popup-theme-target.md diff --git a/.changeset/complex-selector-popup-theme-target.md b/.changeset/complex-selector-popup-theme-target.md new file mode 100644 index 000000000000..cfd00e7452d0 --- /dev/null +++ b/.changeset/complex-selector-popup-theme-target.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] ComplexSelector: expose the popup surface as the `astryx-complex-selector-popup` theme target. The popup content container now paints the surface itself (same background, radius, and shadow tokens as before), so `defineTheme` components — or any plain stylesheet — can restyle the popup's background, border, radius, and width, and `contentXstyle` can override the surface styles too. Rendered defaults are unchanged. + +@AKnassa diff --git a/apps/storybook/stories/ComplexSelector.stories.tsx b/apps/storybook/stories/ComplexSelector.stories.tsx index 797fb63beac0..f9f2fe016425 100644 --- a/apps/storybook/stories/ComplexSelector.stories.tsx +++ b/apps/storybook/stories/ComplexSelector.stories.tsx @@ -10,6 +10,7 @@ import {TextInput} from '@astryxdesign/core/TextInput'; import {HStack, VStack} from '@astryxdesign/core/Layout'; import {Token} from '@astryxdesign/core/Token'; import {TreeList, type TreeListItemData} from '@astryxdesign/core/TreeList'; +import {Theme, defineTheme} from '@astryxdesign/core/theme'; import {useGridFocus} from '@astryxdesign/core/hooks'; import { borderVars, @@ -728,3 +729,70 @@ export const CategoryTreeSelector: Story = { }, }, }; + +/** + * Theme the popup surface via `defineTheme`. + * + * The popup container carries the `astryx-complex-selector-popup` theme + * target and paints the surface itself, so a theme — or any plain + * stylesheet — can restyle the popup's background, border, radius, and + * width without StyleX, the popup counterpart of the trigger's `className` + * route (#4804). Defaults are unchanged; this story only demonstrates the + * override channel. + */ +const popupTheme = defineTheme({ + name: 'complex-selector-popup-demo', + components: { + 'complex-selector-popup': { + base: { + backgroundColor: 'var(--color-background-muted)', + borderWidth: '1px', + borderStyle: 'solid', + borderColor: 'var(--color-border)', + boxShadow: 'none', + }, + }, + }, +}); + +export const ThemedPopupSurface: Story = { + name: 'Themed popup surface', + render: () => { + const [value, setValue] = useState({ + fruit: 'Apple', + ripeness: 'Juicy', + }); + + return ( + + + + label="Fruit blend" + description="The popup renders as a flat, bordered, muted panel through the astryx-complex-selector-popup target." + value={value} + onChange={setValue} + triggerLabel={formatFruitValue(value)} + contentXstyle={styles.fruitContent}> + {(selectedValue, onChange, close) => ( + { + onChange(nextValue); + close(); + }} + /> + )} + + + + ); + }, + parameters: { + docs: { + description: { + story: + 'Apps whose menus are bordered flat panels can match the popup to them with defineTheme alone — no StyleX in the consuming app. The border, background, and shadow land on the element that paints the popup surface.', + }, + }, + }, +}; diff --git a/packages/core/src/ComplexSelector/ComplexSelector.doc.mjs b/packages/core/src/ComplexSelector/ComplexSelector.doc.mjs index a1a52f1efa8a..6476eef4e2bc 100644 --- a/packages/core/src/ComplexSelector/ComplexSelector.doc.mjs +++ b/packages/core/src/ComplexSelector/ComplexSelector.doc.mjs @@ -20,6 +20,7 @@ export const docs = { theming: { targets: [ {className: 'astryx-complex-selector', visualProps: ['size', 'status']}, + {className: 'astryx-complex-selector-popup'}, { className: 'astryx-complex-selector-indicator-icon', states: ['state'], @@ -109,7 +110,8 @@ export const docs = { { name: 'contentXstyle', type: 'StyleXStyles', - description: 'StyleX styles for the popup content container.', + description: + 'StyleX styles for the popup content container. The container is the popup surface (it carries the astryx-complex-selector-popup theme target), so these can override background, border, radius, and shadow as well as padding.', }, ], }, diff --git a/packages/core/src/ComplexSelector/ComplexSelector.test.tsx b/packages/core/src/ComplexSelector/ComplexSelector.test.tsx index 7e0cd6200343..0e0961caa861 100644 --- a/packages/core/src/ComplexSelector/ComplexSelector.test.tsx +++ b/packages/core/src/ComplexSelector/ComplexSelector.test.tsx @@ -12,7 +12,11 @@ import {describe, expect, it, vi} from 'vitest'; import {render, screen, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import * as stylex from '@stylexjs/stylex'; import {ComplexSelector} from './ComplexSelector'; +import {colorVars} from '../theme/tokens.stylex'; +import {defineTheme} from '../theme/defineTheme'; +import {generateThemeRules} from '../theme/generateThemeRules'; type FruitValue = { fruit: 'Apple' | 'Banana'; @@ -153,4 +157,171 @@ describe('ComplexSelector', () => { await user.click(screen.getByRole('button', {name: 'Done', ...h})); expect(trigger).toHaveAttribute('aria-expanded', 'false'); }); + + // =========================================================================== + // Popup theme target (#4804) + // =========================================================================== + + describe('popup theme target', () => { + // The trigger container publishes `astryx-complex-selector`, but the popup + // rendered only anonymous hashed classes: no stable class on the surface, + // so neither defineTheme components nor a plain stylesheet could reach its + // background, border, radius, or width. + + const fruitValue: FruitValue = {fruit: 'Apple', ripeness: 'Ripe'}; + + async function openPopup( + user: ReturnType, + contentXstyle?: stylex.StyleXStyles, + ) { + render( + {}} + contentXstyle={contentXstyle} + triggerLabel="Apple Ripe"> + {value => {}} />} + , + ); + await user.click(screen.getByRole('button', {name: 'Fruit blend'})); + return document.querySelector('.astryx-complex-selector-popup'); + } + + // Collect every injected CSS rule (StyleX runtime injection is enabled in + // vitest), so assertions read the real declarations behind the popup's + // atomic classes instead of hashed class names. + function injectedCss(): string { + let out = ''; + for (const sheet of Array.from(document.styleSheets)) { + try { + for (const rule of Array.from(sheet.cssRules)) { + out += rule.cssText + '\n'; + } + } catch { + // ignore cross-origin sheets + } + } + out += Array.from(document.querySelectorAll('style')) + .map(s => s.textContent || '') + .join('\n'); + return out; + } + + // True when one of el's StyleX atomic classes declares `property` + // (optionally with a specific value fragment). + function declares( + css: string, + el: Element, + property: string, + value?: string, + ): boolean { + return Array.from(el.classList) + .filter(c => c.startsWith('x')) + .some(c => { + const rules = css.match(new RegExp(`\\.${c}[^{]*\\{[^}]*\\}`, 'g')); + return (rules ?? []).some( + rule => + rule.includes(property) && + (value == null || rule.includes(value)), + ); + }); + } + + it('stamps the stable popup class on the popup content container', async () => { + const user = userEvent.setup(); + const popup = await openPopup(user); + + expect(popup).not.toBeNull(); + // The classed element is the container the trigger controls… + expect(popup).toHaveAttribute( + 'id', + screen + .getByRole('button', {name: 'Fruit blend'}) + .getAttribute('aria-controls'), + ); + // …and the custom content renders inside it. + expect( + popup!.contains( + screen.getByRole('grid', {name: 'Fruit blend choices', ...h}), + ), + ).toBe(true); + }); + + it('paints the popup surface on the classed element', async () => { + const user = userEvent.setup(); + const popup = await openPopup(user); + expect(popup).not.toBeNull(); + const css = injectedCss(); + + // The stable-classed element owns the surface paint… + expect(declares(css, popup!, 'background-color')).toBe(true); + expect(declares(css, popup!, 'border-radius')).toBe(true); + expect(declares(css, popup!, 'box-shadow')).toBe(true); + + // …and the dialog wrapper above it paints no second surface behind, so + // a theme override genuinely replaces the surface instead of floating + // over a differently-shaped default. + const dialog = screen.getByRole('dialog', {name: 'Fruit blend', ...h}); + expect(popup!.parentElement).toBe(dialog); + expect(declares(css, dialog, 'background-color')).toBe(false); + expect(declares(css, dialog, 'box-shadow')).toBe(false); + }); + + it('lets contentXstyle override the surface paint', async () => { + // The StyleX escape hatch gains the same reach: with the surface on the + // popup element itself, contentXstyle merges after the surface styles + // and can replace them. + const overrides = stylex.create({ + surface: {backgroundColor: colorVars['--color-background-surface']}, + }); + const user = userEvent.setup(); + const popup = await openPopup(user, overrides.surface); + expect(popup).not.toBeNull(); + const css = injectedCss(); + + expect( + declares(css, popup!, 'background-color', '--color-background-surface'), + ).toBe(true); + // StyleX merge dedupes by property, so the default surface background + // is gone rather than merely covered. + expect( + declares(css, popup!, 'background-color', '--color-background-popover'), + ).toBe(false); + }); + + it('keeps the existing trigger and indicator targets intact', async () => { + // Guard, not red proof: both targets exist before this change too. + const user = userEvent.setup(); + await openPopup(user); + + expect(document.querySelector('.astryx-complex-selector')).not.toBeNull(); + expect( + document.querySelector('.astryx-complex-selector-indicator-icon'), + ).not.toBeNull(); + }); + + it('emits theme CSS for the popup target via defineTheme', () => { + // Guard for the documented route (defineTheme emits for any target + // class): the issue's exact use case — a bordered, fixed-width panel. + const theme = defineTheme({ + name: 'complex-selector-popup-test', + components: { + 'complex-selector-popup': { + base: { + borderWidth: '1px', + borderStyle: 'solid', + borderColor: 'var(--color-border)', + inlineSize: '288px', + }, + }, + }, + }); + const css = generateThemeRules(theme).join('\n'); + + expect(css).toContain('.astryx-complex-selector-popup'); + expect(css).toContain('border-width: 1px'); + expect(css).toContain('inline-size: 288px'); + }); + }); }); diff --git a/packages/core/src/ComplexSelector/ComplexSelector.tsx b/packages/core/src/ComplexSelector/ComplexSelector.tsx index d165b5feac6a..1b523088d63a 100644 --- a/packages/core/src/ComplexSelector/ComplexSelector.tsx +++ b/packages/core/src/ComplexSelector/ComplexSelector.tsx @@ -30,7 +30,7 @@ import {Icon} from '../Icon'; import {Spinner} from '../Spinner'; import {useTranslator} from '../i18n'; import {layerAnimations} from '../Layer/layerAnimations.stylex'; -import {usePopover} from '../Popover/usePopover'; +import {popoverSurfaceStyles, usePopover} from '../Popover/usePopover'; import { colorVars, durationVars, @@ -208,7 +208,14 @@ export interface ComplexSelectorProps extends Omit< width?: SizeValue; /** Popup placement. */ placement?: 'above' | 'below' | 'start' | 'end'; - /** StyleX styles for the popup content container. */ + /** + * StyleX styles for the popup content container. + * + * The container is the popup surface itself (it carries the + * `astryx-complex-selector-popup` theme target and paints the surface), + * so these styles can override background, border, radius, and shadow + * as well as the content padding. + */ contentXstyle?: StyleXStyles; /** Test ID for the trigger container. */ 'data-testid'?: string; @@ -292,6 +299,10 @@ export function ComplexSelector({ dialogLabel: label, hasCloseButton: false, hasAutoFocus: true, + // The content container paints the surface itself so that the + // `astryx-complex-selector-popup` theme target reaches the element + // owning background, border, radius, and width (#4804). + hasSurface: false, onHide: () => { document.getElementById(triggerId)?.focus(); }, @@ -313,7 +324,16 @@ export function ComplexSelector({ const triggerContent = triggerLabel ?? placeholder; const content = ( -
+
{children(optimisticValue, commitValue, popover.hide, { isOpen: popover.isOpen, isBusy, diff --git a/packages/core/src/Popover/usePopover.tsx b/packages/core/src/Popover/usePopover.tsx index d213475ad29d..7f1e46620513 100644 --- a/packages/core/src/Popover/usePopover.tsx +++ b/packages/core/src/Popover/usePopover.tsx @@ -31,15 +31,22 @@ import {rtlStyles} from '../utils'; import {useTranslator} from '../i18n'; import {useDevWarning} from '../hooks/useDevWarning'; -const styles = stylex.create({ - // Default popover surface — background, radius, shadow. - // Applied automatically unless hasSurface is false. - // Consumers that need a raw positioned layer should use useLayer instead. +// Default popover surface — background, radius, shadow. +// Applied automatically unless hasSurface is false. +// Consumers that need a raw positioned layer should use useLayer instead. +// Exported so popover-based components that opt out via `hasSurface: false` +// can paint the identical surface on their own stable-classed element +// (e.g. ComplexSelector's `astryx-complex-selector-popup` theme target) +// without duplicating these declarations. +export const popoverSurfaceStyles = stylex.create({ surface: { backgroundColor: colorVars['--color-background-popover'], borderRadius: radiusVars['--radius-container'], boxShadow: shadowVars['--shadow-low'], }, +}); + +const styles = stylex.create({ // Focus trap container contentWrapper: { position: 'relative', @@ -414,7 +421,7 @@ export function usePopover(options: UsePopoverOptions = {}): UsePopoverReturn { aria-label={role === 'dialog' ? dialogLabel : undefined} {...stylex.props( styles.contentWrapper, - hasSurface && styles.surface, + hasSurface && popoverSurfaceStyles.surface, xstyle, )}> {children} From e5fb2406734887cbae8eea0f4d50c745985df328 Mon Sep 17 00:00:00 2001 From: AK <144495202+AKnassa@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:56:16 -0400 Subject: [PATCH 2/2] test(ComplexSelector): harden popup paint probe Self-review round on #4821: the declares() helper could match a longer atomic class sharing a prefix (.x14o matched .x14odbl rules), weakening assertions in both directions; a boundary lookahead closes it. The paint assertions now pin the exact surface tokens (--color-background-popover, --radius-container, --shadow-low) so the test states what the changeset promises: same tokens, same surface. --- .../ComplexSelector/ComplexSelector.test.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/core/src/ComplexSelector/ComplexSelector.test.tsx b/packages/core/src/ComplexSelector/ComplexSelector.test.tsx index 0e0961caa861..a51caa266e22 100644 --- a/packages/core/src/ComplexSelector/ComplexSelector.test.tsx +++ b/packages/core/src/ComplexSelector/ComplexSelector.test.tsx @@ -219,7 +219,11 @@ describe('ComplexSelector', () => { return Array.from(el.classList) .filter(c => c.startsWith('x')) .some(c => { - const rules = css.match(new RegExp(`\\.${c}[^{]*\\{[^}]*\\}`, 'g')); + // The lookahead stops a class from matching a longer class that + // shares its prefix (`.x14o` must not match `.x14odbl{…}`). + const rules = css.match( + new RegExp(`\\.${c}(?![a-zA-Z0-9_-])[^{]*\\{[^}]*\\}`, 'g'), + ); return (rules ?? []).some( rule => rule.includes(property) && @@ -254,10 +258,15 @@ describe('ComplexSelector', () => { expect(popup).not.toBeNull(); const css = injectedCss(); - // The stable-classed element owns the surface paint… - expect(declares(css, popup!, 'background-color')).toBe(true); - expect(declares(css, popup!, 'border-radius')).toBe(true); - expect(declares(css, popup!, 'box-shadow')).toBe(true); + // The stable-classed element owns the surface paint, with the same + // tokens every popover surface uses… + expect( + declares(css, popup!, 'background-color', '--color-background-popover'), + ).toBe(true); + expect(declares(css, popup!, 'border-radius', '--radius-container')).toBe( + true, + ); + expect(declares(css, popup!, 'box-shadow', '--shadow-low')).toBe(true); // …and the dialog wrapper above it paints no second surface behind, so // a theme override genuinely replaces the surface instead of floating