diff --git a/packages/@react-aria/grid/src/GridKeyboardDelegate.ts b/packages/@react-aria/grid/src/GridKeyboardDelegate.ts index cd9aa9cb089..5755a5214b4 100644 --- a/packages/@react-aria/grid/src/GridKeyboardDelegate.ts +++ b/packages/@react-aria/grid/src/GridKeyboardDelegate.ts @@ -381,4 +381,3 @@ export class GridKeyboardDelegate> implements Key return null; } } - diff --git a/packages/@react-aria/grid/src/useGridCell.ts b/packages/@react-aria/grid/src/useGridCell.ts index 285f0c96aaa..edeb40f08b8 100644 --- a/packages/@react-aria/grid/src/useGridCell.ts +++ b/packages/@react-aria/grid/src/useGridCell.ts @@ -67,6 +67,11 @@ export function useGridCell>(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; diff --git a/packages/@react-spectrum/list/src/ListView.tsx b/packages/@react-spectrum/list/src/ListView.tsx index fa3bc65280d..c2fd4e81952 100644 --- a/packages/@react-spectrum/list/src/ListView.tsx +++ b/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 { state: GridState>, - keyboardDelegate: GridKeyboardDelegate>, dragState: DraggableCollectionState, - onAction:(key: string) => void, isListDraggable: boolean, layout: ListLayout } @@ -74,7 +72,8 @@ export function useListLayout(state: ListState, density: ListViewProps[ 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(props: ListViewProps, ref: DOMRef new GridCollection({ columnCount: 1, items: [...collection].map(item => ({ @@ -151,21 +149,10 @@ function ListView(props: ListViewProps, ref: DOMRef 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,21 +170,15 @@ function ListView(props: ListViewProps, ref: DOMRef + (props: ListViewProps, ref: DOMRef(props: ListViewProps, ref: DOMRef { if (type === 'item') { return ( - + ); } else if (type === 'loader') { return ( diff --git a/packages/@react-spectrum/list/src/ListViewItem.tsx b/packages/@react-spectrum/list/src/ListViewItem.tsx index 69284ab0c5e..971c567c16a 100644 --- a/packages/@react-spectrum/list/src/ListViewItem.tsx +++ b/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(); - let cellRef = useRef(); 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 (
+ role="gridcell" + aria-colindex={1}> {isListDraggable &&
diff --git a/packages/@react-spectrum/list/stories/ListView.stories.tsx b/packages/@react-spectrum/list/stories/ListView.stories.tsx index 24c0877a69f..302757f8697 100644 --- a/packages/@react-spectrum/list/stories/ListView.stories.tsx +++ b/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() : + ( + <> + + + {storyFn()} + + + + ); +}; + storiesOf('ListView', module) + .addDecorator(decorator) .add('default', () => ( Adobe Photoshop diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 0ce9fc5870e..8f32d723df5 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -37,7 +37,7 @@ function pointerEvent(type, opts) { } describe('ListView', function () { - let offsetWidth, offsetHeight; + let offsetWidth, offsetHeight, scrollHeight; let onSelectionChange = jest.fn(); let onAction = jest.fn(); let onDragStart = jest.fn(); @@ -48,10 +48,21 @@ describe('ListView', function () { expect(onSelectionChange).toHaveBeenCalledTimes(1); expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(selectedKeys)); }; + let items = [ + {key: 'foo', label: 'Foo'}, + {key: 'bar', label: 'Bar'}, + {key: 'baz', label: 'Baz'} + ]; + + let manyItems = []; + for (let i = 1; i <= 100; i++) { + manyItems.push({id: i, label: 'Foo ' + i}); + } beforeAll(function () { offsetWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); offsetHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); + scrollHeight = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 40); jest.useFakeTimers(); }); @@ -62,6 +73,7 @@ describe('ListView', function () { afterAll(function () { offsetWidth.mockReset(); offsetHeight.mockReset(); + scrollHeight.mockReset(); }); let render = (children, locale = 'en-US', scale = 'medium') => { @@ -75,10 +87,50 @@ describe('ListView', function () { return tree; }; - let getCell = (tree, text) => { - // Find by text, then go up to the element with the cell role. + let renderList = (props = {}) => { + let { + locale, + scale, + ...otherProps + } = props; + return render( + + {item => ( + + {item.label} + + )} + , + locale, + scale + ); + }; + + let renderListWithFocusables = (props = {}) => { + let { + locale, + scale, + ...otherProps + } = props; + return render( + + {item => ( + + {item.label} + button1 {item.label} + button2 {item.label} + + )} + , + locale, + scale + ); + }; + + let getRow = (tree, text) => { + // Find by text, then go up to the element with the row role. let el = tree.getByText(text); - while (el && !/gridcell|rowheader|columnheader/.test(el.getAttribute('role'))) { + while (el && !/row/.test(el.getAttribute('role'))) { el = el.parentElement; } @@ -114,7 +166,7 @@ describe('ListView', function () { expect(gridCells[0]).toHaveTextContent('Foo'); }); - it('renders a dynamic table', function () { + it('renders a dynamic listview', function () { let items = [ {key: 'foo', label: 'Foo'}, {key: 'bar', label: 'Bar'}, @@ -166,42 +218,37 @@ describe('ListView', function () { expect(gridCells[0]).toHaveTextContent('Foo'); }); - describe('keyboard focus', function () { - let items = [ - {key: 'foo', label: 'Foo'}, - {key: 'bar', label: 'Bar'}, - {key: 'baz', label: 'Baz'} - ]; - let renderList = () => render( - - {item => ( - - {item.label} - - )} - - ); + it('should retain focus on the pressed child', function () { + let tree = renderListWithFocusables(); + let button = within(getRow(tree, 'Foo')).getAllByRole('button')[1]; + triggerPress(button); + expect(document.activeElement).toBe(button); + }); - let renderListWithFocusables = (locale, scale) => render( - - {item => ( - - {item.label} - button1 {item.label} - button2 {item.label} - - )} - , - locale, - scale - ); + it('should focus the row if the cell is pressed', function () { + let tree = renderList({selectionMode: 'single'}); + let cell = within(getRow(tree, 'Bar')).getByRole('gridcell'); + triggerPress(cell); + act(() => { + jest.runAllTimers(); + }); + expect(document.activeElement).toBe(getRow(tree, 'Bar')); + }); + + it('should have an aria-label on the row for the row text content', function () { + let tree = renderList(); + expect(getRow(tree, 'Foo')).toHaveAttribute('aria-label', 'Foo'); + expect(getRow(tree, 'Bar')).toHaveAttribute('aria-label', 'Bar'); + expect(getRow(tree, 'Baz')).toHaveAttribute('aria-label', 'Baz'); + }); + describe('keyboard focus', function () { describe('Type to select', function () { it('focuses the correct cell when typing', function () { let tree = renderList(); - let target = getCell(tree, 'Baz'); + let target = getRow(tree, 'Baz'); let grid = tree.getByRole('grid'); - act(() => grid.focus()); + userEvent.tab(); fireEvent.keyDown(grid, {key: 'B'}); fireEvent.keyUp(grid, {key: 'Enter'}); fireEvent.keyDown(grid, {key: 'A'}); @@ -215,8 +262,8 @@ describe('ListView', function () { describe('ArrowRight', function () { it('should not move focus if no focusables present', function () { let tree = renderList(); - let start = getCell(tree, 'Foo'); - act(() => start.focus()); + let start = getRow(tree, 'Foo'); + userEvent.tab(); moveFocus('ArrowRight'); expect(document.activeElement).toBe(start); }); @@ -224,9 +271,9 @@ describe('ListView', function () { describe('with cell focusables', function () { it('should move focus to next cell and back to row', function () { let tree = renderListWithFocusables(); - let focusables = within(tree.getAllByRole('row')[0]).getAllByRole('button'); - let start = getCell(tree, 'Foo'); - act(() => start.focus()); + let start = getRow(tree, 'Foo'); + let focusables = within(start).getAllByRole('button'); + userEvent.tab(); moveFocus('ArrowRight'); expect(document.activeElement).toBe(focusables[0]); moveFocus('ArrowRight'); @@ -236,12 +283,14 @@ describe('ListView', function () { }); it('should move focus to previous cell in RTL', function () { - let tree = renderListWithFocusables('ar-AE'); + let tree = renderListWithFocusables({locale: 'ar-AE'}); // Should move from button two to button one - let start = within(tree.getAllByRole('row')[0]).getAllByRole('button')[1]; - let end = within(tree.getAllByRole('row')[0]).getAllByRole('button')[0]; - act(() => start.focus()); + let start = within(getRow(tree, 'Foo')).getAllByRole('button')[1]; + let end = within(getRow(tree, 'Foo')).getAllByRole('button')[0]; + // Need to press to set a modality, otherwise useSelectableCollection will think this is a tab operation + triggerPress(start); expect(document.activeElement).toHaveTextContent('button2 Foo'); + expect(document.activeElement).toBe(start); moveFocus('ArrowRight'); expect(document.activeElement).toBe(end); expect(document.activeElement).toHaveTextContent('button1 Foo'); @@ -252,8 +301,8 @@ describe('ListView', function () { describe('ArrowLeft', function () { it('should not move focus if no focusables present', function () { let tree = renderList(); - let start = getCell(tree, 'Foo'); - act(() => start.focus()); + let start = getRow(tree, 'Foo'); + userEvent.tab(); moveFocus('ArrowLeft'); expect(document.activeElement).toBe(start); }); @@ -261,10 +310,9 @@ describe('ListView', function () { describe('with cell focusables', function () { it('should move focus to previous cell and back to row', function () { let tree = renderListWithFocusables(); - let focusables = within(tree.getAllByRole('row')[0]).getAllByRole('button'); - let start = getCell(tree, 'Foo'); - // console.log('start', start) - act(() => start.focus()); + let focusables = within(getRow(tree, 'Foo')).getAllByRole('button'); + let start = getRow(tree, 'Foo'); + userEvent.tab(); moveFocus('ArrowLeft'); expect(document.activeElement).toBe(focusables[1]); moveFocus('ArrowLeft'); @@ -274,12 +322,14 @@ describe('ListView', function () { }); it('should move focus to next cell in RTL', function () { - let tree = renderListWithFocusables('ar-AE'); + let tree = renderListWithFocusables({locale: 'ar-AE'}); // Should move from button one to button two - let start = within(tree.getAllByRole('row')[0]).getAllByRole('button')[0]; - let end = within(tree.getAllByRole('row')[0]).getAllByRole('button')[1]; - act(() => start.focus()); + let start = within(getRow(tree, 'Foo')).getAllByRole('button')[0]; + let end = within(getRow(tree, 'Foo')).getAllByRole('button')[1]; + // Need to press to set a modality, otherwise useSelectableCollection will think this is a tab operation + triggerPress(start); expect(document.activeElement).toHaveTextContent('button1 Foo'); + expect(document.activeElement).toBe(start); moveFocus('ArrowLeft'); expect(document.activeElement).toBe(end); expect(document.activeElement).toHaveTextContent('button2 Foo'); @@ -288,42 +338,142 @@ describe('ListView', function () { }); describe('ArrowUp', function () { - it('should not change focus from first item', function () { + it('should not wrap focus', function () { let tree = renderListWithFocusables(); - let start = getCell(tree, 'Foo'); - act(() => start.focus()); + let start = getRow(tree, 'Foo'); + userEvent.tab(); moveFocus('ArrowUp'); expect(document.activeElement).toBe(start); }); it('should move focus to above row', function () { - let tree = renderListWithFocusables(); - let start = getCell(tree, 'Bar'); - let end = getCell(tree, 'Foo'); - act(() => start.focus()); + let tree = renderListWithFocusables({selectionMode: 'single'}); + let start = getRow(tree, 'Bar'); + let end = getRow(tree, 'Foo'); + triggerPress(start); + moveFocus('ArrowUp'); + expect(document.activeElement).toBe(end); + }); + + it('should allow focus on disabled rows', function () { + let tree = renderListWithFocusables({disabledKeys: ['foo'], selectionMode: 'single'}); + let start = getRow(tree, 'Bar'); + let end = getRow(tree, 'Foo'); + triggerPress(start); moveFocus('ArrowUp'); expect(document.activeElement).toBe(end); }); }); describe('ArrowDown', function () { - it('should not change focus from first item', function () { - let tree = renderListWithFocusables(); - let start = getCell(tree, 'Baz'); - act(() => start.focus()); + it('should not wrap focus', function () { + let tree = renderListWithFocusables({selectionMode: 'single'}); + let start = getRow(tree, 'Baz'); + triggerPress(start); moveFocus('ArrowDown'); expect(document.activeElement).toBe(start); }); it('should move focus to below row', function () { - let tree = renderListWithFocusables(); - let start = getCell(tree, 'Foo'); - let end = getCell(tree, 'Bar'); - act(() => start.focus()); + let tree = renderListWithFocusables({selectionMode: 'single'}); + let start = getRow(tree, 'Foo'); + let end = getRow(tree, 'Bar'); + triggerPress(start); + moveFocus('ArrowDown'); + expect(document.activeElement).toBe(end); + }); + + it('should allow focus on disabled rows', function () { + let tree = renderListWithFocusables({disabledKeys: ['bar'], selectionMode: 'single'}); + let start = getRow(tree, 'Foo'); + let end = getRow(tree, 'Bar'); + triggerPress(start); moveFocus('ArrowDown'); expect(document.activeElement).toBe(end); }); }); + + describe('PageUp', function () { + it('should move focus to a row a page above when focus starts on a row', function () { + let tree = renderListWithFocusables({items: manyItems, selectionMode: 'single'}); + let start = getRow(tree, 'Foo 25'); + triggerPress(start); + moveFocus('PageUp'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 1')); + }); + + it('should move focus to a row a page above when focus starts in the row cell', function () { + let tree = renderListWithFocusables({items: manyItems}); + let focusables = within(getRow(tree, 'Foo 25')).getAllByRole('button'); + let start = focusables[0]; + triggerPress(start); + expect(document.activeElement).toBe(start); + moveFocus('PageUp'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 1')); + }); + }); + + describe('PageDown', function () { + it('should move focus to a row a page below when focus starts on a row', function () { + let tree = renderListWithFocusables({items: manyItems, selectionMode: 'single'}); + userEvent.tab(); + moveFocus('PageDown'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 25')); + moveFocus('PageDown'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 49')); + }); + + it('should move focus to a row a page below when focus starts in the row cell', function () { + let tree = renderListWithFocusables({items: manyItems}); + let focusables = within(getRow(tree, 'Foo 1')).getAllByRole('button'); + let start = focusables[0]; + triggerPress(start); + expect(document.activeElement).toBe(start); + moveFocus('PageDown'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 25')); + moveFocus('PageDown'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 49')); + }); + }); + + describe('Home', function () { + it('should move focus to the first row when focus starts on a row', function () { + let tree = renderListWithFocusables({items: manyItems, selectionMode: 'single'}); + let start = getRow(tree, 'Foo 15'); + triggerPress(start); + moveFocus('Home'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 1')); + }); + + it('should move focus to the first row when focus starts in the row cell', function () { + let tree = renderListWithFocusables({items: manyItems}); + let focusables = within(getRow(tree, 'Foo 15')).getAllByRole('button'); + let start = focusables[0]; + triggerPress(start); + expect(document.activeElement).toBe(start); + moveFocus('Home'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 1')); + }); + }); + + describe('End', function () { + it('should move focus to the last row when focus starts on a row', function () { + let tree = renderListWithFocusables({items: manyItems}); + userEvent.tab(); + moveFocus('End'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 100')); + }); + + it('should move focus to the last row when focus starts in the row cell', function () { + let tree = renderListWithFocusables({items: manyItems}); + let focusables = within(getRow(tree, 'Foo 1')).getAllByRole('button'); + let start = focusables[0]; + triggerPress(start); + expect(document.activeElement).toBe(start); + moveFocus('End'); + expect(document.activeElement).toBe(getRow(tree, 'Foo 100')); + }); + }); }); it('should display loading affordance with proper height (isLoading)', function () { @@ -464,13 +614,13 @@ describe('ListView', function () { let rows = tree.getAllByRole('row'); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); expect(rows[2]).toHaveAttribute('aria-selected', 'false'); - act(() => userEvent.click(getCell(tree, 'Bar'), {ctrlKey: true})); + act(() => userEvent.click(getRow(tree, 'Bar'), {ctrlKey: true})); checkSelection(onSelectionChange, ['bar']); expect(rows[1]).toHaveAttribute('aria-selected', 'true'); onSelectionChange.mockClear(); - act(() => userEvent.click(getCell(tree, 'Baz'), {ctrlKey: true})); + act(() => userEvent.click(getRow(tree, 'Baz'), {ctrlKey: true})); checkSelection(onSelectionChange, ['baz']); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); expect(rows[2]).toHaveAttribute('aria-selected', 'true'); @@ -486,13 +636,13 @@ describe('ListView', function () { expect(rows[0]).toHaveAttribute('aria-selected', 'false'); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); expect(rows[2]).toHaveAttribute('aria-selected', 'false'); - act(() => userEvent.click(getCell(tree, 'Foo'), {ctrlKey: true})); + act(() => userEvent.click(getRow(tree, 'Foo'), {ctrlKey: true})); checkSelection(onSelectionChange, ['foo']); expect(rows[0]).toHaveAttribute('aria-selected', 'true'); onSelectionChange.mockClear(); - act(() => userEvent.click(getCell(tree, 'Baz'), {ctrlKey: true})); + act(() => userEvent.click(getRow(tree, 'Baz'), {ctrlKey: true})); checkSelection(onSelectionChange, ['foo', 'baz']); expect(rows[0]).toHaveAttribute('aria-selected', 'true'); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); @@ -508,13 +658,13 @@ describe('ListView', function () { let rows = tree.getAllByRole('row'); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); expect(rows[2]).toHaveAttribute('aria-selected', 'false'); - act(() => userEvent.click(getCell(tree, 'Bar'), {metaKey: true})); + act(() => userEvent.click(getRow(tree, 'Bar'), {metaKey: true})); checkSelection(onSelectionChange, ['bar']); expect(rows[1]).toHaveAttribute('aria-selected', 'true'); onSelectionChange.mockClear(); - act(() => userEvent.click(getCell(tree, 'Baz'), {metaKey: true})); + act(() => userEvent.click(getRow(tree, 'Baz'), {metaKey: true})); checkSelection(onSelectionChange, ['baz']); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); expect(rows[2]).toHaveAttribute('aria-selected', 'true'); @@ -576,7 +726,7 @@ describe('ListView', function () { let row = tree.getAllByRole('row')[1]; expect(row).toHaveAttribute('aria-selected', 'false'); - act(() => userEvent.click(getCell(tree, 'Bar'), {ctrlKey: true})); + act(() => userEvent.click(getRow(tree, 'Bar'), {ctrlKey: true})); checkSelection(onSelectionChange, ['bar']); expect(row).toHaveAttribute('aria-selected', 'true'); @@ -597,14 +747,7 @@ describe('ListView', function () { }); describe('scrolling', function () { - beforeAll(() => { - jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get') - .mockImplementation(function () { - return 40; - }); - }); - - it('should scroll to a cell when it is focused', function () { + it('should scroll to a row when it is focused', function () { let tree = render( cell.focus()); + userEvent.tab(); let draghandle = within(cell).getAllByRole('button')[0]; expect(draghandle).toBeTruthy(); expect(draghandle).toHaveAttribute('draggable', 'true'); @@ -987,7 +1130,7 @@ describe('ListView', function () { expect(cellD).toHaveTextContent('Adobe InDesign'); expect(rows[3]).toHaveAttribute('draggable', 'true'); - act(() => cellA.focus()); + userEvent.tab(); let draghandle = within(cellA).getAllByRole('button')[0]; expect(draghandle).toBeTruthy(); @@ -1075,26 +1218,32 @@ describe('ListView', function () { it('should only display the drag handle on keyboard focus for dragggable items', function () { let {getAllByRole} = render( - + ); let rows = getAllByRole('row'); let cellA = within(rows[0]).getByRole('gridcell'); + triggerPress(cellA); + expect(document.activeElement).toBe(rows[0]); let dragHandle = within(cellA).getAllByRole('button')[0]; // If the dragHandle has a style applied, it is visually hidden expect(dragHandle.style).toBeTruthy(); + expect(dragHandle.style.position).toBe('absolute'); - fireEvent.pointerDown(cellA, {button: 0, pointerId: 1}); + fireEvent.pointerDown(rows[0], {button: 0, pointerId: 1}); dragHandle = within(cellA).getAllByRole('button')[0]; expect(dragHandle.style).toBeTruthy(); - fireEvent.pointerUp(cellA, {button: 0, pointerId: 1}); + expect(dragHandle.style.position).toBe('absolute'); + fireEvent.pointerUp(rows[0], {button: 0, pointerId: 1}); - fireEvent.pointerEnter(cellA); + fireEvent.pointerEnter(rows[0]); dragHandle = within(cellA).getAllByRole('button')[0]; expect(dragHandle.style).toBeTruthy(); + expect(dragHandle.style.position).toBe('absolute'); // If dragHandle doesn't have a position applied, it isn't visually hidden - act(() => cellA.focus()); + fireEvent.keyDown(rows[0], {key: 'Enter'}); + fireEvent.keyUp(rows[0], {key: 'Enter'}); dragHandle = within(cellA).getAllByRole('button')[0]; expect(dragHandle.style.position).toBe(''); }); @@ -1121,7 +1270,7 @@ describe('ListView', function () { let cellA = within(rows[0]).getByRole('gridcell'); let cellB = within(rows[1]).getByRole('gridcell'); - act(() => cellA.focus()); + userEvent.tab(); expect(hasDragHandle(cellA)).toBeFalsy(); moveFocus('ArrowDown'); expect(hasDragHandle(cellB)).toBeFalsy(); diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 56015b350d7..3ec73608ed8 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -1356,6 +1356,13 @@ describe('TableView', function () { ); + it('should retain focus on the pressed child', function () { + let tree = renderFocusable(); + let switchToPress = tree.getAllByRole('switch')[2]; + act(() => triggerPress(switchToPress)); + expect(document.activeElement).toBe(switchToPress); + }); + it('should marshall focus to the focusable element inside a cell', function () { let tree = renderFocusable(); focusCell(tree, 'Baz 1'); diff --git a/packages/@react-stately/layout/src/ListLayout.ts b/packages/@react-stately/layout/src/ListLayout.ts index 75399edf6f7..34f2fd1a11b 100644 --- a/packages/@react-stately/layout/src/ListLayout.ts +++ b/packages/@react-stately/layout/src/ListLayout.ts @@ -25,7 +25,8 @@ export type ListLayoutOptions = { indentationForItem?: (collection: Collection>, key: Key) => number, collator?: Intl.Collator, loaderHeight?: number, - placeholderHeight?: number + placeholderHeight?: number, + allowDisabledKeyFocus?: boolean }; // A wrapper around LayoutInfo that supports hierarchy @@ -60,6 +61,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { protected contentSize: Size; collection: Collection>; disabledKeys: Set = new Set(); + allowDisabledKeyFocus: boolean = false; isLoading: boolean; protected lastWidth: number; protected lastCollection: Collection>; @@ -89,6 +91,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { this.rootNodes = []; this.lastWidth = 0; this.lastCollection = null; + this.allowDisabledKeyFocus = options.allowDisabledKeyFocus; } getLayoutInfo(key: Key) { @@ -350,7 +353,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { key = collection.getKeyBefore(key); while (key != null) { let item = collection.getItem(key); - if (item.type === 'item' && !this.disabledKeys.has(item.key)) { + if (item.type === 'item' && (this.allowDisabledKeyFocus || !this.disabledKeys.has(item.key))) { return key; } @@ -364,7 +367,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { key = collection.getKeyAfter(key); while (key != null) { let item = collection.getItem(key); - if (item.type === 'item' && !this.disabledKeys.has(item.key)) { + if (item.type === 'item' && (this.allowDisabledKeyFocus || !this.disabledKeys.has(item.key))) { return key; } @@ -372,6 +375,14 @@ export class ListLayout extends Layout> implements KeyboardDelegate { } } + getKeyLeftOf(key: Key): Key { + return key; + } + + getKeyRightOf(key: Key): Key { + return key; + } + getKeyPageAbove(key: Key) { let layoutInfo = this.getLayoutInfo(key); @@ -413,7 +424,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { let key = collection.getFirstKey(); while (key != null) { let item = collection.getItem(key); - if (item.type === 'item' && !this.disabledKeys.has(item.key)) { + if (item.type === 'item' && (this.allowDisabledKeyFocus || !this.disabledKeys.has(item.key))) { return key; } @@ -426,7 +437,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { let key = collection.getLastKey(); while (key != null) { let item = collection.getItem(key); - if (item.type === 'item' && !this.disabledKeys.has(item.key)) { + if (item.type === 'item' && (this.allowDisabledKeyFocus || !this.disabledKeys.has(item.key))) { return key; } diff --git a/scripts/lint-packages.js b/scripts/lint-packages.js index 964ac3a7e19..fccc80e5260 100644 --- a/scripts/lint-packages.js +++ b/scripts/lint-packages.js @@ -46,6 +46,22 @@ softAssert.equal = function (val, val2, message) { } }; +// Checks if a dependency is actually being imported somewhere +function isDepUsed(dep, src) { + let depRegex = new RegExp(`import .* from '${dep}'`); + let files = glob.sync(src, { + ignore: ['**/node_modules/**', '**/dist/**'] + }); + + for (let file of files) { + let contents = fs.readFileSync(file, 'utf8'); + if (depRegex.test(contents)) { + return true; + } + } + return false; +} + let pkgNames = {}; for (let pkg of packages) { let json = JSON.parse(fs.readFileSync(pkg)); @@ -106,29 +122,27 @@ for (let pkg of packages) { for (let pkg of packages) { + let globSrc = pkg.replace('package.json', '**/*.{js,ts,tsx}'); let json = JSON.parse(fs.readFileSync(pkg)); let [scope, basename] = json.name.split('/'); if (basename.includes('utils') || basename.includes('layout')) { continue; } - if (scope === '@react-spectrum') { - let aria = `@react-aria/${basename}`; - let stately = `@react-stately/${basename}`; - let types = `@react-types/${basename}`; + let aria = `@react-aria/${basename}`; + let stately = `@react-stately/${basename}`; + let types = `@react-types/${basename}`; + + if (scope === '@react-spectrum' && isDepUsed(aria, globSrc)) { softAssert(!pkgNames[aria] || json.dependencies[aria], `${pkg} is missing a dependency on ${aria}`); - softAssert(!pkgNames[stately] || json.dependencies[stately], `${pkg} is missing a dependency on ${stately}`); - softAssert(!pkgNames[types] || json.dependencies[types], `${pkg} is missing a dependency on ${types}`); - } else if (scope === '@react-aria') { - let stately = `@react-stately/${basename}`; - let types = `@react-types/${basename}`; + } + if ((scope === '@react-aria' || scope === '@react-spectrum') && isDepUsed(stately, globSrc)) { softAssert(!pkgNames[stately] || json.dependencies[stately], `${pkg} is missing a dependency on ${stately}`); - softAssert(!pkgNames[types] || json.dependencies[types], `${pkg} is missing a dependency on ${types}`); - } else if (scope === '@react-stately') { - let types = `@react-types/${basename}`; + } + if ((scope === '@react-aria' || scope === '@react-spectrum' || scope === '@react-stately') && isDepUsed(types, globSrc)) { softAssert(!pkgNames[types] || json.dependencies[types], `${pkg} is missing a dependency on ${types}`); } }