fix(@react-aria/selection): don't mutate non-empty selection upon focus (#7513)

* fix(@react-aria/selection): don't mutate non-empty selection upon focus

closes #7512

* add some assertions

---------

Co-authored-by: GitHub <[email protected]>
Co-authored-by: Robert Snow <[email protected]>
Co-authored-by: Reid Barber <[email protected]>

Author: 3 people

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.

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';
@@ -107,5 +107,43 @@ describe('useSelectableCollection', () => {
       expect(options[1]).not.toHaveAttribute('aria-selected');
       expect(options[2]).toHaveAttribute('aria-selected', 'true');
     });
+
+    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, getAllByRole} = 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>
+        </>
+      );
+      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();
+    });
   });
 });