diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 825888ffea6..4259dc51132 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/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); } } diff --git a/packages/@react-spectrum/s2/src/TableView.tsx b/packages/@react-spectrum/s2/src/TableView.tsx index c6441c762fd..9cc6791d954 100644 --- a/packages/@react-spectrum/s2/src/TableView.tsx +++ b/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}) { 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()}
diff --git a/packages/@react-spectrum/s2/stories/TableView.stories.tsx b/packages/@react-spectrum/s2/stories/TableView.stories.tsx index c2367d0c302..045b6a4ecf6 100644 --- a/packages/@react-spectrum/s2/stories/TableView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TableView.stories.tsx @@ -1458,6 +1458,114 @@ let editableColumns: Array & {name: string}> = [ interface EditableTableProps extends TableViewProps {} export const EditableTable: StoryObj = { + render: function EditableTable(props) { + let columns = editableColumns; + let [editableItems, setEditableItems] = useState(defaultItems); + let intermediateValue = useRef(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 ( +
+ + + {(column) => ( + {column.name} + )} + + + {item => ( + + {(column) => { + if (column.id === 'fruits') { + return ( + onChange(item.id, column.id!)} + onCancel={() => {}} + isSaving={item.isSaving[column.id!]} + renderEditing={() => ( + value.length > 0 ? null : 'Fruit name is required'} + styles={style({flexGrow: 1, flexShrink: 1, minWidth: 0})} + defaultValue={item[column.id!]} + onChange={value => onIntermediateChange(value)} /> + )}> +
+ {item[column.id]} + + +
+ + ); + } + if (column.id === 'farmer') { + return ( + onChange(item.id, column.id!)} + onCancel={() => {}} + isSaving={item.isSaving[column.id!]} + renderEditing={() => ( + onIntermediateChange(value)}> + Eva + Steven + Michael + Sara + Karina + Otto + Matt + Emily + Amelia + Isla + + )}> +
{item[column.id]}
+ + ); + } + if (column.id === 'status') { + return ( + + {item[column.id]} + + ); + } + return {item[column.id!]}; + }} + + )} + + +
+ ); + } +}; + +export const EditableTableWithAsyncSaving: StoryObj = { render: function EditableTable(props) { let columns = editableColumns; let [editableItems, setEditableItems] = useState(defaultItems); diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index a36b23e1559..745e36b9959 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/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( + + ); + + 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'); + }); }); }); diff --git a/packages/react-aria-components/stories/Popover.stories.tsx b/packages/react-aria-components/stories/Popover.stories.tsx index 6f4ba5e19cd..74850fe79e8 100644 --- a/packages/react-aria-components/stories/Popover.stories.tsx +++ b/packages/react-aria-components/stories/Popover.stories.tsx @@ -469,7 +469,7 @@ export const PopoverTriggerWidthExample: PopoverStory = () => ( function ScrollingBoundaryContainerExample(args) { let [boundaryElem, setBoundaryElem] = useState(null); return ( -
+
@@ -483,7 +483,7 @@ function ScrollingBoundaryContainerExample(args) { zIndex: 5 }}> - Should match the width of the trigger button + This is some dummy content for the popover