Make rows focusable, remove focus sink (#3963)

* Make rows focusable, remove focus sink

Fixes #3957

* tweaks

* add comment

* smaller area

* fix mousewheel clicks on GroupCell

* event.buttons -> event.button

* only set tabindex when in a treegrid

* fix react warning

Author: nstepien

src/DataGrid.tsx

@@ -1,11 +1,4 @@
-import {
-  useCallback,
-  useImperativeHandle,
-  useLayoutEffect,
-  useMemo,
-  useRef,
-  useState
-} from 'react';
+import { useCallback, useImperativeHandle, useLayoutEffect, useMemo, useState } from 'react';
 import type { Key, KeyboardEvent } from 'react';
 import { flushSync } from 'react-dom';
 
@@ -76,13 +69,7 @@ import type { PartialPosition } from './ScrollToCell';
 import ScrollToCell from './ScrollToCell';
 import { default as defaultRenderSortStatus } from './sortStatus';
 import { cellDragHandleClassname, cellDragHandleFrozenClassname } from './style/cell';
-import {
-  focusSinkClassname,
-  focusSinkHeaderAndSummaryClassname,
-  rootClassname,
-  viewportDraggingClassname
-} from './style/core';
-import { rowSelected, rowSelectedWithFrozenCell } from './style/row';
+import { rootClassname, viewportDraggingClassname } from './style/core';
 import SummaryRow from './SummaryRow';
 
 export interface SelectCellState extends Position {
@@ -386,11 +373,6 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
     (): SelectCellState | EditCellState<R> => ({ idx: -1, rowIdx: minRowIdx - 1, mode: 'SELECT' })
   );
 
-  /**
-   * refs
-   */
-  const focusSinkRef = useRef<HTMLDivElement>(null);
-
   /**
    * computed values
    */
@@ -499,37 +481,19 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
   const selectCellLatest = useLatestFunc(selectCell);
   const selectHeaderCellLatest = useLatestFunc(selectHeaderCell);
 
-  /**
-   * callbacks
-   */
-  const focusCell = useCallback(
-    (shouldScroll = true) => {
-      const cell = getCellToScroll(gridRef.current!);
-      if (cell === null) return;
-
-      if (shouldScroll) {
-        scrollIntoView(cell);
-      }
-
-      cell.focus({ preventScroll: true });
-    },
-    [gridRef]
-  );
-
   /**
    * effects
    */
   useLayoutEffect(() => {
     if (shouldFocusCell) {
-      if (focusSinkRef.current !== null && selectedPosition.idx === -1) {
-        focusSinkRef.current.focus({ preventScroll: true });
-        scrollIntoView(focusSinkRef.current);
+      if (selectedPosition.idx === -1) {
+        focusRow(gridRef.current!);
       } else {
-        focusCell();
+        focusCell(gridRef.current!);
       }
       setShouldFocusCell(false);
     }
-  }, [shouldFocusCell, focusCell, selectedPosition.idx]);
+  }, [shouldFocusCell, selectedPosition.idx, gridRef]);
 
   useImperativeHandle(
     ref,
@@ -632,9 +596,13 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
       if (cellEvent.isGridDefaultPrevented()) return;
     }
 
-    if (!(event.target instanceof Element)) return;
-    const isCellEvent = event.target.closest('.rdg-cell') !== null;
-    const isRowEvent = isTreeGrid && event.target === focusSinkRef.current;
+    const { target } = event;
+
+    if (!(target instanceof Element)) return;
+
+    const isCellEvent = target.closest('.rdg-cell') !== null;
+    const isRowEvent = isTreeGrid && target.role === 'row';
+
     if (!isCellEvent && !isRowEvent) return;
 
     switch (event.key) {
@@ -734,7 +702,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
   function handleDragHandlePointerDown(event: React.PointerEvent<HTMLDivElement>) {
     // keep the focus on the cell
     event.preventDefault();
-    if (event.pointerType === 'mouse' && event.buttons !== 1) {
+    if (event.pointerType === 'mouse' && event.button !== 0) {
       return;
     }
     setDragging(true);
@@ -774,7 +742,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
 
   function handleDragHandleClick() {
     // keep the focus on the cell but do not scroll
-    focusCell(false);
+    focusCell(gridRef.current!, false);
   }
 
   function handleDragHandleDoubleClick(event: React.MouseEvent<HTMLDivElement>) {
@@ -870,20 +838,32 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
     const isRowSelected = selectedCellIsWithinSelectionBounds && idx === -1;
 
     switch (key) {
-      case 'ArrowUp':
-        return { idx, rowIdx: rowIdx - 1 };
+      case 'ArrowUp': {
+        const nextRowIdx = rowIdx - 1;
+        return {
+          // avoid selecting header rows
+          idx: idx === -1 && nextRowIdx < -topSummaryRowsCount ? 0 : idx,
+          rowIdx: nextRowIdx
+        };
+      }
       case 'ArrowDown':
         return { idx, rowIdx: rowIdx + 1 };
-      case leftKey:
-        return { idx: idx - 1, rowIdx };
+      case leftKey: {
+        const nextIdx = idx - 1;
+        return {
+          // avoid selecting header rows
+          idx: rowIdx < -topSummaryRowsCount && nextIdx < 0 ? 0 : nextIdx,
+          rowIdx
+        };
+      }
       case rightKey:
         return { idx: idx + 1, rowIdx };
       case 'Tab':
         return { idx: idx + (shiftKey ? -1 : 1), rowIdx };
       case 'Home':
-        // If row is selected then move focus to the first row
-        if (isRowSelected) return { idx, rowIdx: minRowIdx };
-        return { idx: 0, rowIdx: ctrlKey ? minRowIdx : rowIdx };
+        // If row is selected then move focus to the first header row's cell.
+        if (isRowSelected || ctrlKey) return { idx: 0, rowIdx: minRowIdx };
+        return { idx: 0, rowIdx };
       case 'End':
         // If row is selected then move focus to the last row.
         if (isRowSelected) return { idx, rowIdx: maxRowIdx };
@@ -1149,7 +1129,8 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
           lastFrozenColumnIndex,
           onRowChange: handleFormatterRowChangeLatest,
           selectCell: selectCellLatest,
-          selectedCellEditor: getCellEditor(rowIdx)
+          selectedCellEditor: getCellEditor(rowIdx),
+          isTreeGrid
         })
       );
     }
@@ -1179,9 +1160,6 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
     templateRows += ` repeat(${bottomSummaryRowsCount}, ${summaryRowHeight}px)`;
   }
 
-  const isGroupRowFocused =
-    selectedPosition.idx === -1 && selectedPosition.rowIdx !== minRowIdx - 1;
-
   return (
     <div
       role={role}
@@ -1204,18 +1182,10 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
       )}
       style={{
         ...style,
-        // set scrollPadding to correctly position non-sticky cells after scrolling
-        scrollPaddingInlineStart:
-          selectedPosition.idx > lastFrozenColumnIndex || scrollToPosition?.idx !== undefined
-            ? `${totalFrozenColumnWidth}px`
-            : undefined,
-        scrollPaddingBlock:
-          isRowIdxWithinViewportBounds(selectedPosition.rowIdx) ||
-          scrollToPosition?.rowIdx !== undefined
-            ? `${headerRowsHeight + topSummaryRowsCount * summaryRowHeight}px ${
-                bottomSummaryRowsCount * summaryRowHeight
-              }px`
-            : undefined,
+        // set scrollPadding to correctly scroll to non-sticky cells/rows
+        scrollPaddingInlineStart: totalFrozenColumnWidth,
+        scrollPaddingBlockStart: headerRowsHeight + topSummaryRowsCount * summaryRowHeight,
+        scrollPaddingBlockEnd: bottomSummaryRowsCount * summaryRowHeight,
         gridTemplateColumns,
         gridTemplateRows: templateRows,
         '--rdg-header-row-height': `${headerRowHeight}px`,
@@ -1289,6 +1259,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
                   selectedCellIdx={isSummaryRowSelected ? selectedPosition.idx : undefined}
                   isTop
                   selectCell={selectCellLatest}
+                  isTreeGrid={isTreeGrid}
                 />
               );
             })}
@@ -1322,6 +1293,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
                   selectedCellIdx={isSummaryRowSelected ? selectedPosition.idx : undefined}
                   isTop={false}
                   selectCell={selectCellLatest}
+                  isTreeGrid={isTreeGrid}
                 />
               );
             })}
@@ -1334,24 +1306,6 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
       {/* render empty cells that span only 1 column so we can safely measure column widths, regardless of colSpan */}
       {renderMeasuringCells(viewportColumns)}
 
-      {/* extra div is needed for row navigation in a treegrid */}
-      {isTreeGrid && (
-        <div
-          ref={focusSinkRef}
-          tabIndex={isGroupRowFocused ? 0 : -1}
-          className={classnames(focusSinkClassname, {
-            [focusSinkHeaderAndSummaryClassname]: !isRowIdxWithinViewportBounds(
-              selectedPosition.rowIdx
-            ),
-            [rowSelected]: isGroupRowFocused,
-            [rowSelectedWithFrozenCell]: isGroupRowFocused && lastFrozenColumnIndex !== -1
-          })}
-          style={{
-            gridRowStart: selectedPosition.rowIdx + headerAndTopSummaryRowsCount + 1
-          }}
-        />
-      )}
-
       {scrollToPosition !== null && (
         <ScrollToCell
           scrollToPosition={scrollToPosition}
@@ -1363,10 +1317,32 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
   );
 }
 
+function getRowToScroll(gridEl: HTMLDivElement) {
+  return gridEl.querySelector<HTMLDivElement>(':scope > [role="row"][tabindex="0"]');
+}
+
 function getCellToScroll(gridEl: HTMLDivElement) {
   return gridEl.querySelector<HTMLDivElement>(':scope > [role="row"] > [tabindex="0"]');
 }
 
 function isSamePosition(p1: Position, p2: Position) {
   return p1.idx === p2.idx && p1.rowIdx === p2.rowIdx;
 }
+
+function focusElement(element: HTMLDivElement | null, shouldScroll: boolean) {
+  if (element === null) return;
+
+  if (shouldScroll) {
+    scrollIntoView(element);
+  }
+
+  element.focus({ preventScroll: true });
+}
+
+function focusRow(gridEl: HTMLDivElement) {
+  focusElement(getRowToScroll(gridEl), true);
+}
+
+function focusCell(gridEl: HTMLDivElement, shouldScroll = true) {
+  focusElement(getCellToScroll(gridEl), shouldScroll);
+}

src/GroupCell.tsx

@@ -40,20 +40,19 @@ function GroupCell<R, SR>({
 
   return (
     <div
+      key={column.key}
       role="gridcell"
       aria-colindex={column.idx + 1}
       aria-selected={isCellSelected}
-      tabIndex={tabIndex}
-      key={column.key}
+      // tabIndex={undefined} prevents clicks on the cell
+      // from stealing focus from the row.
+      // onMouseDown={preventDefault} would break mousewheel clicks
+      tabIndex={tabIndex === -1 ? undefined : tabIndex}
       className={getCellClassname(column)}
       style={{
         ...getCellStyle(column),
         cursor: isLevelMatching ? 'pointer' : 'default'
       }}
-      onMouseDown={(event) => {
-        // prevents clicking on the cell from stealing focus from focusSink
-        event.preventDefault();
-      }}
       onClick={isLevelMatching ? toggleGroup : undefined}
       onFocus={onFocus}
     >

src/GroupRow.tsx

@@ -2,7 +2,7 @@ import { memo, useMemo } from 'react';
 import { css } from 'ecij';
 
 import { RowSelectionContext, type RowSelectionContextValue } from './hooks';
-import { classnames, getRowStyle } from './utils';
+import { classnames } from './utils';
 import type { BaseRenderRowProps, GroupRow } from './types';
 import { SELECT_COLUMN_KEY } from './Columns';
 import GroupCell from './GroupCell';
@@ -44,6 +44,7 @@ function GroupedRow<R, SR>({
   isRowSelectionDisabled: _isRowSelectionDisabled,
   ...props
 }: GroupRowRendererProps<R, SR>) {
+  const isPositionOnRow = selectedCellIdx === -1;
   // Select is always the first column
   const idx = viewportColumns[0].key === SELECT_COLUMN_KEY ? row.level + 1 : row.level;
 
@@ -64,15 +65,16 @@ function GroupedRow<R, SR>({
         aria-setsize={row.setSize}
         aria-posinset={row.posInSet + 1} // aria-posinset is 1-based
         aria-expanded={row.isExpanded}
+        tabIndex={isPositionOnRow ? 0 : -1}
         className={classnames(
           rowClassname,
           groupRowClassname,
           `rdg-row-${rowIdx % 2 === 0 ? 'even' : 'odd'}`,
-          selectedCellIdx === -1 && rowSelectedClassname,
+          isPositionOnRow && rowSelectedClassname,
           className
         )}
         onMouseDown={handleSelectGroup}
-        style={getRowStyle(gridRowStart)}
+        style={{ gridRowStart }}
         {...props}
       >
         {viewportColumns.map((column) => (

src/HeaderCell.tsx

@@ -45,21 +45,15 @@ const resizeHandleClassname = `rdg-resize-handle ${resizeHandle}`;
 
 const cellDraggableClassname = 'rdg-cell-draggable';
 
-const cellDragging = css`
+const cellDraggingOrOver = css`
   @layer rdg.HeaderCell {
     background-color: var(--rdg-header-draggable-background-color);
   }
 `;
 
-const cellDraggingClassname = `rdg-cell-dragging ${cellDragging}`;
+const cellDraggingClassname = `rdg-cell-dragging ${cellDraggingOrOver}`;
 
-const cellOver = css`
-  @layer rdg.HeaderCell {
-    background-color: var(--rdg-header-draggable-background-color);
-  }
-`;
-
-const cellOverClassname = `rdg-cell-drag-over ${cellOver}`;
+const cellOverClassname = `rdg-cell-drag-over ${cellDraggingOrOver}`;
 
 const dragImageClassname = css`
   @layer rdg.HeaderCell {
@@ -336,7 +330,7 @@ function ResizeHandle<R, SR>({
   const isRtl = direction === 'rtl';
 
   function onPointerDown(event: React.PointerEvent<HTMLDivElement>) {
-    if (event.pointerType === 'mouse' && event.buttons !== 1) {
+    if (event.pointerType === 'mouse' && event.button !== 0) {
       return;
     }
 

src/HeaderRow.tsx

@@ -62,6 +62,7 @@ function HeaderRow<R, SR, K extends React.Key>({
   direction
 }: HeaderRowProps<R, SR, K>) {
   const [draggedColumnKey, setDraggedColumnKey] = useState<string>();
+  const isPositionOnRow = selectedCellIdx === -1;
 
   const cells = [];
   for (let index = 0; index < columns.length; index++) {
@@ -98,9 +99,7 @@ function HeaderRow<R, SR, K extends React.Key>({
       aria-rowindex={rowIdx} // aria-rowindex is 1 based
       className={classnames(
         headerRowClassname,
-        {
-          [rowSelectedClassname]: selectedCellIdx === -1
-        },
+        isPositionOnRow && rowSelectedClassname,
         headerRowClass
       )}
     >