Skip to content

fix: preserve selection anchor for multi-Select range selection#10150

Merged
reidbarber merged 2 commits into
adobe:mainfrom
plore:fix/select-multiple-shift-range
Jun 4, 2026
Merged

fix: preserve selection anchor for multi-Select range selection#10150
reidbarber merged 2 commits into
adobe:mainfrom
plore:fix/select-multiple-shift-range

Conversation

@plore

@plore plore commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Closes #10147

Problem

Shift+click (and shift+ArrowUp/Down) range selection did not work in Select/Picker with selectionMode="multiple".
useSelectState exposes the selection as a plain Key[] and rebuilds an anchorless Selection every render, so SelectionManager.extendSelection never had an anchor and the range collapsed to just the clicked item.

Solution strategy

Keep the last Selection produced by the listbox and feed it back as selectedKeys while its membership still matches value, so anchorKey survives the round-trip.
Single-selection is untouched; external value changes (clear/reset/controlled) fall back to the plain array.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Open a Select/Picker with selectionMode="multiple", click an option, then shift+click another — the full range fills in.
Shift+clicking again from the same anchor re-extends/shrinks the range.

@plore plore marked this pull request as ready for review June 2, 2026 14:50

@LFDanLu LFDanLu left a comment

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.

Looks good to me, will see about getting another pair of eyes on this

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

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

@reidbarber reidbarber added this pull request to the merge queue Jun 4, 2026
Merged via the queue into adobe:main with commit 0f5ae89 Jun 4, 2026
29 checks passed
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shift+click range selection doesn't work in multi-select Select/Picker (anchorKey lost in useSelectState)

6 participants