Skip to content

Commit 6f2a8fb

Browse files
snowystingerLFDanLu
authored and
devongovett
committed
fix: Inline editing tables (#8983)
* fix: Inline editing tables * fix edit buttons not showing up by default on Android phones * fix case where user cant tab forward in a table w/ edit buttons and no selection * make scrolling boundary story more obvious * fix text * fix lint --------- Co-authored-by: Daniel Lu <[email protected]>
1 parent 4ae6ad0 commit 6f2a8fb

File tree

5 files changed

+186
-11
lines changed

5 files changed

+186
-11
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
13+
import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
1414
import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus';
1515
import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared';
1616
import {flushSync} from 'react-dom';
@@ -312,7 +312,10 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
312312
}
313313
} while (last);
314314

315-
if (next && !next.contains(document.activeElement)) {
315+
// 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
316+
// the containing element. We need to special case this so that tab will move focus out of the grid instead of looping between
317+
// focusing the containing cell and back to the non-tabbable child element
318+
if (next && (!next.contains(document.activeElement) || (document.activeElement && !isTabbable(document.activeElement)))) {
316319
focusWithoutScrolling(next);
317320
}
318321
}

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import Chevron from '../ui-icons/Chevron';
5757
import Close from '../s2wf-icons/S2_Icon_Close_20_N.svg';
5858
import {ColumnSize} from '@react-types/table';
5959
import {DOMRef, DOMRefValue, forwardRefType, GlobalDOMAttributes, LoadingState, Node} from '@react-types/shared';
60+
import {getActiveElement, getOwnerDocument, useLayoutEffect, useObjectRef} from '@react-aria/utils';
6061
import {GridNode} from '@react-types/grid';
6162
import {IconContext} from './Icon';
6263
// @ts-ignore
@@ -66,12 +67,11 @@ import {Menu, MenuItem, MenuSection, MenuTrigger} from './Menu';
6667
import Nubbin from '../ui-icons/S2_MoveHorizontalTableWidget.svg';
6768
import {ProgressCircle} from './ProgressCircle';
6869
import {raw} from '../style/style-macro' with {type: 'macro'};
69-
import React, {createContext, CSSProperties, ForwardedRef, forwardRef, ReactElement, ReactNode, RefObject, useCallback, useContext, useMemo, useRef, useState} from 'react';
70+
import React, {createContext, CSSProperties, ForwardedRef, forwardRef, ReactElement, ReactNode, RefObject, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
7071
import SortDownArrow from '../s2wf-icons/S2_Icon_SortDown_20_N.svg';
7172
import SortUpArrow from '../s2wf-icons/S2_Icon_SortUp_20_N.svg';
7273
import {useActionBarContainer} from './ActionBar';
7374
import {useDOMRef} from '@react-spectrum/utils';
74-
import {useLayoutEffect, useObjectRef} from '@react-aria/utils';
7575
import {useLocalizedStringFormatter} from '@react-aria/i18n';
7676
import {useScale} from './utils';
7777
import {useSpectrumContextProps} from './useSpectrumContextProps';
@@ -1115,6 +1115,18 @@ export const EditableCell = forwardRef(function EditableCell(props: EditableCell
11151115
);
11161116
});
11171117

1118+
const nonTextInputTypes = new Set([
1119+
'checkbox',
1120+
'radio',
1121+
'range',
1122+
'color',
1123+
'file',
1124+
'image',
1125+
'button',
1126+
'submit',
1127+
'reset'
1128+
]);
1129+
11181130
function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean, cellRef: RefObject<HTMLDivElement>}) {
11191131
let {children, align, renderEditing, isSaving, onSubmit, onCancel, isFocusVisible, cellRef} = props;
11201132
let [isOpen, setIsOpen] = useState(false);
@@ -1134,7 +1146,6 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
11341146
size = 'L';
11351147
}
11361148

1137-
11381149
// Popover positioning
11391150
useLayoutEffect(() => {
11401151
if (!isOpen) {
@@ -1151,6 +1162,24 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
11511162
setTableWidth(tableWidth);
11521163
}, [cellRef, density, isOpen]);
11531164

1165+
// Auto select the entire text range of the autofocused input on overlay opening
1166+
// Maybe replace with FocusScope or one of those utilities
1167+
useEffect(() => {
1168+
if (isOpen) {
1169+
let activeElement = getActiveElement(getOwnerDocument(formRef.current));
1170+
if (activeElement
1171+
&& formRef.current?.contains(activeElement)
1172+
// not going to handle contenteditable https://stackoverflow.com/questions/6139107/programmatically-select-text-in-a-contenteditable-html-element
1173+
// seems like an edge case anyways
1174+
&& (
1175+
(activeElement instanceof HTMLInputElement && !nonTextInputTypes.has(activeElement.type))
1176+
|| activeElement instanceof HTMLTextAreaElement)
1177+
&& typeof activeElement.select === 'function') {
1178+
activeElement.select();
1179+
}
1180+
}
1181+
}, [isOpen]);
1182+
11541183
// Cancel, don't save the value
11551184
let cancel = () => {
11561185
setIsOpen(false);
@@ -1178,7 +1207,7 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
11781207
isForcedVisible: 'visible',
11791208
':is([role="row"]:hover *)': 'visible',
11801209
':is([role="row"][data-focus-visible-within] *)': 'visible',
1181-
'@media not (any-pointer: fine)': 'visible'
1210+
'@media not ((hover: hover) and (pointer: fine))': 'visible'
11821211
}
11831212
})({isForcedVisible: isOpen || !!isSaving})
11841213
}
@@ -1225,7 +1254,7 @@ function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean,
12251254
onSubmit();
12261255
setIsOpen(false);
12271256
}}
1228-
className={style({width: 'full', display: 'flex', alignItems: 'baseline', gap: 16})}
1257+
className={style({width: 'full', display: 'flex', alignItems: 'start', gap: 16})}
12291258
style={{'--input-width': `calc(${triggerWidth}px - 32px)`} as CSSProperties}>
12301259
{renderEditing()}
12311260
<div className={style({display: 'flex', flexDirection: 'row', alignItems: 'baseline', flexShrink: 0, flexGrow: 0})}>

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,6 +1458,114 @@ let editableColumns: Array<Omit<ColumnProps, 'children'> & {name: string}> = [
14581458
interface EditableTableProps extends TableViewProps {}
14591459

14601460
export const EditableTable: StoryObj<EditableTableProps> = {
1461+
render: function EditableTable(props) {
1462+
let columns = editableColumns;
1463+
let [editableItems, setEditableItems] = useState(defaultItems);
1464+
let intermediateValue = useRef<any>(null);
1465+
1466+
let onChange = useCallback((id: Key, columnId: Key) => {
1467+
let value = intermediateValue.current;
1468+
if (value === null) {
1469+
return;
1470+
}
1471+
intermediateValue.current = null;
1472+
setEditableItems(prev => {
1473+
let newItems = prev.map(i => i.id === id && i[columnId] !== value ? {...i, [columnId]: value} : i);
1474+
return newItems;
1475+
});
1476+
}, []);
1477+
1478+
let onIntermediateChange = useCallback((value: any) => {
1479+
intermediateValue.current = value;
1480+
}, []);
1481+
1482+
return (
1483+
<div className={style({display: 'flex', flexDirection: 'column', gap: 16})}>
1484+
<TableView aria-label="Dynamic table" {...props} styles={style({width: 800, maxWidth: 'calc(100vw - 2rem)', height: 208})}>
1485+
<TableHeader columns={columns}>
1486+
{(column) => (
1487+
<Column {...column}>{column.name}</Column>
1488+
)}
1489+
</TableHeader>
1490+
<TableBody items={editableItems}>
1491+
{item => (
1492+
<Row id={item.id} columns={columns}>
1493+
{(column) => {
1494+
if (column.id === 'fruits') {
1495+
return (
1496+
<EditableCell
1497+
align={column.align}
1498+
showDivider={column.showDivider}
1499+
onSubmit={() => onChange(item.id, column.id!)}
1500+
onCancel={() => {}}
1501+
isSaving={item.isSaving[column.id!]}
1502+
renderEditing={() => (
1503+
<TextField
1504+
aria-label="Edit fruit"
1505+
autoFocus
1506+
validate={value => value.length > 0 ? null : 'Fruit name is required'}
1507+
styles={style({flexGrow: 1, flexShrink: 1, minWidth: 0})}
1508+
defaultValue={item[column.id!]}
1509+
onChange={value => onIntermediateChange(value)} />
1510+
)}>
1511+
<div className={style({display: 'flex', alignItems: 'center', gap: 8, justifyContent: 'space-between'})}>
1512+
{item[column.id]}
1513+
<ActionButton slot="edit" aria-label="Edit fruit">
1514+
<Edit />
1515+
</ActionButton></div>
1516+
</EditableCell>
1517+
);
1518+
}
1519+
if (column.id === 'farmer') {
1520+
return (
1521+
<EditableCell
1522+
align={column.align}
1523+
showDivider={column.showDivider}
1524+
onSubmit={() => onChange(item.id, column.id!)}
1525+
onCancel={() => {}}
1526+
isSaving={item.isSaving[column.id!]}
1527+
renderEditing={() => (
1528+
<Picker
1529+
aria-label="Edit farmer"
1530+
autoFocus
1531+
styles={style({flexGrow: 1, flexShrink: 1, minWidth: 0})}
1532+
defaultValue={item[column.id!]}
1533+
onChange={value => onIntermediateChange(value)}>
1534+
<PickerItem textValue="Eva" id="Eva"><User /><Text>Eva</Text></PickerItem>
1535+
<PickerItem textValue="Steven" id="Steven"><User /><Text>Steven</Text></PickerItem>
1536+
<PickerItem textValue="Michael" id="Michael"><User /><Text>Michael</Text></PickerItem>
1537+
<PickerItem textValue="Sara" id="Sara"><User /><Text>Sara</Text></PickerItem>
1538+
<PickerItem textValue="Karina" id="Karina"><User /><Text>Karina</Text></PickerItem>
1539+
<PickerItem textValue="Otto" id="Otto"><User /><Text>Otto</Text></PickerItem>
1540+
<PickerItem textValue="Matt" id="Matt"><User /><Text>Matt</Text></PickerItem>
1541+
<PickerItem textValue="Emily" id="Emily"><User /><Text>Emily</Text></PickerItem>
1542+
<PickerItem textValue="Amelia" id="Amelia"><User /><Text>Amelia</Text></PickerItem>
1543+
<PickerItem textValue="Isla" id="Isla"><User /><Text>Isla</Text></PickerItem>
1544+
</Picker>
1545+
)}>
1546+
<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>
1547+
</EditableCell>
1548+
);
1549+
}
1550+
if (column.id === 'status') {
1551+
return (
1552+
<Cell align={column.align} showDivider={column.showDivider}>
1553+
<StatusLight variant="informative">{item[column.id]}</StatusLight>
1554+
</Cell>
1555+
);
1556+
}
1557+
return <Cell align={column.align} showDivider={column.showDivider}>{item[column.id!]}</Cell>;
1558+
}}
1559+
</Row>
1560+
)}
1561+
</TableBody>
1562+
</TableView>
1563+
</div>
1564+
);
1565+
}
1566+
};
1567+
1568+
export const EditableTableWithAsyncSaving: StoryObj<EditableTableProps> = {
14611569
render: function EditableTable(props) {
14621570
let columns = editableColumns;
14631571
let [editableItems, setEditableItems] = useState(defaultItems);

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ describe('TableView', () => {
258258
let input = within(dialog).getByRole('textbox');
259259
expect(input).toHaveFocus();
260260

261-
await user.keyboard(' Crisp');
261+
await user.keyboard('Apples Crisp');
262262
await user.keyboard('{Enter}'); // implicitly submit through form
263263

264264
act(() => {jest.runAllTimers();});
@@ -429,7 +429,7 @@ describe('TableView', () => {
429429
let input = within(dialog).getByRole('textbox');
430430
expect(input).toHaveFocus();
431431

432-
await user.keyboard(' Crisp');
432+
await user.keyboard('Apples Crisp');
433433
await user.keyboard('{Enter}'); // implicitly submit through form
434434

435435
act(() => {jest.advanceTimersByTime(5000);});
@@ -445,5 +445,40 @@ describe('TableView', () => {
445445
expect(button).not.toHaveAttribute('aria-disabled');
446446
expect(button).toHaveFocus();
447447
});
448+
449+
it('should allow tabbing off a pending button', async () => {
450+
let {getByRole} = render(
451+
<EditableTable delay={10000} />
452+
);
453+
454+
let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')});
455+
await user.tab();
456+
await user.keyboard('{ArrowRight}');
457+
await user.keyboard('{Enter}');
458+
459+
let dialog = getByRole('dialog');
460+
expect(dialog).toBeVisible();
461+
462+
let input = within(dialog).getByRole('textbox');
463+
expect(input).toHaveFocus();
464+
465+
await user.keyboard('Apples Crisp');
466+
await user.keyboard('{Enter}'); // implicitly submit through form
467+
468+
act(() => {jest.advanceTimersByTime(5000);});
469+
470+
expect(dialog).not.toBeInTheDocument();
471+
expect(tableTester.findRow({rowIndexOrText: 'Apples Crisp'})).toBeInTheDocument();
472+
let button = within(tableTester.findCell({text: 'Apples Crisp'})).getByRole('button');
473+
expect(button).toHaveAttribute('aria-disabled', 'true');
474+
expect(button).toHaveFocus();
475+
476+
await user.tab();
477+
expect(getByRole('button', {name: 'After'})).toHaveFocus();
478+
479+
act(() => {jest.runAllTimers();});
480+
481+
expect(button).not.toHaveAttribute('aria-disabled');
482+
});
448483
});
449484
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ export const PopoverTriggerWidthExample: PopoverStory = () => (
469469
function ScrollingBoundaryContainerExample(args) {
470470
let [boundaryElem, setBoundaryElem] = useState<HTMLDivElement | null>(null);
471471
return (
472-
<div id="scrolling-boundary" ref={setBoundaryElem} style={{height: 300, width: 300, overflow: 'auto'}}>
472+
<div id="scrolling-boundary" ref={setBoundaryElem} style={{height: 300, width: 300, overflow: 'auto', border: '1px solid light-dark(black, white)'}}>
473473
<div style={{width: 600, height: 600, display: 'flex', alignItems: 'center', justifyContent: 'center'}}>
474474
<DialogTrigger>
475475
<Button style={{width: 200, height: 200, display: 'flex', alignItems: 'center', justifyContent: 'center'}}>Open popover</Button>
@@ -483,7 +483,7 @@ function ScrollingBoundaryContainerExample(args) {
483483
zIndex: 5
484484
}}>
485485
<Dialog>
486-
Should match the width of the trigger button
486+
This is some dummy content for the popover
487487
</Dialog>
488488
</Popover>
489489
</DialogTrigger>

0 commit comments

Comments
 (0)