fix: preserve selection anchor for multi-Select range selection#10150
Conversation
LFDanLu
left a comment
There was a problem hiding this comment.
Looks good to me, will see about getting another pair of eyes on this
| let selectedKeys = convertValue(displayValue); | ||
| let last = lastSelection.current; | ||
| if (last != null && Array.isArray(selectedKeys) && isSameSelection(last, selectedKeys)) { | ||
| return last; | ||
| } | ||
| return selectedKeys; |
There was a problem hiding this comment.
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
| selectedKeys: useMemo(() => convertValue(displayValue), [displayValue]), | ||
| selectedKeys: useMemo(() => { | ||
| let selectedKeys = convertValue(displayValue); | ||
| let last = lastSelection.current; |
There was a problem hiding this comment.
I think this technically breaks the rules of react because it reads from a ref during render.
There was a problem hiding this comment.
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);There was a problem hiding this comment.
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
Closes #10147
Problem
Shift+click (and shift+ArrowUp/Down) range selection did not work in
Select/PickerwithselectionMode="multiple".useSelectStateexposes the selection as a plainKey[]and rebuilds an anchorlessSelectionevery render, soSelectionManager.extendSelectionnever had an anchor and the range collapsed to just the clicked item.Solution strategy
Keep the last
Selectionproduced by the listbox and feed it back asselectedKeyswhile its membership still matchesvalue, soanchorKeysurvives the round-trip.Single-selection is untouched; external value changes (clear/reset/controlled) fall back to the plain array.
✅ Pull Request Checklist:
and storybookfor this change (for new code or code which already has tests).📝 Test Instructions:
Open a
Select/PickerwithselectionMode="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.