fix selection with falsy keys (adobe#7079)

* fix selection with falsy keys * fix falsy check if Selection class * add unit tests * revert additional change
boutahlilsoufiane · Sep 25, 2024 · 246225b · 246225b
1 parent 7c0e286
commit 246225b
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 5 deletions.
diff --git a/packages/@react-stately/selection/src/Selection.ts b/packages/@react-stately/selection/src/Selection.ts
@@ -23,8 +23,8 @@ export class Selection extends Set<Key> {
   constructor(keys?: Iterable<Key> | Selection, anchorKey?: Key, currentKey?: Key) {
     super(keys);
     if (keys instanceof Selection) {
-      this.anchorKey = anchorKey || keys.anchorKey;
-      this.currentKey = currentKey || keys.currentKey;
+      this.anchorKey = anchorKey ?? keys.anchorKey;
+      this.currentKey = currentKey ?? keys.currentKey;
     } else {
       this.anchorKey = anchorKey;
       this.currentKey = currentKey;

diff --git a/packages/@react-stately/selection/src/SelectionManager.ts b/packages/@react-stately/selection/src/SelectionManager.ts
@@ -226,9 +226,9 @@ export class SelectionManager implements MultipleSelectionManager {
       selection = new Selection([toKey], toKey, toKey);
     } else {
       let selectedKeys = this.state.selectedKeys as Selection;
-      let anchorKey = selectedKeys.anchorKey || toKey;
+      let anchorKey = selectedKeys.anchorKey ?? toKey;
       selection = new Selection(selectedKeys, anchorKey, toKey);
-      for (let key of this.getKeyRange(anchorKey, selectedKeys.currentKey || toKey)) {
+      for (let key of this.getKeyRange(anchorKey, selectedKeys.currentKey ?? toKey)) {
         selection.delete(key);
       }
 
@@ -263,7 +263,7 @@ export class SelectionManager implements MultipleSelectionManager {
 
     let keys: Key[] = [];
     let key = from;
-    while (key) {
+    while (key != null) {
       let item = this.collection.getItem(key);
       if (item && item.type === 'item' || (item.type === 'cell' && this.allowsCellSelection)) {
         keys.push(key);

diff --git a/packages/react-aria-components/test/ListBox.test.js b/packages/react-aria-components/test/ListBox.test.js
@@ -1070,4 +1070,94 @@ describe('ListBox', () => {
       });
     });
   });
+
+  const FalsyExample = () => (
+    <ListBox aria-label="Test" selectionMode="multiple" selectionBehavior="replace">
+      <ListBoxItem id="0">Item 0</ListBoxItem>
+      <ListBoxItem id="1">Item 1</ListBoxItem>
+      <ListBoxItem id="2">Item 2</ListBoxItem>
+    </ListBox>
+  );
+
+  describe('selection with falsy keys', () => {
+    describe('keyboard', () => {
+      it('should deselect item 0 when navigating back in replace selection mode', async () => {
+        let {getAllByRole} = render(<FalsyExample />);
+
+        let items = getAllByRole('option');
+
+        await user.click(items[1]);
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+
+        // Hold Shift and press ArrowUp to select item 0
+        await user.keyboard('{Shift>}{ArrowUp}{/Shift}');
+
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+        expect(items[2]).toHaveAttribute('aria-selected', 'false');
+
+        // Hold Shift and press ArrowDown to navigate back to item 1
+        await user.keyboard('{Shift>}{ArrowDown}{/Shift}');
+
+        expect(items[0]).toHaveAttribute('aria-selected', 'false');
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+        expect(items[2]).toHaveAttribute('aria-selected', 'false');
+      });
+
+      it('should correctly handle starting selection at item 0 and extending to item 2', async () => {
+        let {getAllByRole} = render(<FalsyExample />);
+
+        let items = getAllByRole('option');
+
+        await user.click(items[0]);
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+
+        // Hold Shift and press ArrowDown to select item 1
+        await user.keyboard('{Shift>}{ArrowDown}{/Shift}');
+
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+        expect(items[2]).toHaveAttribute('aria-selected', 'false');
+
+        // Hold Shift and press ArrowDown to select item 2
+        await user.keyboard('{Shift>}{ArrowDown}{/Shift}');
+
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+        expect(items[2]).toHaveAttribute('aria-selected', 'true');
+      });
+    });
+
+    describe('mouse', () => {
+      it('should deselect item 0 when clicking another item in replace selection mode', async () => {
+        let {getAllByRole} = render(<FalsyExample />);
+
+        let items = getAllByRole('option');
+
+        await user.click(items[1]);
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+
+        await user.click(items[0]);
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+        expect(items[1]).toHaveAttribute('aria-selected', 'false');
+
+        await user.click(items[1]);
+        expect(items[1]).toHaveAttribute('aria-selected', 'true');
+        expect(items[0]).toHaveAttribute('aria-selected', 'false');
+      });
+
+      it('should correctly handle mouse selection starting at item 0 and extending to item 2', async () => {
+        let {getAllByRole} = render(<FalsyExample />);
+
+        let items = getAllByRole('option');
+
+        await user.click(items[0]);
+        expect(items[0]).toHaveAttribute('aria-selected', 'true');
+
+        await user.click(items[2]);
+        expect(items[0]).toHaveAttribute('aria-selected', 'false');
+        expect(items[2]).toHaveAttribute('aria-selected', 'true');
+      });
+    });
+  });
 });