diff --git a/.changeset/selector-menu-clearance.md b/.changeset/selector-menu-clearance.md new file mode 100644 index 0000000000000..2a4335a8b0a3d --- /dev/null +++ b/.changeset/selector-menu-clearance.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] Selector's menu now clears the trigger by the standard `--spacing-1` gap whenever it is not overlaying it — every explicit `placement`, and search mode. It was the only anchored menu in the system sitting flush against its anchor; DropdownMenu, MultiSelector, ComplexSelector, Popover, and Tooltip all use this clearance. The default selected-item overlay is unchanged: it owns its block geometry and is meant to sit on the trigger. + +@cixzhang diff --git a/apps/storybook/stories/Selector.stories.tsx b/apps/storybook/stories/Selector.stories.tsx index b86630df58981..a81362a6b4f0e 100644 --- a/apps/storybook/stories/Selector.stories.tsx +++ b/apps/storybook/stories/Selector.stories.tsx @@ -746,6 +746,40 @@ export const PlacementAbove: Story = { }, }; +export const Placements: Story = { + render: () => { + const [below, setBelow] = useState('Banana'); + const [start, setStart] = useState('Banana'); + const [end, setEnd] = useState('Banana'); + const options = ['Apple', 'Banana', 'Cherry', 'Date']; + return ( +
+ setBelow(v)} + placement="below" + /> + setStart(v)} + placement="start" + /> + setEnd(v)} + placement="end" + /> +
+ ); + }, +}; + export const StatusVariantComparison: Story = { render: () => { const [a, setA] = useState(); diff --git a/packages/core/src/Selector/Selector.test.tsx b/packages/core/src/Selector/Selector.test.tsx index dab7ca95eab21..68ea918b04b65 100644 --- a/packages/core/src/Selector/Selector.test.tsx +++ b/packages/core/src/Selector/Selector.test.tsx @@ -28,6 +28,7 @@ import {__resetLiveRegionsForTest} from '../hooks/useAnnounce'; import {defineTheme} from '../theme/defineTheme'; import {Theme} from '../theme/Theme'; import {generateThemeCSS} from '../theme/generateThemeRules'; +import {spacingVars} from '../theme/tokens.stylex'; function generateThemeTestCSS(theme: Parameters[0]) { const {prose, component} = generateThemeCSS(theme); @@ -508,6 +509,89 @@ describe('Selector', () => { } }); + describe('menu clearance', () => { + it('clears the trigger by the standard menu offset when placement is explicit', async () => { + const user = userEvent.setup(); + render( + {}} + placement="above" + />, + ); + + await user.click(screen.getByRole('combobox')); + const popover = screen + .getByRole('listbox', {hidden: true}) + .closest('[popover]') as HTMLElement; + // Both block edges, so the gap survives a position-try-fallbacks flip + // to the opposite side (#4803). + await waitFor(() => { + expect(popover.style.getPropertyValue('--x-marginBlockStart')).toBe( + spacingVars['--spacing-1'], + ); + }); + expect(popover.style.getPropertyValue('--x-marginBlockEnd')).toBe( + spacingVars['--spacing-1'], + ); + }); + + it('clears the trigger in search mode', async () => { + const user = userEvent.setup(); + render( + {}} + hasSearch + />, + ); + + // In hasSearch mode the trigger is a plain button, not a combobox. + await user.click(screen.getByRole('button', {name: 'Fruit'})); + const popover = screen + .getByRole('listbox', {hidden: true}) + .closest('[popover]') as HTMLElement; + await waitFor(() => { + expect(popover.style.getPropertyValue('--x-marginBlockStart')).toBe( + spacingVars['--spacing-1'], + ); + }); + }); + + it('stays flush in the default selected-item overlay', async () => { + const restoreRects = mockSelectorRects(); + const user = userEvent.setup(); + try { + render( + {}} + />, + ); + + await user.click(screen.getByRole('combobox')); + const popover = screen + .getByRole('listbox', {hidden: true}) + .closest('[popover]') as HTMLElement; + await waitFor(() => { + expect(popover.getAttribute('style')).toContain( + 'margin-block-start: -110px', + ); + }); + expect(popover.style.getPropertyValue('--x-marginBlockStart')).toBe(''); + expect(popover.style.getPropertyValue('--x-marginBlockEnd')).toBe(''); + } finally { + restoreRects(); + } + }); + }); + describe('hasClear', () => { it('shows selected value label when hasClear is enabled', () => { render( diff --git a/packages/core/src/Selector/Selector.tsx b/packages/core/src/Selector/Selector.tsx index 2353436eef1bc..8f0a5f842375c 100644 --- a/packages/core/src/Selector/Selector.tsx +++ b/packages/core/src/Selector/Selector.tsx @@ -1412,6 +1412,12 @@ export function Selector( { placement: popoverPlacement, alignment: 'start', + // The system's standard menu clearance, except in overlay mode: + // there the measured negative margin owns the block geometry and + // the menu is meant to sit on the trigger, not clear it. + offset: shouldOverlaySelectedItem + ? undefined + : spacingVars['--spacing-1'], xstyle: [styles.popover, layerAnimations[popoverPlacement]], style: popoverOffsetStyle, },