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
34 changes: 34 additions & 0 deletions packages/@react-spectrum/s2/test/Picker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,40 @@ describe('Picker', () => {
expect(tree.getByTestId('custom-value')).toHaveTextContent('Chocolate, Vanilla');
});

it('supports shift+click to select a range in multi-selection', async () => {
let user = userEvent.setup({delay: null, pointerMap});
let items = [
{id: 'chocolate', name: 'Chocolate'},
{id: 'strawberry', name: 'Strawberry'},
{id: 'vanilla', name: 'Vanilla'}
];
let tree = render(
<Picker label="Test picker" selectionMode="multiple" items={items}>
{(item: any) => (
<PickerItem id={item.id} textValue={item.name}>
{item.name}
</PickerItem>
)}
</Picker>
);

let selectTester = testUtilUser.createTester('Select', {
root: tree.container,
interactionType: 'mouse'
});
await selectTester.open();
let options = selectTester.getOptions();

await user.click(options[0]);
await user.keyboard('{Shift>}');
await user.click(options[2]);
await user.keyboard('{/Shift}');

expect(options[0]).toHaveAttribute('aria-selected', 'true');
expect(options[1]).toHaveAttribute('aria-selected', 'true');
expect(options[2]).toHaveAttribute('aria-selected', 'true');
});

it('should warn if the custom render value output has a interactive child', async () => {
using spy = jest.spyOn(console, 'warn').mockImplementation(() => {}) as jest.SpyInstance &
Disposable;
Expand Down
44 changes: 44 additions & 0 deletions packages/react-aria-components/test/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,50 @@ describe('Select', () => {
expect(trigger).toHaveTextContent('2 selected items');
});

it('supports shift+click to select a range in multi-selection', async () => {
let {getByTestId} = render(<TestSelect selectionMode="multiple" />);
let selectTester = testUtilUser.createTester('Select', {root: getByTestId('select')});

await selectTester.open();
let options = selectTester.getOptions();

await user.click(options[0]);
expect(options[0]).toHaveAttribute('aria-selected', 'true');

await user.keyboard('{Shift>}');
await user.click(options[2]);
await user.keyboard('{/Shift}');

expect(options[0]).toHaveAttribute('aria-selected', 'true');
expect(options[1]).toHaveAttribute('aria-selected', 'true');
expect(options[2]).toHaveAttribute('aria-selected', 'true');
});

it('keeps a stable anchor across consecutive shift+clicks', async () => {
let {getByTestId} = render(<TestSelect selectionMode="multiple" />);
let selectTester = testUtilUser.createTester('Select', {root: getByTestId('select')});

await selectTester.open();
let options = selectTester.getOptions();

await user.click(options[0]);

await user.keyboard('{Shift>}');
await user.click(options[2]);
expect(options[0]).toHaveAttribute('aria-selected', 'true');
expect(options[1]).toHaveAttribute('aria-selected', 'true');
expect(options[2]).toHaveAttribute('aria-selected', 'true');

// Shift+click again from the same anchor: the range shrinks rather than the
// anchor jumping to the previous target.
await user.click(options[1]);
await user.keyboard('{/Shift}');

expect(options[0]).toHaveAttribute('aria-selected', 'true');
expect(options[1]).toHaveAttribute('aria-selected', 'true');
expect(options[2]).toHaveAttribute('aria-selected', 'false');
});

it('has a value immediately after rendering', async () => {
function Example() {
const ref = useRef(null);
Expand Down
34 changes: 32 additions & 2 deletions packages/react-stately/src/select/useSelectState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {FormValidationState, useFormValidationState} from '../form/useFormValida
import {ListState, useListState} from '../list/useListState';
import {OverlayTriggerState, useOverlayTriggerState} from '../overlays/useOverlayTriggerState';
import {useControlledState} from '../utils/useControlledState';
import {useMemo, useState} from 'react';
import {useMemo, useRef, useState} from 'react';

export type SelectionMode = 'single' | 'multiple';
export type ValueType<M extends SelectionMode> = M extends 'single' ? Key | null : readonly Key[];
Expand Down Expand Up @@ -196,12 +196,27 @@ export function useSelectState<T, M extends SelectionMode = 'single'>(
}
};

// Preserve the selection's anchor (anchorKey/currentKey) across renders. The
// multiple-selection `value` is a plain Key[], so without this the listbox
// would rebuild an anchorless Selection on every render and range selection
// (shift+click / shift+arrow) would collapse to just the clicked item. We keep
// the last Selection produced internally and feed it back while its membership
// still matches `value`.
let lastSelection = useRef<Set<Key> | null>(null);

let listState = useListState({
...props,
selectionMode,
disallowEmptySelection: selectionMode === 'single',
allowDuplicateSelectionEvents: true,
selectedKeys: useMemo(() => convertValue(displayValue), [displayValue]),
selectedKeys: useMemo(() => {
let selectedKeys = convertValue(displayValue);
let last = lastSelection.current;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this technically breaks the rules of react because it reads from a ref during render.

@devongovett devongovett Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternative suggestion: we could associate the extra info directly with the value via a WeakMap in onChange, and read it here.

let selectionMap = new WeakMap();

// in onChange
let value = [...keys];
selectionMap.set(value, keys);

// in render, when getting selectedKeys
let selectedKeys = selectionMap.get(displayValue) || convertValue(displayValue);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a little looking into it, the current implementation does indeed fail the compiler, though it isn't being caught in lint at the moment even when updated, not sure why. You can see more info here #10174

if (last != null && Array.isArray(selectedKeys) && isSameSelection(last, selectedKeys)) {
return last;
}
return selectedKeys;
Comment on lines +213 to +218

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking maybe we could preserve the anchorKey and currentKey from the Selection instead and preserve them instead of walking and replicating the Selection, but I suppose we would still need to guard against if the set of selected keys changed outside of this flow still and thus walk the set anyways

}, [displayValue]),
onSelectionChange: (keys: Selection) => {
// impossible, but TS doesn't know that
if (keys === 'all') {
Expand All @@ -212,6 +227,9 @@ export function useSelectState<T, M extends SelectionMode = 'single'>(
let key = keys.values().next().value ?? null;
setValue(key);
} else {
// Remember the Selection (with its anchor) so it survives the round-trip
// through the plain `value` array on the next render.
lastSelection.current = keys;
setValue([...keys]);
}
if (shouldCloseOnSelect) {
Expand Down Expand Up @@ -278,3 +296,15 @@ function convertValue(value: Key | Key[] | null | undefined) {
}
return Array.isArray(value) ? value : [value];
}

function isSameSelection(selection: Set<Key>, keys: Key[]): boolean {
if (selection.size !== keys.length) {
return false;
}
for (let key of keys) {
if (!selection.has(key)) {
return false;
}
}
return true;
}