diff --git a/.changeset/selector-indicator-position.md b/.changeset/selector-indicator-position.md new file mode 100644 index 0000000000000..b4ebdbdcfd5d8 --- /dev/null +++ b/.changeset/selector-indicator-position.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] Selector and MultiSelector: `indicatorPosition` places the selection indicator on either edge of the option row — `start` or `end`, logical, so it follows RTL. Defaults keep today's rendering (`end` for Selector's check, `start` for MultiSelector's checkbox); a start-positioned check reserves its column on every row so labels stay aligned. + +@cixzhang diff --git a/apps/storybook/stories/MultiSelector.stories.tsx b/apps/storybook/stories/MultiSelector.stories.tsx index ebf092907d2aa..150edd5f7c9f9 100644 --- a/apps/storybook/stories/MultiSelector.stories.tsx +++ b/apps/storybook/stories/MultiSelector.stories.tsx @@ -577,3 +577,26 @@ export const ThemedIcons: Story = { ); }, }; + +/** + * `indicatorPosition="end"` moves the checkbox to the trailing edge of each + * row. The default is `start`, where the checkbox leads the label as it does in + * CheckboxList. + */ +export const EndIndicatorPosition: Story = { + render: () => { + const [value, setValue] = useState(['Name', 'Email']); + return ( + // No hasSelectAll: its divider is an unallowed listbox child and fails + // the a11y audit as soon as a story opens the popup (#4994). + + ); + }, +}; diff --git a/apps/storybook/stories/Selector.stories.tsx b/apps/storybook/stories/Selector.stories.tsx index b86630df58981..772207a400100 100644 --- a/apps/storybook/stories/Selector.stories.tsx +++ b/apps/storybook/stories/Selector.stories.tsx @@ -890,3 +890,27 @@ export const DefaultSelectionIndicator: Story = { ); }, }; + +/** + * `indicatorPosition="start"` moves the mark to the leading edge, the way a + * native menu marks its chosen row. + * + * The column is reserved on every row, not just the chosen one, so the labels + * stay on one line — the default check draws nothing when unchecked, and + * without the column only the chosen label would be indented. + */ +export const StartIndicatorPosition: Story = { + render: () => { + const [value, setValue] = useState('Banana'); + return ( + + ); + }, +}; diff --git a/packages/core/src/Indicator/index.ts b/packages/core/src/Indicator/index.ts index 4787f3fea50b3..5bb4dc05b3bd1 100644 --- a/packages/core/src/Indicator/index.ts +++ b/packages/core/src/Indicator/index.ts @@ -38,6 +38,7 @@ export type { IndicatorMap, IndicatorName, IndicatorNameOfFamily, + IndicatorPosition, IndicatorProps, IndicatorRegistry, IndicatorSize, diff --git a/packages/core/src/Indicator/types.ts b/packages/core/src/Indicator/types.ts index f76ebcab9fc0f..cb3b3ff44b8be 100644 --- a/packages/core/src/Indicator/types.ts +++ b/packages/core/src/Indicator/types.ts @@ -67,6 +67,17 @@ export type IndicatorState = /** Indicator size scale — matches the control sizes of the owning inputs. */ export type IndicatorSize = 'sm' | 'md'; +/** + * Which edge of its row an indicator sits on. + * + * Logical, not physical: `start` is the left edge in LTR and the right edge in + * RTL. Owned by the host component, not by the indicator — an indicator draws a + * picture and has no say in where the row puts it — which is why this is a prop + * on the components that lay out rows rather than part of + * {@link IndicatorProps}. + */ +export type IndicatorPosition = 'start' | 'end'; + /** * Props every indicator accepts. * diff --git a/packages/core/src/MultiSelector/MultiSelector.doc.mjs b/packages/core/src/MultiSelector/MultiSelector.doc.mjs index 61bd33476b316..ad76fc74ab736 100644 --- a/packages/core/src/MultiSelector/MultiSelector.doc.mjs +++ b/packages/core/src/MultiSelector/MultiSelector.doc.mjs @@ -192,6 +192,13 @@ export const docs = { description: 'Custom render function for each selectable option in the dropdown. Not called for dividers, sections, or the select-all row.', }, + { + name: 'indicatorPosition', + type: "'start' | 'end'", + description: + 'Which edge of the option row carries the checkbox. end pushes it to the far edge of the row, including on the select-all row.', + default: "'start'", + }, { name: 'width', type: 'SizeValue', diff --git a/packages/core/src/MultiSelector/MultiSelector.test.tsx b/packages/core/src/MultiSelector/MultiSelector.test.tsx index 0f809e79fcaa1..0fceec724d26d 100644 --- a/packages/core/src/MultiSelector/MultiSelector.test.tsx +++ b/packages/core/src/MultiSelector/MultiSelector.test.tsx @@ -2110,3 +2110,72 @@ describe('MultiSelector popup theme target', () => { expect(popup.querySelector('[role="listbox"]')).not.toBeNull(); }); }); + +describe('MultiSelector indicatorPosition', () => { + const OPTIONS = ['Apple', 'Banana', 'Cherry']; + const rowFor = (label: string): HTMLElement => + screen + .getAllByRole('option', {hidden: true}) + .find(row => row.textContent?.includes(label))!; + + it('draws the checkbox before the label by default', () => { + render( + {}} + isDefaultOpen + />, + ); + const row = rowFor('Banana'); + const checkbox = row.querySelector('.astryx-checkbox')!; + const label = row.lastElementChild!; + expect(label).toHaveTextContent('Banana'); + expect( + label.compareDocumentPosition(checkbox) & + Node.DOCUMENT_POSITION_PRECEDING, + ).toBeTruthy(); + }); + + it('draws the checkbox after the label when set to end', () => { + render( + {}} + indicatorPosition="end" + isDefaultOpen + />, + ); + const row = rowFor('Banana'); + const checkbox = row.querySelector('.astryx-checkbox')!; + const label = row.firstElementChild!; + expect(label).toHaveTextContent('Banana'); + expect( + label.compareDocumentPosition(checkbox) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + + it('keeps the select-all row on the same edge as the options', () => { + render( + {}} + hasSelectAll + indicatorPosition="end" + isDefaultOpen + />, + ); + const row = rowFor('Select all'); + expect( + row.firstElementChild!.compareDocumentPosition( + row.querySelector('.astryx-checkbox')!, + ) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); +}); diff --git a/packages/core/src/MultiSelector/MultiSelector.tsx b/packages/core/src/MultiSelector/MultiSelector.tsx index 1b7e790e502d8..a963678ec9277 100644 --- a/packages/core/src/MultiSelector/MultiSelector.tsx +++ b/packages/core/src/MultiSelector/MultiSelector.tsx @@ -44,6 +44,7 @@ import {Divider} from '../Divider'; import {Spinner} from '../Spinner'; import {TextInput} from '../TextInput'; import {CheckboxInput} from '../CheckboxInput'; +import type {IndicatorPosition} from '../Indicator'; import {Badge} from '../Badge'; import { colorVars, @@ -320,6 +321,14 @@ const styles = stylex.create({ display: 'flex', flexShrink: 0, }, + // Pushed to the row's far edge rather than sitting against the label, which + // is what an end-positioned control means here. The row is not + // `space-between` (a truncating label plus a trailing control is what wants + // the auto margin), and `renderOption` content is not wrapped in a growing + // span, so the margin has to live on the checkbox itself. + checkboxDecorativeEnd: { + marginInlineStart: 'auto', + }, // Label text for items (rendered outside checkbox for correct click // behavior). Typography is inherited from the row; this only handles @@ -607,6 +616,13 @@ export interface MultiSelectorProps< */ renderOption?: (option: MultiSelectorOptionData) => ReactNode; + /** + * Which edge of the option row carries the checkbox. + * + * @default 'start' + */ + indicatorPosition?: IndicatorPosition; + /** * Whether the dropdown starts open on mount. * Useful for showcases and previews. @@ -692,6 +708,7 @@ export function MultiSelector({ triggerDisplay = 'count', maxBadges = 3, renderOption, + indicatorPosition = 'start', isDefaultOpen = false, 'data-testid': testId, htmlName, @@ -1234,6 +1251,24 @@ export function MultiSelector({ const isPartiallySelected = isSelectAll && selectAllState === 'indeterminate'; + const checkbox = ( +
+ {}} + isDisabled={item.disabled} + size={size === 'lg' ? 'md' : size} + /> +
+ ); + return (
({ item.disabled && styles.itemDisabled, ), )}> -
- {}} - isDisabled={item.disabled} - size={size === 'lg' ? 'md' : size} - /> -
+ {indicatorPosition === 'start' && checkbox} {renderOption && !isSelectAll ? ( renderOption(item) ) : ( @@ -1290,11 +1316,13 @@ export function MultiSelector({ {item.label ?? item.value} )} + {indicatorPosition === 'end' && checkbox}
); }, [ renderOption, + indicatorPosition, highlightedIndex, optimisticValue, allEnabledSelected, diff --git a/packages/core/src/Selector/Selector.doc.mjs b/packages/core/src/Selector/Selector.doc.mjs index ebdeccbae309a..26539b5f9e9ae 100644 --- a/packages/core/src/Selector/Selector.doc.mjs +++ b/packages/core/src/Selector/Selector.doc.mjs @@ -156,6 +156,13 @@ export const docs = { description: 'Custom render function for each selectable option in the dropdown. Use this instead of JSX children; dividers and sections are rendered by the selector.', }, + { + name: 'indicatorPosition', + type: "'start' | 'end'", + description: + 'Which edge of the option row carries the selected mark. start reserves a mark column ahead of every label so they stay aligned, the way a native menu does; end is the house convention shared with Typeahead and CommandPalette.', + default: "'end'", + }, { name: 'width', type: 'SizeValue', diff --git a/packages/core/src/Selector/Selector.test.tsx b/packages/core/src/Selector/Selector.test.tsx index dab7ca95eab21..ebcb2cafba487 100644 --- a/packages/core/src/Selector/Selector.test.tsx +++ b/packages/core/src/Selector/Selector.test.tsx @@ -2550,3 +2550,108 @@ describe('Selector disabled state theme target', () => { expect(css).toContain('opacity: 0.4'); }); }); + +describe('Selector indicatorPosition', () => { + const openRows = (): HTMLElement[] => screen.getAllByRole('option', h); + const rowFor = (label: string): HTMLElement => + openRows().find(row => row.textContent?.includes(label))!; + + it('draws the mark after the option content by default', () => { + render( + {}} + isDefaultOpen + />, + ); + const row = rowFor('Banana'); + const mark = row.querySelector('.astryx-selector-check')!; + const content = row.querySelector('.astryx-selector-option')!; + expect( + content.compareDocumentPosition(mark) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + + it('draws the mark before the option content when set to start', () => { + render( + {}} + indicatorPosition="start" + isDefaultOpen + />, + ); + const row = rowFor('Banana'); + const mark = row.querySelector('.astryx-selector-check')!; + const content = row.querySelector('.astryx-selector-option')!; + expect( + content.compareDocumentPosition(mark) & Node.DOCUMENT_POSITION_PRECEDING, + ).toBeTruthy(); + }); + + it('reserves the mark column on every row, at either position', () => { + // The default check draws nothing when unchecked, so without a reserved + // column the chosen row would be laid out differently from the rest — + // indented at the start, truncating earlier at the end. Every row is two + // children wide either way, so a row's geometry does not depend on whether + // it happens to be the chosen one. + const {unmount} = render( + {}} + indicatorPosition="start" + isDefaultOpen + />, + ); + for (const row of openRows()) { + expect(row.children).toHaveLength(2); + } + unmount(); + + render( + {}} + isDefaultOpen + />, + ); + for (const row of openRows()) { + expect(row.children).toHaveLength(2); + } + }); + + it('positions a themed replacement indicator the same way', () => { + const theme = defineTheme({ + name: 'selector-start-radio-mark-test', + indicators: {check: RadioIndicator}, + }); + render( + + {}} + indicatorPosition="start" + isDefaultOpen + /> + , + ); + for (const row of openRows()) { + const radio = row.querySelector('.astryx-radio')!; + const content = row.querySelector('.astryx-selector-option')!; + expect( + content.compareDocumentPosition(radio) & + Node.DOCUMENT_POSITION_PRECEDING, + ).toBeTruthy(); + } + }); +}); diff --git a/packages/core/src/Selector/Selector.tsx b/packages/core/src/Selector/Selector.tsx index 2353436eef1bc..dd35543760f2e 100644 --- a/packages/core/src/Selector/Selector.tsx +++ b/packages/core/src/Selector/Selector.tsx @@ -33,6 +33,7 @@ import {usePopover} from '../Popover/usePopover'; import {useTooltip} from '../Tooltip'; import {Icon, renderIconSlot, type IconType} from '../Icon'; import {useIndicator} from '../Indicator'; +import type {IndicatorPosition} from '../Indicator'; import type {IconName} from '../Icon'; import { Field, @@ -301,6 +302,19 @@ const styles = stylex.create({ flex: 1, minWidth: 0, }, + // The mark's column, reserved on every row and at either position, so a row + // occupies the same geometry whether or not it is the chosen one — the + // default check draws nothing when unchecked, and without the column a list + // would indent (or truncate) its chosen row differently from the rest. + // `minWidth` rather than `width`: a theme can replace `check` with a larger + // indicator (a radio is 20px at `sm`), and the column has to grow with it. + itemMarkColumn: { + display: 'inline-flex', + alignItems: 'center', + justifyContent: 'center', + flexShrink: 0, + minWidth: '1rem', + }, itemCheckmark: { flexShrink: 0, width: 16, @@ -518,6 +532,16 @@ interface SelectorPropsBase< */ renderOption?: (option: SelectorOptionData) => ReactNode; + /** + * Which edge of the option row carries the selected mark. `start` reserves a + * mark column ahead of every label so they stay aligned, the way a native + * menu does; `end` is the house convention shared with Typeahead and + * CommandPalette. + * + * @default 'end' + */ + indicatorPosition?: IndicatorPosition; + /** * Whether to show a search input for filtering options. * @default false @@ -668,6 +692,7 @@ export function Selector( startIcon, htmlName, renderOption, + indicatorPosition = 'end', hasSearch = false, searchPlaceholder: searchPlaceholderFromProps, placement, @@ -1088,6 +1113,45 @@ export function Selector( const isHighlighted = flatIndex === highlightedIndex; const isSelected = item.value === normalizedValue; + /* + * Rendered UNCONDITIONALLY, with the state passed down: the default + * check draws nothing when unchecked, but a theme that replaces the + * `check` indicator with a radio needs the unselected state to draw + * its empty circle. `{isSelected && …}` would make that impossible. + * + * `selector-check` stays the stable target for the mark's position + * in the row; the indicator owns what the mark looks like. + */ + const mark = ( + + + + ); + + const optionContent = ( + + {renderOption ? renderOption(item) : } + + ); + + const content = + indicatorPosition === 'start' ? ( + <> + {mark} + {optionContent} + + ) : ( + <> + {optionContent} + {mark} + + ); + return (
( isSelected && styles.itemSelected, item.disabled && styles.itemDisabled, )}> - - {renderOption ? ( - renderOption(item) - ) : ( - - )} - - {/* - * Rendered UNCONDITIONALLY, with the state passed down: the default - * check draws nothing when unchecked, but a theme that replaces the - * `check` indicator with a radio needs the unselected state to draw - * its empty circle. `{isSelected && …}` would make that impossible. - * - * `selector-check` stays the stable target for the mark's position - * in the row; the indicator owns what the mark looks like. - */} - + {content}
); }, [ renderOption, + indicatorPosition, highlightedIndex, size, normalizedValue,