Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/selector-menu-clearance.md
Original file line number Diff line number Diff line change
@@ -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
34 changes: 34 additions & 0 deletions apps/storybook/stories/Selector.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div style={{display: 'flex', flexDirection: 'column', gap: 32}}>
<Selector
label="placement=below"
options={options}
value={below}
onChange={v => setBelow(v)}
placement="below"
/>
<Selector
label="placement=start"
options={options}
value={start}
onChange={v => setStart(v)}
placement="start"
/>
<Selector
label="placement=end"
options={options}
value={end}
onChange={v => setEnd(v)}
placement="end"
/>
</div>
);
},
};

export const StatusVariantComparison: Story = {
render: () => {
const [a, setA] = useState<string | undefined>();
Expand Down
84 changes: 84 additions & 0 deletions packages/core/src/Selector/Selector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof generateThemeCSS>[0]) {
const {prose, component} = generateThemeCSS(theme);
Expand Down Expand Up @@ -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(
<Selector
label="Fruit"
options={OPTIONS}
value="Banana"
onChange={() => {}}
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(
<Selector
label="Fruit"
options={OPTIONS}
value="Banana"
onChange={() => {}}
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(
<Selector
label="Fruit"
options={OPTIONS}
value="Banana"
onChange={() => {}}
/>,
);

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(
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/Selector/Selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1412,6 +1412,12 @@ export function Selector<T extends SelectorOptionType>(
{
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,
},
Expand Down
Loading