fix: Inline editing tables

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

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
+import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
 import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus';
 import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared';
 import {flushSync} from 'react-dom';
@@ -312,7 +312,10 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
               }
             } while (last);
 
-            if (next && !next.contains(document.activeElement)) {
+            // If the active element is NOT tabbable but is contained by an element that IS tabbable (aka the cell), the browser will actually move focus to
+            // the containing element. We need to special case this so that tab will move focus out of the grid instead of looping between
+            // focusing the containing cell and back to the non-tabbable child element
+            if (next && (!next.contains(document.activeElement) || (document.activeElement && !isTabbable(document.activeElement)))) {
               focusWithoutScrolling(next);
             }
           }

packages/@react-spectrum/s2/src/TableView.tsx

@@ -57,6 +57,7 @@ import Chevron from '../ui-icons/Chevron';
 import Close from '../s2wf-icons/S2_Icon_Close_20_N.svg';
 import {ColumnSize} from '@react-types/table';
 import {DOMRef, DOMRefValue, forwardRefType, GlobalDOMAttributes, LoadingState, Node} from '@react-types/shared';
+import {getActiveElement, getOwnerDocument, useLayoutEffect, useObjectRef} from '@react-aria/utils';
 import {GridNode} from '@react-types/grid';
 import {IconContext} from './Icon';
 // @ts-ignore
@@ -66,12 +67,11 @@ import {Menu, MenuItem, MenuSection, MenuTrigger} from './Menu';
 import Nubbin from '../ui-icons/S2_MoveHorizontalTableWidget.svg';
 import {ProgressCircle} from './ProgressCircle';
 import {raw} from '../style/style-macro' with {type: 'macro'};
-import React, {createContext, CSSProperties, ForwardedRef, forwardRef, ReactElement, ReactNode, RefObject, useCallback, useContext, useMemo, useRef, useState} from 'react';
+import React, {createContext, CSSProperties, ForwardedRef, forwardRef, ReactElement, ReactNode, RefObject, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
 import SortDownArrow from '../s2wf-icons/S2_Icon_SortDown_20_N.svg';
 import SortUpArrow from '../s2wf-icons/S2_Icon_SortUp_20_N.svg';
 import {useActionBarContainer} from './ActionBar';
 import {useDOMRef} from '@react-spectrum/utils';
-import {useLayoutEffect, useObjectRef} from '@react-aria/utils';
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
 import {useScale} from './utils';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
@@ -1115,6 +1115,18 @@ export const EditableCell = forwardRef(function EditableCell(props: EditableCell
   );
 });
 
+const nonTextInputTypes = new Set([
+  'checkbox',
+  'radio',
+  'range',
+  'color',
+  'file',
+  'image',
+  'button',
+  'submit',
+  'reset'
+]);
+
 function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean, cellRef: RefObject<HTMLDivElement>}) {
   let {children, align, renderEditing, isSaving, onSubmit, onCancel, isFocusVisible, cellRef} = props;
   let [isOpen, setIsOpen] = useState(false);
@@ -1134,7 +1146,6 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
     size = 'L';
   }
 
-
   // Popover positioning
   useLayoutEffect(() => {
     if (!isOpen) {
@@ -1151,6 +1162,24 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
     setTableWidth(tableWidth);
   }, [cellRef, density, isOpen]);
 
+  // Auto select the entire text range of the autofocused input on overlay opening
+  // Maybe replace with FocusScope or one of those utilities
+  useEffect(() => {
+    if (isOpen) {
+      let activeElement = getActiveElement(getOwnerDocument(formRef.current));
+      if (activeElement
+        && formRef.current?.contains(activeElement)
+        // not going to handle contenteditable https://stackoverflow.com/questions/6139107/programmatically-select-text-in-a-contenteditable-html-element
+        // seems like an edge case anyways
+        && (
+          (activeElement instanceof HTMLInputElement && !nonTextInputTypes.has(activeElement.type))
+          || activeElement instanceof HTMLTextAreaElement)
+        && typeof activeElement.select === 'function') {
+        activeElement.select();
+      }
+    }
+  }, [isOpen]);
+
   // Cancel, don't save the value
   let cancel = () => {
     setIsOpen(false);
@@ -1178,7 +1207,7 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
                   isForcedVisible: 'visible',
                   ':is([role="row"]:hover *)': 'visible',
                   ':is([role="row"][data-focus-visible-within] *)': 'visible',
-                  '@media not (any-pointer: fine)': 'visible'
+                  '@media not ((hover: hover) and (pointer: fine))': 'visible'
                 }
               })({isForcedVisible: isOpen || !!isSaving})
             }
@@ -1225,7 +1254,7 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
                 onSubmit();
                 setIsOpen(false);
               }}
-              className={style({width: 'full', display: 'flex', alignItems: 'baseline', gap: 16})}
+              className={style({width: 'full', display: 'flex', alignItems: 'start', gap: 16})}
               style={{'--input-width': `calc(${triggerWidth}px - 32px)`} as CSSProperties}>
               {renderEditing()}
               <div className={style({display: 'flex', flexDirection: 'row', alignItems: 'baseline', flexShrink: 0, flexGrow: 0})}>

packages/@react-spectrum/s2/stories/TableView.stories.tsx

@@ -1458,6 +1458,114 @@ let editableColumns: Array<Omit<ColumnProps, 'children'> & {name: string}> = [
 interface EditableTableProps extends TableViewProps {}
 
 export const EditableTable: StoryObj<EditableTableProps> = {
+  render: function EditableTable(props) {
+    let columns = editableColumns;
+    let [editableItems, setEditableItems] = useState(defaultItems);
+    let intermediateValue = useRef<any>(null);
+
+    let onChange = useCallback((id: Key, columnId: Key) => {
+      let value = intermediateValue.current;
+      if (value === null) {
+        return;
+      }
+      intermediateValue.current = null;
+      setEditableItems(prev => {
+        let newItems = prev.map(i => i.id === id && i[columnId] !== value ? {...i, [columnId]: value} : i);
+        return newItems;
+      });
+    }, []);
+
+    let onIntermediateChange = useCallback((value: any) => {
+      intermediateValue.current = value;
+    }, []);
+
+    return (
+      <div className={style({display: 'flex', flexDirection: 'column', gap: 16})}>
+        <TableView aria-label="Dynamic table" {...props} styles={style({width: 800, maxWidth: 'calc(100vw - 2rem)', height: 208})}>
+          <TableHeader columns={columns}>
+            {(column) => (
+              <Column {...column}>{column.name}</Column>
+            )}
+          </TableHeader>
+          <TableBody items={editableItems}>
+            {item => (
+              <Row id={item.id} columns={columns}>
+                {(column) => {
+                  if (column.id === 'fruits') {
+                    return (
+                      <EditableCell
+                        align={column.align}
+                        showDivider={column.showDivider}
+                        onSubmit={() => onChange(item.id, column.id!)}
+                        onCancel={() => {}}
+                        isSaving={item.isSaving[column.id!]}
+                        renderEditing={() => (
+                          <TextField
+                            aria-label="Edit fruit"
+                            autoFocus
+                            validate={value => value.length > 0 ? null : 'Fruit name is required'}
+                            styles={style({flexGrow: 1, flexShrink: 1, minWidth: 0})}
+                            defaultValue={item[column.id!]}
+                            onChange={value => onIntermediateChange(value)} />
+                        )}>
+                        <div className={style({display: 'flex', alignItems: 'center', gap: 8, justifyContent: 'space-between'})}>
+                          {item[column.id]}
+                          <ActionButton slot="edit" aria-label="Edit fruit">
+                            <Edit />
+                          </ActionButton></div>
+                      </EditableCell>
+                    );
+                  }
+                  if (column.id === 'farmer') {
+                    return (
+                      <EditableCell
+                        align={column.align}
+                        showDivider={column.showDivider}
+                        onSubmit={() => onChange(item.id, column.id!)}
+                        onCancel={() => {}}
+                        isSaving={item.isSaving[column.id!]}
+                        renderEditing={() => (
+                          <Picker
+                            aria-label="Edit farmer"
+                            autoFocus
+                            styles={style({flexGrow: 1, flexShrink: 1, minWidth: 0})}
+                            defaultValue={item[column.id!]}
+                            onChange={value => onIntermediateChange(value)}>
+                            <PickerItem textValue="Eva" id="Eva"><User /><Text>Eva</Text></PickerItem>
+                            <PickerItem textValue="Steven" id="Steven"><User /><Text>Steven</Text></PickerItem>
+                            <PickerItem textValue="Michael" id="Michael"><User /><Text>Michael</Text></PickerItem>
+                            <PickerItem textValue="Sara" id="Sara"><User /><Text>Sara</Text></PickerItem>
+                            <PickerItem textValue="Karina" id="Karina"><User /><Text>Karina</Text></PickerItem>
+                            <PickerItem textValue="Otto" id="Otto"><User /><Text>Otto</Text></PickerItem>
+                            <PickerItem textValue="Matt" id="Matt"><User /><Text>Matt</Text></PickerItem>
+                            <PickerItem textValue="Emily" id="Emily"><User /><Text>Emily</Text></PickerItem>
+                            <PickerItem textValue="Amelia" id="Amelia"><User /><Text>Amelia</Text></PickerItem>
+                            <PickerItem textValue="Isla" id="Isla"><User /><Text>Isla</Text></PickerItem>
+                          </Picker>
+                        )}>
+                        <div className={style({display: 'flex', alignItems: 'center', gap: 8, justifyContent: 'space-between'})}>{item[column.id]}<ActionButton slot="edit" aria-label="Edit fruit"><Edit /></ActionButton></div>
+                      </EditableCell>
+                    );
+                  }
+                  if (column.id === 'status') {
+                    return (
+                      <Cell align={column.align} showDivider={column.showDivider}>
+                        <StatusLight variant="informative">{item[column.id]}</StatusLight>
+                      </Cell>
+                    );
+                  }
+                  return <Cell align={column.align} showDivider={column.showDivider}>{item[column.id!]}</Cell>;
+                }}
+              </Row>
+            )}
+          </TableBody>
+        </TableView>
+      </div>
+    );
+  }
+};
+
+export const EditableTableWithAsyncSaving: StoryObj<EditableTableProps> = {
   render: function EditableTable(props) {
     let columns = editableColumns;
     let [editableItems, setEditableItems] = useState(defaultItems);

packages/@react-spectrum/s2/test/EditableTableView.test.tsx

@@ -258,7 +258,7 @@ describe('TableView', () => {
       let input = within(dialog).getByRole('textbox');
       expect(input).toHaveFocus();
 
-      await user.keyboard(' Crisp');
+      await user.keyboard('Apples Crisp');
       await user.keyboard('{Enter}'); // implicitly submit through form
 
       act(() => {jest.runAllTimers();});
@@ -429,7 +429,7 @@ describe('TableView', () => {
       let input = within(dialog).getByRole('textbox');
       expect(input).toHaveFocus();
 
-      await user.keyboard(' Crisp');
+      await user.keyboard('Apples Crisp');
       await user.keyboard('{Enter}'); // implicitly submit through form
 
       act(() => {jest.advanceTimersByTime(5000);});
@@ -445,5 +445,40 @@ describe('TableView', () => {
       expect(button).not.toHaveAttribute('aria-disabled');
       expect(button).toHaveFocus();
     });
+
+    it('should allow tabbing off a pending button', async () => {
+      let {getByRole} = render(
+        <EditableTable delay={10000} />
+      );
+
+      let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')});
+      await user.tab();
+      await user.keyboard('{ArrowRight}');
+      await user.keyboard('{Enter}');
+
+      let dialog = getByRole('dialog');
+      expect(dialog).toBeVisible();
+
+      let input = within(dialog).getByRole('textbox');
+      expect(input).toHaveFocus();
+
+      await user.keyboard('Apples Crisp');
+      await user.keyboard('{Enter}'); // implicitly submit through form
+
+      act(() => {jest.advanceTimersByTime(5000);});
+
+      expect(dialog).not.toBeInTheDocument();
+      expect(tableTester.findRow({rowIndexOrText: 'Apples Crisp'})).toBeInTheDocument();
+      let button = within(tableTester.findCell({text: 'Apples Crisp'})).getByRole('button');
+      expect(button).toHaveAttribute('aria-disabled', 'true');
+      expect(button).toHaveFocus();
+
+      await user.tab();
+      expect(getByRole('button', {name: 'After'})).toHaveFocus();
+
+      act(() => {jest.runAllTimers();});
+
+      expect(button).not.toHaveAttribute('aria-disabled');
+    });
   });
 });

packages/react-aria-components/stories/Popover.stories.tsx

@@ -469,7 +469,7 @@ export const PopoverTriggerWidthExample: PopoverStory = () => (
 function ScrollingBoundaryContainerExample(args) {
   let [boundaryElem, setBoundaryElem] = useState<HTMLDivElement | null>(null);
   return (
-    <div id="scrolling-boundary" ref={setBoundaryElem} style={{height: 300, width: 300, overflow: 'auto'}}>
+    <div id="scrolling-boundary" ref={setBoundaryElem} style={{height: 300, width: 300, overflow: 'auto', border: '1px solid light-dark(black, white)'}}>
       <div style={{width: 600, height: 600, display: 'flex', alignItems: 'center', justifyContent: 'center'}}>
         <DialogTrigger>
           <Button style={{width: 200, height: 200, display: 'flex', alignItems: 'center', justifyContent: 'center'}}>Open popover</Button>
@@ -483,7 +483,7 @@ function ScrollingBoundaryContainerExample(args) {
               zIndex: 5
             }}>
             <Dialog>
-              Should match the width of the trigger button
+              This is some dummy content for the popover
             </Dialog>
           </Popover>
         </DialogTrigger>