Fix table keyboard navigation bugs (#1993)

* fixing Tab/shift tab behavior for table

* fixing option/alt+tabbing in table

* updating comment

* fixing lint

* adding test for option + tab focus movement

* preventing option + tab from moving focus in Table

* simplifying fix for shift tabbing into TableView

seems to work if we switch to scrollToItem from TableView instead of always using Virtualizer.scrolltoItem

* adding aria labels

Author: LFDanLu

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

@@ -104,6 +104,11 @@ export function useSelectableCollection(options: SelectableCollectionOptions): S
   let {direction} = useLocale();
 
   let onKeyDown = (e: KeyboardEvent) => {
+    // Prevent option + tab from doing anything since it doesn't move focus to the cells, only buttons/checkboxes
+    if (e.altKey && e.key === 'Tab') {
+      e.preventDefault();
+    }
+
     // Let child element (e.g. menu button) handle the event if the Alt key is pressed.
     // Keyboard events bubble through portals. Don't handle keyboard events
     // for elements outside the collection (e.g. menus).

packages/@react-aria/virtualizer/src/Virtualizer.tsx

@@ -141,14 +141,16 @@ export function useVirtualizer<T extends object, V, W>(props: VirtualizerOptions
   let onFocus = useCallback((e: FocusEvent) => {
     // If the focused item is scrolled out of view and is not in the DOM, the collection
     // will have tabIndex={0}. When tabbing in from outside, scroll the focused item into view.
-    // We only want to do this if the collection itself is receiving focus, not a child
-    // element, and we aren't moving focus to the collection from within (see below).
-    if (e.target === ref.current && !isFocusWithin.current) {
-      virtualizer.scrollToItem(focusedKey, {duration: 0});
+    if (!isFocusWithin.current) {
+      if (scrollToItem) {
+        scrollToItem(focusedKey);
+      } else {
+        virtualizer.scrollToItem(focusedKey, {duration: 0});
+      }
     }
 
     isFocusWithin.current = e.target !== ref.current;
-  }, [ref, virtualizer, focusedKey]);
+  }, [ref, virtualizer, focusedKey, scrollToItem]);
 
   let onBlur = useCallback((e: FocusEvent) => {
     isFocusWithin.current = ref.current.contains(e.relatedTarget as Element);

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

@@ -339,20 +339,24 @@ storiesOf('TableView', module)
   .add(
     'many columns and rows',
     () => (
-      <TableView aria-label="TableView with many columns and rows" selectionMode="multiple" width={700} height={500} onSelectionChange={s => onSelectionChange([...s])}>
-        <TableHeader columns={manyColunns}>
-          {column =>
-            <Column minWidth={100}>{column.name}</Column>
-          }
-        </TableHeader>
-        <TableBody items={manyRows}>
-          {item =>
-            (<Row key={item.foo}>
-              {key => <Cell>{item[key]}</Cell>}
-            </Row>)
-          }
-        </TableBody>
-      </TableView>
+      <>
+        <input aria-label="Focusable before" placeholder="Focusable before" />
+        <TableView aria-label="TableView with many columns and rows" selectionMode="multiple" width={700} height={500} onSelectionChange={s => onSelectionChange([...s])}>
+          <TableHeader columns={manyColunns}>
+            {column =>
+              <Column minWidth={100}>{column.name}</Column>
+            }
+          </TableHeader>
+          <TableBody items={manyRows}>
+            {item =>
+              (<Row key={item.foo}>
+                {key => <Cell>{item[key]}</Cell>}
+              </Row>)
+            }
+          </TableBody>
+        </TableView>
+        <input aria-label="Focusable after" placeholder="Focusable after" />
+      </>
     ),
     {chromatic: {disable: true}}
   )