Skip to content

Commit cf60fcb

Browse files
authored
Skip row contents when cycling through droppable ListView rows via TalkBack (#3401)
1 parent 79aa5e5 commit cf60fcb

File tree

7 files changed

+34
-16
lines changed

7 files changed

+34
-16
lines changed

packages/@react-aria/dnd/src/DragManager.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ export function useDragSession() {
100100
return session;
101101
}
102102

103+
export function isVirtualDragging(): boolean {
104+
return !!dragSession;
105+
}
106+
103107
function endDragging() {
104108
dragSession = null;
105109
for (let cb of subscriptions) {

packages/@react-aria/dnd/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,4 @@ export {useDraggableItem} from './useDraggableItem';
2929
export {useClipboard} from './useClipboard';
3030
export {DragPreview} from './DragPreview';
3131
export {ListDropTargetDelegate} from './ListDropTargetDelegate';
32+
export {isVirtualDragging} from './DragManager';

packages/@react-aria/overlays/src/ariaHideOutside.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,16 @@ export function ariaHideOutside(targets: Element[], root = document.body) {
3636
}
3737

3838
// Skip this node and its children if it is one of the target nodes, or a live announcer.
39-
// Also skip children of already hidden nodes, as aria-hidden is recursive.
39+
// Also skip children of already hidden nodes, as aria-hidden is recursive. An exception is
40+
// made for elements with role="row" since VoiceOver on iOS has issues hiding elements with role="row".
41+
// For that case we want to hide the cells inside as well (https://bugs.webkit.org/show_bug.cgi?id=222623).
4042
if (
4143
visibleNodes.has(node as Element) ||
42-
hiddenNodes.has(node.parentElement)
44+
(hiddenNodes.has(node.parentElement) && node.parentElement.getAttribute('role') !== 'row')
4345
) {
4446
return NodeFilter.FILTER_REJECT;
4547
}
4648

47-
// VoiceOver on iOS has issues hiding elements with role="row". Hide the cells inside instead.
48-
// https://bugs.webkit.org/show_bug.cgi?id=222623
49-
if (node instanceof Element && node.getAttribute('role') === 'row') {
50-
return NodeFilter.FILTER_SKIP;
51-
}
52-
5349
// Skip this node but continue to children if one of the targets is inside the node.
5450
if (targets.some(target => node.contains(target))) {
5551
return NodeFilter.FILTER_SKIP;

packages/@react-aria/overlays/test/ariaHideOutside.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,9 @@ describe('ariaHideOutside', function () {
340340

341341
let revert = ariaHideOutside([rows[1]]);
342342

343-
expect(rows[0]).not.toHaveAttribute('aria-hidden', 'true');
343+
// Applies aria-hidden to the row and cell despite recursive nature of aria-hidden
344+
// for https://bugs.webkit.org/show_bug.cgi?id=222623
345+
expect(rows[0]).toHaveAttribute('aria-hidden', 'true');
344346
expect(cells[0]).toHaveAttribute('aria-hidden', 'true');
345347
expect(rows[1]).not.toHaveAttribute('aria-hidden', 'true');
346348
expect(cells[1]).not.toHaveAttribute('aria-hidden', 'true');

packages/@react-spectrum/dnd/src/useDropHooks.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
DroppableCollectionResult,
66
DroppableItemOptions,
77
DroppableItemResult,
8+
isVirtualDragging,
89
useDropIndicator,
910
useDroppableCollection,
1011
useDroppableItem
@@ -17,7 +18,8 @@ export interface DropHooks {
1718
useDroppableCollectionState(props: DroppableCollectionStateOptions): DroppableCollectionState,
1819
useDroppableCollection(props: DroppableCollectionOptions, state: DroppableCollectionState, ref: RefObject<HTMLElement>): DroppableCollectionResult,
1920
useDroppableItem(options: DroppableItemOptions, state: DroppableCollectionState, ref: RefObject<HTMLElement>): DroppableItemResult,
20-
useDropIndicator(props: DropIndicatorProps, state: DroppableCollectionState, ref: RefObject<HTMLElement>): DropIndicatorAria
21+
useDropIndicator(props: DropIndicatorProps, state: DroppableCollectionState, ref: RefObject<HTMLElement>): DropIndicatorAria,
22+
isVirtualDragging(): boolean
2123
}
2224

2325
export function useDropHooks(options: DroppableCollectionProps): DropHooks {
@@ -29,6 +31,7 @@ export function useDropHooks(options: DroppableCollectionProps): DropHooks {
2931
useDroppableCollection(props, state, ref) {
3032
return useDroppableCollection({...props, ...options}, state, ref);
3133
},
32-
useDropIndicator
34+
useDropIndicator,
35+
isVirtualDragging
3336
}), [options]);
3437
}

packages/@react-spectrum/list/src/ListViewItem.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ export function ListViewItem<T>(props: ListViewItemProps<T>) {
8686
let target = {type: 'item', key: item.key, dropPosition: 'on'} as DropTarget;
8787
isDropTarget = dropState.isDropTarget(target);
8888
// eslint-disable-next-line react-hooks/rules-of-hooks
89-
droppableItem = dropHooks.useDroppableItem({target}, dropState, rowRef);
9089
dropIndicator = dropHooks.useDropIndicator({target}, dropState, dropIndicatorRef);
9190
}
9291

@@ -136,7 +135,10 @@ export function ListViewItem<T>(props: ListViewItemProps<T>) {
136135
dropProps,
137136
hoverProps,
138137
focusWithinProps,
139-
focusProps
138+
focusProps,
139+
// Remove tab index from list row if performing a screenreader drag. This prevents TalkBack from focusing the row,
140+
// allowing for single swipe navigation between row drop indicator
141+
dropHooks?.isVirtualDragging() && {tabIndex: null}
140142
);
141143

142144
let isFirstRow = item.prevKey == null;
@@ -226,7 +228,7 @@ export function ListViewItem<T>(props: ListViewItemProps<T>) {
226228
}
227229
</div>
228230
}
229-
{isDropTarget && !dropIndicator?.dropIndicatorProps['aria-hidden'] &&
231+
{isListDroppable && !dropIndicator?.isHidden &&
230232
<div role="button" {...visuallyHiddenProps} {...dropIndicator?.dropIndicatorProps} ref={dropIndicatorRef} />
231233
}
232234
<CSSTransition

packages/@react-spectrum/list/test/ListViewDnd.test.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,9 +1350,19 @@ describe('ListView', function () {
13501350
expect(row).toHaveAttribute('draggable', 'true');
13511351
fireEvent.keyDown(draghandle, {key: 'Enter'});
13521352
fireEvent.keyUp(draghandle, {key: 'Enter'});
1353+
act(() => jest.runAllTimers());
13531354

1354-
for (let row of rows) {
1355-
expect(row).toHaveAttribute('aria-hidden', 'true');
1355+
for (let [index, row] of rows.entries()) {
1356+
let cell = within(row).getByRole('gridcell', {hidden: true});
1357+
// We hide the cell for iOS Safari and remove the row tab index so TalkBack doesn't focus the row
1358+
expect(row).not.toHaveAttribute('tabindex');
1359+
if (index === 0) {
1360+
expect(row).not.toHaveAttribute('aria-hidden', 'true');
1361+
expect(cell).not.toHaveAttribute('aria-hidden', 'true');
1362+
} else {
1363+
expect(row).toHaveAttribute('aria-hidden', 'true');
1364+
expect(cell).toHaveAttribute('aria-hidden', 'true');
1365+
}
13561366
}
13571367

13581368
fireEvent.keyDown(document.body, {key: 'Escape'});

0 commit comments

Comments
 (0)