Ensure that arrow keys in grid cells always win over children (#6116)

Author: devongovett

packages/@react-aria/focus/src/isElementVisible.ts

@@ -43,6 +43,8 @@ function isStyleVisible(element: Element) {
 function isAttributeVisible(element: Element, childElement?: Element) {
   return (
     !element.hasAttribute('hidden') &&
+    // Ignore HiddenSelect when tree walking.
+    !(element.hasAttribute('data-a11y-ignore') && element.getAttribute('aria-hidden') === 'true') &&
     (element.nodeName === 'DETAILS' &&
       childElement &&
       childElement.nodeName !== 'SUMMARY'

packages/@react-aria/grid/src/useGridCell.ts

@@ -124,9 +124,9 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
           focusable = null;
         }
 
+        e.preventDefault();
+        e.stopPropagation();
         if (focusable) {
-          e.preventDefault();
-          e.stopPropagation();
           focusSafely(focusable);
           scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)});
         } else {
@@ -137,11 +137,15 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
           // child, depending on the focus mode.
           let prev = keyboardDelegate.getKeyLeftOf(node.key);
           if (prev !== node.key) {
+            // We prevent the capturing event from reaching children of the cell, e.g. pickers.
+            // We want arrow keys to navigate to the next cell instead. We need to re-dispatch 
+            // the event from a higher parent so it still bubbles and gets handled by useSelectableCollection.
+            ref.current.parentElement.dispatchEvent(
+              new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
+            );
             break;
           }
 
-          e.preventDefault();
-          e.stopPropagation();
           if (focusMode === 'cell' && direction === 'rtl') {
             focusSafely(ref.current);
             scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)});
@@ -167,19 +171,23 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
           focusable = null;
         }
 
+        e.preventDefault();
+        e.stopPropagation();
         if (focusable) {
-          e.preventDefault();
-          e.stopPropagation();
           focusSafely(focusable);
           scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)});
         } else {
           let next = keyboardDelegate.getKeyRightOf(node.key);
           if (next !== node.key) {
+            // We prevent the capturing event from reaching children of the cell, e.g. pickers.
+            // We want arrow keys to navigate to the next cell instead. We need to re-dispatch 
+            // the event from a higher parent so it still bubbles and gets handled by useSelectableCollection.
+            ref.current.parentElement.dispatchEvent(
+              new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
+            );
             break;
           }
 
-          e.preventDefault();
-          e.stopPropagation();
           if (focusMode === 'cell' && direction === 'ltr') {
             focusSafely(ref.current);
             scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)});

packages/@react-spectrum/table/stories/Table.stories.tsx

@@ -31,6 +31,7 @@ import {IllustratedMessage} from '@react-spectrum/illustratedmessage';
 import {Key, LoadingState} from '@react-types/shared';
 import {Link} from '@react-spectrum/link';
 import NoSearchResults from '@spectrum-icons/illustrations/NoSearchResults';
+import {Picker} from '@react-spectrum/picker';
 import {Radio, RadioGroup} from '@react-spectrum/radio';
 import React, {useCallback, useState} from 'react';
 import {SearchField} from '@react-spectrum/searchfield';
@@ -450,6 +451,17 @@ export const FocusableCells: TableStory = {
             <Cell><Link><a href="https://yahoo.com" target="_blank">Yahoo</a></Link></Cell>
             <Cell>Three</Cell>
           </Row>
+          <Row>
+            <Cell><Switch aria-label="Foo" /></Cell>
+            <Cell>
+              <Picker aria-label="Search engine" placeholder="Search with:" width={'100%'} isQuiet>
+                <Item key="Yahoo">Yahoo</Item>
+                <Item key="Google">Google</Item>
+                <Item key="DuckDuckGo">DuckDuckGo</Item>
+              </Picker>
+            </Cell>
+            <Cell>Three</Cell>
+          </Row>
         </TableBody>
       </TableView>
       <label htmlFor="focus-after">Focus after</label>
@@ -927,7 +939,7 @@ function AsyncLoadingExample(props) {
       let url = new URL('https://www.reddit.com/r/upliftingnews.json');
       if (cursor) {
         url.searchParams.append('after', cursor);
-      }      
+      }
       let res = await fetch(url.toString(), {signal});
       let json = await res.json();
       return {items: json.data.children, cursor: json.data.after};
@@ -1076,14 +1088,14 @@ function AsyncLoadingExampleQuarryTest(props) {
       return {
         items: items.slice().sort((a, b) => {
           let cmp = a[sortDescriptor.column] < b[sortDescriptor.column] ? -1 : 1;
-    
+
           if (sortDescriptor.direction === 'descending') {
             cmp *= -1;
           }
-    
+
           return cmp;
         })
-        
+
       };
     }
   });

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

@@ -25,6 +25,7 @@ import {Divider} from '@react-spectrum/divider';
 import {enableTableNestedRows} from '@react-stately/flags';
 import {getFocusableTreeWalker} from '@react-aria/focus';
 import {Heading} from '@react-spectrum/text';
+import {Item, Picker} from '@react-spectrum/picker';
 import {Link} from '@react-spectrum/link';
 import {Provider} from '@react-spectrum/provider';
 import React from 'react';
@@ -1499,6 +1500,31 @@ export let tableTests = () => {
         </>
       );
 
+      let renderWithPicker = () => render(
+        <>
+          <TableView aria-label="Table">
+            <TableHeader>
+              <Column>Foo</Column>
+              <Column>Bar</Column>
+              <Column>baz</Column>
+            </TableHeader>
+            <TableBody>
+              <Row>
+                <Cell textValue="Foo 1"><Switch aria-label="Foo 1" /></Cell>
+                <Cell textValue="Search engine">
+                  <Picker aria-label="Search engine" placeholder="Search with:" width={'100%'} isQuiet>
+                    <Item key="Yahoo">Yahoo</Item>
+                    <Item key="Google">Google</Item>
+                    <Item key="DuckDuckGo">DuckDuckGo</Item>
+                  </Picker>
+                </Cell>
+                <Cell>Baz 1</Cell>
+              </Row>
+            </TableBody>
+          </TableView>
+        </>
+      );
+
       it('should retain focus on the pressed child', async function () {
         let tree = renderFocusable();
         let switchToPress = tree.getAllByRole('switch')[2];
@@ -1629,6 +1655,19 @@ export let tableTests = () => {
         expect(document.activeElement).toBe(baz1);
       });
 
+      it('should not trap focus when navigating through a cell with a picker using the arrow keys', function () {
+        let tree = renderWithPicker();
+        focusCell(tree, 'Baz 1');
+        moveFocus('ArrowLeft');
+        expect(document.activeElement).toBe(tree.getByRole('button'));
+        moveFocus('ArrowLeft');
+        expect(document.activeElement).toBe(tree.getByRole('switch'));
+        moveFocus('ArrowRight');
+        expect(document.activeElement).toBe(tree.getByRole('button'));
+        moveFocus('ArrowRight');
+        expect(document.activeElement).toBe(tree.getAllByRole('gridcell')[1]);
+      });
+
       it('should move focus after the table when tabbing', async function () {
         let tree = renderFocusable();