Adding support to SelectionManager for ctrl-clicking on windows (#2557)

Author: ktabors

packages/@react-aria/selection/src/useSelectableCollection.ts

@@ -13,20 +13,12 @@
 import {FocusEvent, HTMLAttributes, Key, KeyboardEvent, RefObject, useEffect, useRef} from 'react';
 import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus';
 import {FocusStrategy, KeyboardDelegate} from '@react-types/shared';
-import {focusWithoutScrolling, isMac, mergeProps, useEvent} from '@react-aria/utils';
-import {isNonContiguousSelectionModifier} from './utils';
+import {focusWithoutScrolling, mergeProps, useEvent} from '@react-aria/utils';
+import {isCtrlKeyPressed, isNonContiguousSelectionModifier} from './utils';
 import {MultipleSelectionManager} from '@react-stately/selection';
 import {useLocale} from '@react-aria/i18n';
 import {useTypeSelect} from './useTypeSelect';
 
-function isCtrlKeyPressed(e: KeyboardEvent) {
-  if (isMac()) {
-    return e.metaKey;
-  }
-
-  return e.ctrlKey;
-}
-
 interface SelectableCollectionOptions {
   /**
    * An interface for reading and updating multiple selection state.

packages/@react-aria/selection/src/useSelectableItem.ts

@@ -12,7 +12,7 @@
 
 import {focusSafely} from '@react-aria/focus';
 import {HTMLAttributes, Key, RefObject, useEffect, useRef} from 'react';
-import {isNonContiguousSelectionModifier} from './utils';
+import {isCtrlKeyPressed, isNonContiguousSelectionModifier} from './utils';
 import {LongPressEvent, PressEvent} from '@react-types/shared';
 import {mergeProps} from '@react-aria/utils';
 import {MultipleSelectionManager} from '@react-stately/selection';
@@ -86,7 +86,24 @@ export function useSelectableItem(options: SelectableItemOptions): SelectableIte
     if (e.pointerType === 'keyboard' && isNonContiguousSelectionModifier(e)) {
       manager.toggleSelection(key);
     } else {
-      manager.select(key, e);
+      if (manager.selectionMode === 'none') {
+        return;
+      }
+
+      if (manager.selectionMode === 'single') {
+        if (manager.isSelected(key) && !manager.disallowEmptySelection) {
+          manager.toggleSelection(key);
+        } else {
+          manager.replaceSelection(key);
+        }
+      } else if (e && e.shiftKey) {
+        manager.extendSelection(key);
+      } else if (manager.selectionBehavior === 'toggle' || (e && (isCtrlKeyPressed(e) || e.pointerType === 'touch' || e.pointerType === 'virtual'))) {
+        // if touch or virtual (VO) then we just want to toggle, otherwise it's impossible to multi select because they don't have modifier keys
+        manager.toggleSelection(key);
+      } else {
+        manager.replaceSelection(key);
+      }
     }
   };
 

packages/@react-aria/selection/src/utils.ts

@@ -11,14 +11,24 @@
  */
 
 import {isAppleDevice} from '@react-aria/utils';
+import {isMac} from '@react-aria/utils';
 
 interface Event {
   altKey: boolean,
-  ctrlKey: boolean
+  ctrlKey: boolean,
+  metaKey: boolean
 }
 
 export function isNonContiguousSelectionModifier(e: Event) {
   // Ctrl + Arrow Up/Arrow Down has a system wide meaning on macOS, so use Alt instead.
   // On Windows and Ubuntu, Alt + Space has a system wide meaning.
   return isAppleDevice() ? e.altKey : e.ctrlKey;
 }
+
+export function isCtrlKeyPressed(e: Event) {
+  if (isMac()) {
+    return e.metaKey;
+  }
+
+  return e.ctrlKey;
+}

packages/@react-spectrum/list/test/ListView.test.js

@@ -387,13 +387,81 @@ describe('ListView', function () {
         expect(rows[2]).toHaveAttribute('aria-selected', 'true');
       });
 
-      it('should support single tap to perform row selection with screen reader if onAction isn\'t provided', function () {
+      it('should toggle items in selection highlight with ctrl-click on Mac', function () {
+        let uaMock = jest.spyOn(navigator, 'platform', 'get').mockImplementation(() => 'Mac');
         let onSelectionChange = jest.fn();
         let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight'});
 
         let rows = tree.getAllByRole('row');
         expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'false');
+        act(() => userEvent.click(getCell(tree, 'Bar'), {ctrlKey: true}));
 
+        checkSelection(onSelectionChange, ['bar']);
+        expect(rows[1]).toHaveAttribute('aria-selected', 'true');
+
+        onSelectionChange.mockClear();
+        act(() => userEvent.click(getCell(tree, 'Baz'), {ctrlKey: true}));
+        checkSelection(onSelectionChange, ['baz']);
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'true');
+
+        uaMock.mockRestore();
+      });
+
+      it('should allow multiple items to be selected in selection highlight with ctrl-click on Windows', function () {
+        let uaMock = jest.spyOn(navigator, 'userAgent', 'get').mockImplementation(() => 'Windows');
+        let onSelectionChange = jest.fn();
+        let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight'});
+
+        let rows = tree.getAllByRole('row');
+        expect(rows[0]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'false');
+        act(() => userEvent.click(getCell(tree, 'Foo'), {ctrlKey: true}));
+
+        checkSelection(onSelectionChange, ['foo']);
+        expect(rows[0]).toHaveAttribute('aria-selected', 'true');
+
+        onSelectionChange.mockClear();
+        act(() => userEvent.click(getCell(tree, 'Baz'), {ctrlKey: true}));
+        checkSelection(onSelectionChange, ['foo', 'baz']);
+        expect(rows[0]).toHaveAttribute('aria-selected', 'true');
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'true');
+
+        uaMock.mockRestore();
+      });
+
+      it('should toggle items in selection highlight with meta-click on Windows', function () {
+        let uaMock = jest.spyOn(navigator, 'userAgent', 'get').mockImplementation(() => 'Windows');
+        let onSelectionChange = jest.fn();
+        let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight'});
+
+        let rows = tree.getAllByRole('row');
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'false');
+        act(() => userEvent.click(getCell(tree, 'Bar'), {metaKey: true}));
+
+        checkSelection(onSelectionChange, ['bar']);
+        expect(rows[1]).toHaveAttribute('aria-selected', 'true');
+
+        onSelectionChange.mockClear();
+        act(() => userEvent.click(getCell(tree, 'Baz'), {metaKey: true}));
+        checkSelection(onSelectionChange, ['baz']);
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+        expect(rows[2]).toHaveAttribute('aria-selected', 'true');
+
+        uaMock.mockRestore();
+      });
+
+      it('should support single tap to perform row selection with screen reader if onAction isn\'t provided', function () {
+        let onSelectionChange = jest.fn();
+        let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight'});
+
+        let rows = tree.getAllByRole('row');
+        expect(rows[1]).toHaveAttribute('aria-selected', 'false');
+  
         act(() => userEvent.click(within(rows[1]).getByText('Bar'), {pointerType: 'touch', width: 0, height: 0}));
         checkSelection(onSelectionChange, [
           'bar'

packages/@react-spectrum/table/test/Table.test.js

@@ -2414,6 +2414,7 @@ describe('TableView', function () {
       });
 
       it('will add to the current selection if the command key is pressed', function () {
+        let uaMock = jest.spyOn(navigator, 'platform', 'get').mockImplementation(() => 'Mac');
         let onSelectionChange = jest.fn();
         let tree = renderTable({onSelectionChange, selectionStyle: 'highlight'});
 
@@ -2439,6 +2440,8 @@ describe('TableView', function () {
         checkRowSelection(rows.slice(6, 10), false);
         checkRowSelection(rows.slice(10, 21), true);
         checkRowSelection(rows.slice(21), false);
+
+        uaMock.mockRestore();
       });
 
       it('should toggle selection with touch', function () {

packages/@react-stately/selection/src/SelectionManager.ts

@@ -422,9 +422,7 @@ export class SelectionManager implements MultipleSelectionManager {
       } else {
         this.replaceSelection(key);
       }
-    } else if (e && e.shiftKey) {
-      this.extendSelection(key);
-    } else if (this.selectionBehavior === 'toggle' || (e && (e.metaKey || e.pointerType === 'touch' || e.pointerType === 'virtual'))) {
+    } else if (this.selectionBehavior === 'toggle' || (e && (e.pointerType === 'touch' || e.pointerType === 'virtual'))) {
       // if touch or virtual (VO) then we just want to toggle, otherwise it's impossible to multi select because they don't have modifier keys
       this.toggleSelection(key);
     } else {