Refactor listview to always focus the row

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

@@ -381,4 +381,3 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
     return null;
   }
 }
-

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

@@ -67,6 +67,11 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
   let focus = () => {
     let treeWalker = getFocusableTreeWalker(ref.current);
     if (focusMode === 'child') {
+      // If focus is already on a focusable child within the cell, early return so we don't shift focus
+      if (ref.current.contains(document.activeElement) && ref.current !== document.activeElement) {
+        return;
+      }
+
       let focusable = state.selectionManager.childFocusStrategy === 'last'
         ? last(treeWalker)
         : treeWalker.firstChild() as HTMLElement;

packages/@react-spectrum/list/src/ListView.tsx

@@ -26,7 +26,6 @@ import {DragHooks} from '@react-spectrum/dnd';
 import {DragPreview} from './DragPreview';
 import {filterDOMProps} from '@react-aria/utils';
 import {GridCollection, GridState, useGridState} from '@react-stately/grid';
-import {GridKeyboardDelegate, useGrid} from '@react-aria/grid';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {ListLayout} from '@react-stately/layout';
@@ -36,14 +35,13 @@ import {ListViewItem} from './ListViewItem';
 import {ProgressCircle} from '@react-spectrum/progress';
 import React, {ReactElement, useContext, useMemo, useRef} from 'react';
 import {useCollator, useLocale, useMessageFormatter} from '@react-aria/i18n';
+import {useGrid} from '@react-aria/grid';
 import {useProvider} from '@react-spectrum/provider';
 import {Virtualizer} from '@react-aria/virtualizer';
 
 interface ListViewContextValue<T> {
   state: GridState<T, GridCollection<any>>,
-  keyboardDelegate: GridKeyboardDelegate<T, GridCollection<any>>,
   dragState: DraggableCollectionState,
-  onAction:(key: string) => void,
   isListDraggable: boolean,
   layout: ListLayout<T>
 }
@@ -74,7 +72,8 @@ export function useListLayout<T>(state: ListState<T>, density: ListViewProps<T>[
       estimatedRowHeight: ROW_HEIGHTS[density][scale],
       padding: 0,
       collator,
-      loaderHeight: isEmpty ? null : ROW_HEIGHTS[density][scale]
+      loaderHeight: isEmpty ? null : ROW_HEIGHTS[density][scale],
+      allowDisabledKeyFocus: true
     })
     , [collator, scale, density, isEmpty]);
 
@@ -128,8 +127,7 @@ function ListView<T extends object>(props: ListViewProps<T>, ref: DOMRef<HTMLDiv
   let isLoading = loadingState === 'loading' || loadingState === 'loadingMore';
 
   let {styleProps} = useStyleProps(props);
-  let {direction, locale} = useLocale();
-  let collator = useCollator({usage: 'search', sensitivity: 'base'});
+  let {locale} = useLocale();
   let gridCollection = useMemo(() => new GridCollection({
     columnCount: 1,
     items: [...collection].map(item => ({
@@ -151,21 +149,10 @@ function ListView<T extends object>(props: ListViewProps<T>, ref: DOMRef<HTMLDiv
   let state = useGridState({
     ...props,
     collection: gridCollection,
-    focusMode: 'cell',
+    focusMode: 'row',
     selectionBehavior: props.selectionStyle === 'highlight' ? 'replace' : 'toggle'
   });
   let layout = useListLayout(state, props.density || 'regular');
-  let keyboardDelegate = useMemo(() => new GridKeyboardDelegate({
-    collection: state.collection,
-    disabledKeys: state.disabledKeys,
-    ref: domRef,
-    direction,
-    collator,
-    // Focus the ListView cell instead of the row so that focus doesn't change with left/right arrow keys when there aren't any
-    // focusable children in the cell.
-    focusMode: 'cell'
-  }), [state, domRef, direction, collator]);
-
   let provider = useProvider();
   let dragState: DraggableCollectionState;
   if (isListDraggable) {
@@ -183,29 +170,23 @@ function ListView<T extends object>(props: ListViewProps<T>, ref: DOMRef<HTMLDiv
 
   let {gridProps} = useGrid({
     ...props,
+    onCellAction: onAction,
     isVirtualized: true,
-    keyboardDelegate
+    keyboardDelegate: layout
   }, state, domRef);
 
   // Sync loading state into the layout.
   layout.isLoading = isLoading;
-
-  let focusedKey = state.selectionManager.focusedKey;
-  let focusedItem = gridCollection.getItem(state.selectionManager.focusedKey);
-  if (focusedItem?.parentKey != null) {
-    focusedKey = focusedItem.parentKey;
-  }
-
   return (
-    <ListViewContext.Provider value={{state, keyboardDelegate, dragState, onAction, isListDraggable, layout}}>
+    <ListViewContext.Provider value={{state, dragState, isListDraggable, layout}}>
       <Virtualizer
         {...filterDOMProps(otherProps)}
         {...gridProps}
         {...styleProps}
         isLoading={isLoading}
         onLoadMore={onLoadMore}
         ref={domRef}
-        focusedKey={focusedKey}
+        focusedKey={state.selectionManager.focusedKey}
         scrollDirection="vertical"
         className={
           classNames(
@@ -227,7 +208,7 @@ function ListView<T extends object>(props: ListViewProps<T>, ref: DOMRef<HTMLDiv
         {(type, item) => {
           if (type === 'item') {
             return (
-              <ListViewItem item={item} isEmphasized dragHooks={dragHooks}  />
+              <ListViewItem item={item} isEmphasized dragHooks={dragHooks} hasActions={onAction}  />
             );
           } else if (type === 'loader') {
             return (

packages/@react-spectrum/list/src/ListViewItem.tsx

@@ -23,7 +23,7 @@ import {ListViewContext} from './ListView';
 import {mergeProps} from '@react-aria/utils';
 import React, {useContext, useRef} from 'react';
 import {useButton} from '@react-aria/button';
-import {useGridCell, useGridRow, useGridSelectionCheckbox} from '@react-aria/grid';
+import {useGridCell, useGridSelectionCheckbox} from '@react-aria/grid';
 import {useHover, usePress} from '@react-aria/interactions';
 import {useLocale} from '@react-aria/i18n';
 import {useVisuallyHidden} from '@react-aria/visually-hidden';
@@ -32,47 +32,42 @@ export function ListViewItem(props) {
   let {
     item,
     isEmphasized,
-    dragHooks
+    dragHooks,
+    hasActions
   } = props;
-  let cellNode = [...item.childNodes][0];
-  let {state, dragState, onAction, isListDraggable, layout} = useContext(ListViewContext);
-
+  let {state, dragState, isListDraggable, layout} = useContext(ListViewContext);
   let {direction} = useLocale();
   let rowRef = useRef<HTMLDivElement>();
-  let cellRef =  useRef<HTMLDivElement>();
   let {
     isFocusVisible: isFocusVisibleWithin,
     focusProps: focusWithinProps
   } = useFocusRing({within: true});
   let {isFocusVisible, focusProps} = useFocusRing();
-  let allowsInteraction = state.selectionManager.selectionMode !== 'none' || onAction;
+  let allowsInteraction = state.selectionManager.selectionMode !== 'none' || hasActions;
   let isDisabled = !allowsInteraction || state.disabledKeys.has(item.key);
   let isDraggable = dragState?.isDraggable(item.key) && !isDisabled;
   let {hoverProps, isHovered} = useHover({isDisabled});
   let {pressProps, isPressed} = usePress({isDisabled});
-  let {rowProps} = useGridRow({
+
+  // We only make use of useGridCell here to allow for keyboard navigation to the focusable children of the row.
+  // The actual grid cell of the ListView is inert since we don't want to ever focus it to decrease screenreader
+  // verbosity, so we pretend the row node is the cell for interaction purposes. useGridRow is never used since
+  // it would conflict with useGridCell if applied to the same node.
+  let {gridCellProps} = useGridCell({
     node: item,
+    focusMode: 'cell',
     isVirtualized: true,
-    onAction: onAction ? () => onAction(item.key) : undefined,
     shouldSelectOnPressUp: isListDraggable
   }, state, rowRef);
-  let {gridCellProps} = useGridCell({
-    node: cellNode,
-    focusMode: 'cell'
-  }, state, cellRef);
+  delete gridCellProps['aria-colindex'];
+
   let draggableItem: DraggableItemResult;
   if (isListDraggable) {
     // eslint-disable-next-line react-hooks/rules-of-hooks
     draggableItem = dragHooks.useDraggableItem({key: item.key}, dragState);
   }
-  const mergedProps = mergeProps(
-    gridCellProps,
-    hoverProps,
-    focusWithinProps,
-    focusProps
-  );
-  let {checkboxProps} = useGridSelectionCheckbox({...props, key: item.key}, state);
 
+  let {checkboxProps} = useGridSelectionCheckbox({...props, key: item.key}, state);
   let dragButtonRef = React.useRef();
   let {buttonProps} = useButton({
     ...draggableItem?.dragButtonProps,
@@ -98,6 +93,23 @@ export function ListViewItem(props) {
   let isSelected = state.selectionManager.isSelected(item.key);
   let showDragHandle = isDraggable && isFocusVisibleWithin;
   let {visuallyHiddenProps} = useVisuallyHidden();
+  let rowProps = {
+    role: 'row',
+    'aria-label': item.textValue,
+    'aria-selected': state.selectionManager.selectionMode !== 'none' ? isSelected : undefined,
+    'aria-rowindex': item.index + 1
+  };
+
+  const mergedProps = mergeProps(
+    gridCellProps,
+    rowProps,
+    pressProps,
+    isDraggable && draggableItem?.dragProps,
+    hoverProps,
+    focusWithinProps,
+    focusProps
+  );
+
   let isFirstRow = item.prevKey == null;
   let isLastRow = item.nextKey == null;
   // Figure out if the ListView content is equal or greater in height to the container. If so, we'll need to round the bottom
@@ -112,7 +124,7 @@ export function ListViewItem(props) {
 
   return (
     <div
-      {...mergeProps(rowProps, pressProps, isDraggable && draggableItem?.dragProps)}
+      {...mergedProps}
       className={
         classNames(
           listStyles,
@@ -124,6 +136,7 @@ export function ListViewItem(props) {
       }
       ref={rowRef}>
       <div
+        // TODO: refactor the css here now that we are focusing the row?
         className={
           classNames(
             listStyles,
@@ -143,8 +156,8 @@ export function ListViewItem(props) {
             }
           )
         }
-        ref={cellRef}
-        {...mergedProps}>
+        role="gridcell"
+        aria-colindex={1}>
         <Grid UNSAFE_className={listStyles['react-spectrum-ListViewItem-grid']}>
           {isListDraggable &&
             <div className={listStyles['react-spectrum-ListViewItem-draghandle-container']}>

packages/@react-spectrum/list/stories/ListView.stories.tsx

@@ -52,7 +52,23 @@ function renderEmptyState() {
   );
 }
 
+let decorator = (storyFn, context) => {
+  let omittedStories = ['draggable rows', 'dynamic items + renderEmptyState'];
+  return  window.screen.width <= 700 || omittedStories.some(omittedName => context.name.includes(omittedName)) ?
+  storyFn() :
+  (
+    <>
+      <label htmlFor="focus-before">Focus before</label>
+      <input id="focus-before" />
+      {storyFn()}
+      <label htmlFor="focus-after">Focus after</label>
+      <input id="focus-after" />
+    </>
+  );
+};
+
 storiesOf('ListView', module)
+  .addDecorator(decorator)
   .add('default', () => (
     <ListView width="250px">
       <Item textValue="Adobe Photoshop">Adobe Photoshop</Item>