Skip to content

Commit 499f54e

Browse files
LFDanLudevongovett
andauthored
Fix Android TalkBack double tap behavior with TableView/ListView highlight selection (#2520)
* updating isVirtualPointerEvent to better detect TalkBack double tap fixed TableView/ListView highlight selection behavior where double tapping on the row would replace selection instead of toggling selection. usePress thought that TalkBack double tap was a mouse event rather than a virtual event causing SelectionManager to do a replace instead of a toggle * adding test * adding more tests for ListView and TableView highlight selection * fixing iOS long press and distiguishing talkback double tap from safari click * adding bug link for Safari pointer pressure * use state.pointerType if available in onClick virtual click detection * update android talkback virtual click check * fixing tests * forgot to save fix to usePress test smh Co-authored-by: Devon Govett <[email protected]>
1 parent e81cfec commit 499f54e

File tree

4 files changed

+190
-2
lines changed

4 files changed

+190
-2
lines changed

packages/@react-aria/interactions/src/usePress.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ export function usePress(props: PressHookProps): PressResult {
264264

265265
// If triggered from a screen reader or by using element.click(),
266266
// trigger as if it were a keyboard click.
267-
if (!state.ignoreClickAfterPress && !state.ignoreEmulatedMouseEvents && isVirtualClick(e.nativeEvent)) {
267+
if (!state.ignoreClickAfterPress && !state.ignoreEmulatedMouseEvents && (state.pointerType === 'virtual' || isVirtualClick(e.nativeEvent))) {
268268
// Ensure the element receives focus (VoiceOver on iOS does not do this)
269269
if (!isDisabled && !preventFocusOnPress) {
270270
focusWithoutScrolling(e.currentTarget);
@@ -767,5 +767,16 @@ function shouldPreventDefault(target: Element) {
767767

768768
function isVirtualPointerEvent(event: PointerEvent) {
769769
// If the pointer size is zero, then we assume it's from a screen reader.
770-
return event.width === 0 && event.height === 0;
770+
// Android TalkBack double tap will sometimes return a event with width and height of 1
771+
// and pointerType === 'mouse' so we need to check for a specific combination of event attributes.
772+
// Cannot use "event.pressure === 0" as the sole check due to Safari pointer events always returning pressure === 0
773+
// instead of .5, see https://bugs.webkit.org/show_bug.cgi?id=206216
774+
return (
775+
(event.width === 0 && event.height === 0) ||
776+
(event.width === 1 &&
777+
event.height === 1 &&
778+
event.pressure === 0 &&
779+
event.detail === 0
780+
)
781+
);
771782
}

packages/@react-aria/interactions/test/usePress.test.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,66 @@ describe('usePress', function () {
585585
]);
586586
});
587587

588+
it('should detect Android TalkBack double tap', function () {
589+
let events = [];
590+
let addEvent = (e) => events.push(e);
591+
let res = render(
592+
<Example
593+
onPressStart={addEvent}
594+
onPressEnd={addEvent}
595+
onPress={addEvent}
596+
onPressUp={addEvent} />
597+
);
598+
599+
let el = res.getByText('test');
600+
// Android TalkBack will occasionally fire a pointer down event with "width: 1, height: 1" instead of "width: 0, height: 0".
601+
// Make sure we can still determine that this is a virtual event by checking the pressure, detail, and height/width.
602+
fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
603+
fireEvent(el, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
604+
expect(events).toEqual([]);
605+
606+
// Virtual pointer event sets pointerType and onClick handles the rest
607+
fireEvent.click(el, {pointerType: 'mouse', width: 1, height: 1, detail: 1});
608+
expect(events).toEqual([
609+
{
610+
type: 'pressstart',
611+
target: el,
612+
pointerType: 'virtual',
613+
ctrlKey: false,
614+
metaKey: false,
615+
shiftKey: false,
616+
altKey: false
617+
},
618+
{
619+
type: 'pressup',
620+
target: el,
621+
pointerType: 'virtual',
622+
ctrlKey: false,
623+
metaKey: false,
624+
shiftKey: false,
625+
altKey: false
626+
},
627+
{
628+
type: 'pressend',
629+
target: el,
630+
pointerType: 'virtual',
631+
ctrlKey: false,
632+
metaKey: false,
633+
shiftKey: false,
634+
altKey: false
635+
},
636+
{
637+
type: 'press',
638+
target: el,
639+
pointerType: 'virtual',
640+
ctrlKey: false,
641+
metaKey: false,
642+
shiftKey: false,
643+
altKey: false
644+
}
645+
]);
646+
});
647+
588648
it('should not fire press events for disabled elements', function () {
589649
let events = [];
590650
let addEvent = (e) => events.push(e);

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,27 @@
1111
*/
1212
import {act, fireEvent, render as renderComponent, within} from '@testing-library/react';
1313
import {ActionButton} from '@react-spectrum/button';
14+
import {installPointerEvent} from '@react-spectrum/test-utils';
1415
import {Item, ListView} from '../src';
1516
import {Provider} from '@react-spectrum/provider';
1617
import React from 'react';
1718
import {theme} from '@react-spectrum/theme-default';
1819
import userEvent from '@testing-library/user-event';
1920

21+
function pointerEvent(type, opts) {
22+
let evt = new Event(type, {bubbles: true, cancelable: true});
23+
Object.assign(evt, {
24+
ctrlKey: false,
25+
metaKey: false,
26+
shiftKey: false,
27+
altKey: false,
28+
button: opts.button || 0,
29+
width: 1,
30+
height: 1
31+
}, opts);
32+
return evt;
33+
}
34+
2035
describe('ListView', function () {
2136
let offsetWidth, offsetHeight;
2237

@@ -276,6 +291,7 @@ describe('ListView', function () {
276291
});
277292

278293
describe('selection', function () {
294+
installPointerEvent();
279295
let checkSelection = (onSelectionChange, selectedKeys) => {
280296
expect(onSelectionChange).toHaveBeenCalledTimes(1);
281297
expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(selectedKeys));
@@ -370,6 +386,58 @@ describe('ListView', function () {
370386
expect(rows[1]).toHaveAttribute('aria-selected', 'true');
371387
expect(rows[2]).toHaveAttribute('aria-selected', 'true');
372388
});
389+
390+
it('should support single tap to perform row selection with screen reader if onAction isn\'t provided', function () {
391+
let onSelectionChange = jest.fn();
392+
let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight'});
393+
394+
let rows = tree.getAllByRole('row');
395+
expect(rows[1]).toHaveAttribute('aria-selected', 'false');
396+
397+
act(() => userEvent.click(within(rows[1]).getByText('Bar'), {pointerType: 'touch', width: 0, height: 0}));
398+
checkSelection(onSelectionChange, [
399+
'bar'
400+
]);
401+
expect(rows[1]).toHaveAttribute('aria-selected', 'true');
402+
onSelectionChange.mockReset();
403+
404+
// Android TalkBack double tap test, pointer event sets pointerType and onClick handles the rest
405+
expect(rows[2]).toHaveAttribute('aria-selected', 'false');
406+
act(() => {
407+
let el = within(rows[2]).getByText('Baz');
408+
fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
409+
fireEvent(el, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
410+
fireEvent.click(el, {pointerType: 'mouse', width: 1, height: 1, detail: 1});
411+
});
412+
checkSelection(onSelectionChange, [
413+
'bar', 'baz'
414+
]);
415+
expect(rows[1]).toHaveAttribute('aria-selected', 'true');
416+
expect(rows[2]).toHaveAttribute('aria-selected', 'true');
417+
});
418+
419+
it('should support single tap to perform onAction with screen reader', function () {
420+
let onSelectionChange = jest.fn();
421+
let onAction = jest.fn();
422+
let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', selectionStyle: 'highlight', onAction});
423+
424+
let rows = tree.getAllByRole('row');
425+
act(() => userEvent.click(within(rows[1]).getByText('Bar'), {pointerType: 'touch', width: 0, height: 0}));
426+
expect(onSelectionChange).not.toHaveBeenCalled();
427+
expect(onAction).toHaveBeenCalledTimes(1);
428+
expect(onAction).toHaveBeenCalledWith('bar');
429+
430+
// Android TalkBack double tap test, pointer event sets pointerType and onClick handles the rest
431+
act(() => {
432+
let el = within(rows[2]).getByText('Baz');
433+
fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
434+
fireEvent(el, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
435+
fireEvent.click(el, {pointerType: 'mouse', width: 1, height: 1, detail: 1});
436+
});
437+
expect(onSelectionChange).not.toHaveBeenCalled();
438+
expect(onAction).toHaveBeenCalledTimes(2);
439+
expect(onAction).toHaveBeenCalledWith('baz');
440+
});
373441
});
374442
});
375443

packages/@react-spectrum/table/test/Table.test.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ function ExampleSortTable() {
8686
);
8787
}
8888

89+
function pointerEvent(type, opts) {
90+
let evt = new Event(type, {bubbles: true, cancelable: true});
91+
Object.assign(evt, {
92+
ctrlKey: false,
93+
metaKey: false,
94+
shiftKey: false,
95+
altKey: false,
96+
button: opts.button || 0,
97+
width: 1,
98+
height: 1
99+
}, opts);
100+
return evt;
101+
}
102+
89103
describe('TableView', function () {
90104
let offsetWidth, offsetHeight;
91105

@@ -2465,6 +2479,32 @@ describe('TableView', function () {
24652479
expect(onAction).toHaveBeenCalledWith('Foo 5');
24662480
});
24672481

2482+
it('should support single tap to perform row selection with screen reader if onAction isn\'t provided', function () {
2483+
let onSelectionChange = jest.fn();
2484+
let tree = renderTable({onSelectionChange, selectionStyle: 'highlight'});
2485+
2486+
userEvent.click(getCell(tree, 'Baz 5'), {pointerType: 'touch', width: 0, height: 0});
2487+
checkSelection(onSelectionChange, [
2488+
'Foo 5'
2489+
]);
2490+
onSelectionChange.mockReset();
2491+
2492+
userEvent.click(getCell(tree, 'Foo 8'), {pointerType: 'touch', width: 0, height: 0});
2493+
checkSelection(onSelectionChange, [
2494+
'Foo 5', 'Foo 8'
2495+
]);
2496+
onSelectionChange.mockReset();
2497+
2498+
// Android TalkBack double tap test, virtual pointer event sets pointerType and onClick handles the rest
2499+
let cell = getCell(tree, 'Foo 10');
2500+
fireEvent(cell, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
2501+
fireEvent(cell, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
2502+
fireEvent.click(cell, {pointerType: 'mouse', width: 1, height: 1, detail: 1});
2503+
checkSelection(onSelectionChange, [
2504+
'Foo 5', 'Foo 8', 'Foo 10'
2505+
]);
2506+
});
2507+
24682508
it('should support single tap to perform onAction with screen reader', function () {
24692509
let onSelectionChange = jest.fn();
24702510
let onAction = jest.fn();
@@ -2474,6 +2514,15 @@ describe('TableView', function () {
24742514
expect(onSelectionChange).not.toHaveBeenCalled();
24752515
expect(onAction).toHaveBeenCalledTimes(1);
24762516
expect(onAction).toHaveBeenCalledWith('Foo 5');
2517+
2518+
// Android TalkBack double tap test, virtual pointer event sets pointerType and onClick handles the rest
2519+
let cell = getCell(tree, 'Foo 10');
2520+
fireEvent(cell, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
2521+
fireEvent(cell, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
2522+
fireEvent.click(cell, {pointerType: 'mouse', width: 1, height: 1, detail: 1});
2523+
expect(onSelectionChange).not.toHaveBeenCalled();
2524+
expect(onAction).toHaveBeenCalledTimes(2);
2525+
expect(onAction).toHaveBeenCalledWith('Foo 10');
24772526
});
24782527

24792528
it('should support long press to enter selection mode on touch', function () {

0 commit comments

Comments
 (0)