From 2fb4dd19c059a97d675060fb8e4c733ba5ed3301 Mon Sep 17 00:00:00 2001
From: Alireza <alireza.mirian@gmail.com>
Date: Thu, 12 Dec 2024 21:33:51 +0100
Subject: [PATCH 1/2] fix(@react-aria/selection): don't mutate non-empty
 selection upon focus

closes #7512
---
 .../selection/src/useSelectableCollection.ts  |  9 +++--
 .../test/useSelectableCollection.test.js      | 35 +++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts
index 0d67ea18faf..1b55f350573 100644
--- a/packages/@react-aria/selection/src/useSelectableCollection.ts
+++ b/packages/@react-aria/selection/src/useSelectableCollection.ts
@@ -342,12 +342,11 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
     }
 
     manager.setFocused(true);
-
     if (manager.focusedKey == null) {
-      let navigateToFirstKey = (key: Key | undefined | null) => {
+      let navigateToKey = (key: Key | undefined | null) => {
         if (key != null) {
           manager.setFocusedKey(key);
-          if (selectOnFocus) {
+          if (selectOnFocus && !manager.isSelected(key)) {
             manager.replaceSelection(key);
           }
         }
@@ -357,9 +356,9 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
       // and either focus the first or last item accordingly.
       let relatedTarget = e.relatedTarget as Element;
       if (relatedTarget && (e.currentTarget.compareDocumentPosition(relatedTarget) & Node.DOCUMENT_POSITION_FOLLOWING)) {
-        navigateToFirstKey(manager.lastSelectedKey ?? delegate.getLastKey?.());
+        navigateToKey(manager.lastSelectedKey ?? delegate.getLastKey?.());
       } else {
-        navigateToFirstKey(manager.firstSelectedKey ?? delegate.getFirstKey?.());
+        navigateToKey(manager.firstSelectedKey ?? delegate.getFirstKey?.());
       }
     } else if (!isVirtualized && scrollRef.current) {
       // Restore the scroll position to what it was before.
diff --git a/packages/@react-aria/selection/test/useSelectableCollection.test.js b/packages/@react-aria/selection/test/useSelectableCollection.test.js
index d8c0801177c..89b10f2968d 100644
--- a/packages/@react-aria/selection/test/useSelectableCollection.test.js
+++ b/packages/@react-aria/selection/test/useSelectableCollection.test.js
@@ -107,5 +107,40 @@ describe('useSelectableCollection', () => {
       expect(options[1]).not.toHaveAttribute('aria-selected');
       expect(options[2]).toHaveAttribute('aria-selected', 'true');
     });
+
+    it("doesn't change the selection on focus in multiple selection selectOnFocus", async () => {
+      let onSelectionChange1 = jest.fn();
+      let onSelectionChange2 = jest.fn();
+      let {getByText} = render(
+        <>
+          <List
+            selectionMode="multiple"
+            selectionBehavior="replace"
+            defaultSelectedKeys={['i2', 'i3']}
+            onSelectionChange={onSelectionChange1}>
+            <Item key="i1">Paco de Lucia</Item>
+            <Item key="i2">Vicente Amigo</Item>
+            <Item key="i3">Gerardo Nunez</Item>
+          </List>
+          <button>focus stop</button>
+          <List
+            selectionMode="multiple"
+            selectionBehavior="replace"
+            defaultSelectedKeys={['i2', 'i3']}
+            onSelectionChange={onSelectionChange2}>
+            <Item key="i1">Paco de Lucia</Item>
+            <Item key="i2">Vicente Amigo</Item>
+            <Item key="i3">Gerardo Nunez</Item>
+          </List>
+        </>
+      );
+      await user.click(getByText('focus stop'));
+      await user.tab();
+      await user.click(getByText('focus stop'));
+      await user.tab({shift: true});
+
+      expect(onSelectionChange1).not.toHaveBeenCalled();
+      expect(onSelectionChange2).not.toHaveBeenCalled();
+    });
   });
 });

From 08ef07d58d7e07d7129935e599578e4b4224c208 Mon Sep 17 00:00:00 2001
From: GitHub <snowystinger@gmail.com>
Date: Mon, 13 Jan 2025 14:05:21 +1100
Subject: [PATCH 2/2] add some assertions

---
 .../selection/test/useSelectableCollection.test.js       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/packages/@react-aria/selection/test/useSelectableCollection.test.js b/packages/@react-aria/selection/test/useSelectableCollection.test.js
index 89b10f2968d..bb0f5d5d6e7 100644
--- a/packages/@react-aria/selection/test/useSelectableCollection.test.js
+++ b/packages/@react-aria/selection/test/useSelectableCollection.test.js
@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {fireEvent, installPointerEvent, pointerMap, render, simulateDesktop, simulateMobile} from '@react-spectrum/test-utils-internal';
+import {fireEvent, installPointerEvent, pointerMap, render, simulateDesktop, simulateMobile, within} from '@react-spectrum/test-utils-internal';
 import {Item} from '@react-stately/collections';
 import {List} from '../stories/List';
 import React from 'react';
@@ -108,10 +108,10 @@ describe('useSelectableCollection', () => {
       expect(options[2]).toHaveAttribute('aria-selected', 'true');
     });
 
-    it("doesn't change the selection on focus in multiple selection selectOnFocus", async () => {
+    it('focuses first/last selected item on focus enter and does not change the selection', async () => {
       let onSelectionChange1 = jest.fn();
       let onSelectionChange2 = jest.fn();
-      let {getByText} = render(
+      let {getByText, getAllByRole} = render(
         <>
           <List
             selectionMode="multiple"
@@ -134,10 +134,13 @@ describe('useSelectableCollection', () => {
           </List>
         </>
       );
+      let [firstList, secondList] = getAllByRole('listbox');
       await user.click(getByText('focus stop'));
       await user.tab();
+      expect(document.activeElement).toBe(within(secondList).getByRole('option', {name: 'Vicente Amigo'}));
       await user.click(getByText('focus stop'));
       await user.tab({shift: true});
+      expect(document.activeElement).toBe(within(firstList).getByRole('option', {name: 'Gerardo Nunez'}));
 
       expect(onSelectionChange1).not.toHaveBeenCalled();
       expect(onSelectionChange2).not.toHaveBeenCalled();