Skip to content

Commit cdba748

Browse files
devongovettdevongovett
authored
refactor: No longer preventDefault in usePress and allow browser to manage focus (#7542)
* working first pass * Update mouse and touch handlers as well * Always set a tabIndex on native buttons and inputs so Safari focuses them * lint * Update tests * Make grid selection announcement more resilient to focus order * Remove focus during virtual clicks #5940 * Add test story for #7480 * Add story for #6512 * Make sure to cancel press if onClick's propagation is stopped For #5833 * fix ts * Use focusWithoutScrolling when restoring focus
1 parent df3f1ea commit cdba748

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+886
-516
lines changed

‎packages/@react-aria/breadcrumbs/test/useBreadcrumbItem.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('useBreadcrumbItem', function () {
4444

4545
it('handles descendant link with href', function () {
4646
let {itemProps} = renderLinkHook({children: <a href="https://example.com">Breadcrumb Item</a>});
47-
expect(itemProps.tabIndex).toBeUndefined();
47+
expect(itemProps.tabIndex).toBe(0);
4848
expect(itemProps.role).toBeUndefined();
4949
expect(itemProps['aria-disabled']).toBeUndefined();
5050
expect(typeof itemProps.onKeyDown).toBe('function');

‎packages/@react-aria/button/src/useButton.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ export function useButton(props: AriaButtonOptions<ElementType>, ref: RefObject<
7474
} else {
7575
additionalProps = {
7676
role: 'button',
77-
tabIndex: isDisabled ? undefined : 0,
7877
href: elementType === 'a' && !isDisabled ? href : undefined,
7978
target: elementType === 'a' ? target : undefined,
8079
type: elementType === 'input' ? type : undefined,

‎packages/@react-aria/dnd/test/dnd.test.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2466,7 +2466,7 @@ describe('useDrag and useDrop', function () {
24662466

24672467
let draggable = tree.getByText('Drag me');
24682468

2469-
fireEvent.focus(draggable);
2469+
act(() => draggable.focus());
24702470
fireEvent(draggable, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
24712471
fireEvent(draggable, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
24722472
await user.click(draggable);
@@ -2496,7 +2496,7 @@ describe('useDrag and useDrop', function () {
24962496
let draggable = tree.getByText('Drag me');
24972497
let droppable = tree.getByText('Drop here');
24982498

2499-
fireEvent.focus(draggable);
2499+
act(() => draggable.focus());
25002500
fireEvent(draggable, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
25012501
fireEvent(draggable, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
25022502
await user.click(draggable);
@@ -2505,7 +2505,7 @@ describe('useDrag and useDrop', function () {
25052505

25062506

25072507
// Android Talkback fires with click event of detail = 1, test that our onPointerDown listener detects that it is a virtual click
2508-
fireEvent.focus(droppable);
2508+
act(() => droppable.focus());
25092509
fireEvent(droppable, pointerEvent('pointerdown', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0, pointerType: 'mouse'}));
25102510
fireEvent(droppable, pointerEvent('pointerup', {pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0, pointerType: 'mouse'}));
25112511
fireEvent.click(droppable, {detail: 1});
@@ -2535,7 +2535,7 @@ describe('useDrag and useDrop', function () {
25352535

25362536
let draggable = tree.getByText('Drag me');
25372537

2538-
fireEvent.focus(draggable);
2538+
act(() => draggable.focus());
25392539
await user.click(draggable);
25402540
act(() => jest.runAllTimers());
25412541

@@ -2563,7 +2563,7 @@ describe('useDrag and useDrop', function () {
25632563

25642564
expect(tree.getAllByRole('textbox')).toHaveLength(1);
25652565

2566-
fireEvent.focus(draggable);
2566+
act(() => draggable.focus());
25672567
fireEvent.click(draggable);
25682568
act(() => jest.runAllTimers());
25692569

@@ -2596,7 +2596,7 @@ describe('useDrag and useDrop', function () {
25962596

25972597
let draggable = tree.getByText('Drag me');
25982598

2599-
fireEvent.focus(draggable);
2599+
act(() => draggable.focus());
26002600
await user.click(draggable);
26012601
act(() => jest.runAllTimers());
26022602

@@ -2621,7 +2621,7 @@ describe('useDrag and useDrop', function () {
26212621

26222622
let draggable = tree.getByText('Drag me');
26232623

2624-
fireEvent.focus(draggable);
2624+
act(() => draggable.focus());
26252625
await user.click(draggable);
26262626
act(() => jest.runAllTimers());
26272627

@@ -2644,7 +2644,7 @@ describe('useDrag and useDrop', function () {
26442644
let droppable = tree.getByText('Drop here');
26452645
let input = tree.getByRole('textbox');
26462646

2647-
fireEvent.focus(draggable);
2647+
act(() => draggable.focus());
26482648
await user.click(draggable);
26492649
act(() => jest.runAllTimers());
26502650

@@ -2664,7 +2664,7 @@ describe('useDrag and useDrop', function () {
26642664
let draggable = tree.getByText('Drag me');
26652665
let input = tree.getByRole('textbox');
26662666

2667-
fireEvent.focus(draggable);
2667+
act(() => draggable.focus());
26682668
await user.click(draggable);
26692669
act(() => jest.runAllTimers());
26702670

@@ -2681,7 +2681,7 @@ describe('useDrag and useDrop', function () {
26812681
let draggable = tree.getByText('Drag me');
26822682
let droppable = tree.getByText('Drop here');
26832683

2684-
fireEvent.focus(draggable);
2684+
act(() => draggable.focus());
26852685
await user.click(draggable);
26862686
act(() => jest.runAllTimers());
26872687

@@ -2715,7 +2715,7 @@ describe('useDrag and useDrop', function () {
27152715

27162716
let draggable = tree.getByText('Drag me');
27172717

2718-
fireEvent.focus(draggable);
2718+
act(() => draggable.focus());
27192719
fireEvent.click(draggable, {detail: 1});
27202720
act(() => jest.runAllTimers());
27212721

@@ -2731,7 +2731,7 @@ describe('useDrag and useDrop', function () {
27312731
let draggable = tree.getByText('Drag me');
27322732
let droppable = tree.getByText('Drop here');
27332733

2734-
fireEvent.focus(draggable);
2734+
act(() => draggable.focus());
27352735
await user.click(draggable);
27362736
act(() => jest.runAllTimers());
27372737
expect(draggable).toHaveAttribute('data-dragging', 'true');

‎packages/@react-aria/dnd/test/useDraggableCollection.test.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,8 @@ describe('useDraggableCollection', () => {
781781

782782
let dragButton = within(cells[1]).getByRole('button');
783783
expect(dragButton).toHaveAttribute('aria-label', 'Drag Bar');
784-
fireEvent.focus(dragButton);
784+
act(() => cells[1].focus());
785+
act(() => dragButton.focus());
785786
fireEvent.click(dragButton);
786787
act(() => jest.runAllTimers());
787788
expect(cells[0]).not.toHaveClass('is-dragging');
@@ -852,15 +853,15 @@ describe('useDraggableCollection', () => {
852853
let cells = within(grid).getAllByRole('gridcell');
853854
expect(cells).toHaveLength(3);
854855

855-
fireEvent.focus(cells[0]);
856+
act(() => cells[0].focus());
856857
fireEvent.click(cells[0]);
857858
expect(rows[0]).toHaveAttribute('aria-selected', 'true');
858859
fireEvent.click(cells[1]);
859860
expect(rows[1]).toHaveAttribute('aria-selected', 'true');
860861

861862
let dragButton = within(cells[1]).getByRole('button');
862863
expect(dragButton).toHaveAttribute('aria-label', 'Drag 2 selected items');
863-
fireEvent.focus(dragButton);
864+
act(() => dragButton.focus());
864865
fireEvent.click(dragButton);
865866
act(() => jest.runAllTimers());
866867
expect(cells[0]).toHaveClass('is-dragging');
@@ -939,13 +940,13 @@ describe('useDraggableCollection', () => {
939940
let cells = within(grid).getAllByRole('gridcell');
940941
expect(cells).toHaveLength(3);
941942

942-
fireEvent.focus(cells[0]);
943+
act(() => cells[0].focus());
943944
fireEvent.click(cells[0]);
944945
expect(rows[0]).toHaveAttribute('aria-selected', 'true');
945946

946947
let dragButton = within(cells[1]).getByRole('button');
947948
expect(dragButton).toHaveAttribute('aria-label', 'Drag Bar');
948-
fireEvent.focus(dragButton);
949+
act(() => dragButton.focus());
949950
fireEvent.click(dragButton);
950951
act(() => jest.runAllTimers());
951952
expect(cells[0]).not.toHaveClass('is-dragging');

‎packages/@react-aria/focus/src/FocusScope.tsx‎

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import {FocusableElement, RefObject} from '@react-types/shared';
1414
import {focusSafely} from './focusSafely';
1515
import {getInteractionModality} from '@react-aria/interactions';
16-
import {getOwnerDocument, isAndroid, isChrome, useLayoutEffect} from '@react-aria/utils';
16+
import {getOwnerDocument, isAndroid, isChrome, isFocusable, isTabbable, useLayoutEffect} from '@react-aria/utils';
1717
import {isElementVisible} from './isElementVisible';
1818
import React, {ReactNode, useContext, useEffect, useMemo, useRef} from 'react';
1919

@@ -268,31 +268,6 @@ function createFocusManagerForScope(scopeRef: React.RefObject<Element[] | null>)
268268
};
269269
}
270270

271-
const focusableElements = [
272-
'input:not([disabled]):not([type=hidden])',
273-
'select:not([disabled])',
274-
'textarea:not([disabled])',
275-
'button:not([disabled])',
276-
'a[href]',
277-
'area[href]',
278-
'summary',
279-
'iframe',
280-
'object',
281-
'embed',
282-
'audio[controls]',
283-
'video[controls]',
284-
'[contenteditable]:not([contenteditable^="false"])'
285-
];
286-
287-
const FOCUSABLE_ELEMENT_SELECTOR = focusableElements.join(':not([hidden]),') + ',[tabindex]:not([disabled]):not([hidden])';
288-
289-
focusableElements.push('[tabindex]:not([tabindex="-1"]):not([disabled])');
290-
const TABBABLE_ELEMENT_SELECTOR = focusableElements.join(':not([hidden]):not([tabindex="-1"]),');
291-
292-
export function isFocusable(element: HTMLElement) {
293-
return element.matches(FOCUSABLE_ELEMENT_SELECTOR);
294-
}
295-
296271
function getScopeRoot(scope: Element[]) {
297272
return scope[0].parentElement!;
298273
}
@@ -749,7 +724,7 @@ function restoreFocusToElement(node: FocusableElement) {
749724
* that matches all focusable/tabbable elements.
750725
*/
751726
export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions, scope?: Element[]) {
752-
let selector = opts?.tabbable ? TABBABLE_ELEMENT_SELECTOR : FOCUSABLE_ELEMENT_SELECTOR;
727+
let filter = opts?.tabbable ? isTabbable : isFocusable;
753728
let walker = getOwnerDocument(root).createTreeWalker(
754729
root,
755730
NodeFilter.SHOW_ELEMENT,
@@ -760,7 +735,7 @@ export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions
760735
return NodeFilter.FILTER_REJECT;
761736
}
762737

763-
if ((node as Element).matches(selector)
738+
if (filter(node as Element)
764739
&& isElementVisible(node as Element)
765740
&& (!scope || isElementInScope(node as Element, scope))
766741
&& (!opts?.accept || opts.accept(node as Element))

‎packages/@react-aria/focus/src/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
export {FocusScope, useFocusManager, getFocusableTreeWalker, createFocusManager, isElementInChildOfActiveScope, isFocusable} from './FocusScope';
13+
export {FocusScope, useFocusManager, getFocusableTreeWalker, createFocusManager, isElementInChildOfActiveScope} from './FocusScope';
1414
export {FocusRing} from './FocusRing';
1515
export {FocusableProvider, useFocusable} from './useFocusable';
1616
export {useFocusRing} from './useFocusRing';
1717
export {focusSafely} from './focusSafely';
1818
export {useHasTabbableChild} from './useHasTabbableChild';
19+
// For backward compatibility.
20+
export {isFocusable} from '@react-aria/utils';
1921

2022
export type {FocusScopeProps, FocusManager, FocusManagerOptions} from './FocusScope';
2123
export type {FocusRingProps} from './FocusRing';

‎packages/@react-aria/focus/src/useFocusable.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,17 @@ export function useFocusable<T extends FocusableElement = FocusableElement>(prop
8282
autoFocusRef.current = false;
8383
}, [domRef]);
8484

85+
// Always set a tabIndex so that Safari allows focusing native buttons and inputs.
86+
let tabIndex: number | undefined = props.excludeFromTabOrder ? -1 : 0;
87+
if (props.isDisabled) {
88+
tabIndex = undefined;
89+
}
90+
8591
return {
8692
focusableProps: mergeProps(
8793
{
8894
...interactions,
89-
tabIndex: props.excludeFromTabOrder && !props.isDisabled ? -1 : undefined
95+
tabIndex
9096
},
9197
interactionProps
9298
)

‎packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ import {Collection, Key, Node, Selection} from '@react-types/shared';
1515
// @ts-ignore
1616
import intlMessages from '../intl/*.json';
1717
import {SelectionManager} from '@react-stately/selection';
18+
import {useEffectEvent, useUpdateEffect} from '@react-aria/utils';
1819
import {useLocalizedStringFormatter} from '@react-aria/i18n';
1920
import {useRef} from 'react';
20-
import {useUpdateEffect} from '@react-aria/utils';
2121

2222
export interface GridSelectionAnnouncementProps {
2323
/**
@@ -46,8 +46,8 @@ export function useGridSelectionAnnouncement<T>(props: GridSelectionAnnouncement
4646
// We do this using an ARIA live region.
4747
let selection = state.selectionManager.rawSelection;
4848
let lastSelection = useRef(selection);
49-
useUpdateEffect(() => {
50-
if (!state.selectionManager.isFocused) {
49+
let announceSelectionChange = useEffectEvent(() => {
50+
if (!state.selectionManager.isFocused || selection === lastSelection.current) {
5151
lastSelection.current = selection;
5252

5353
return;
@@ -96,7 +96,17 @@ export function useGridSelectionAnnouncement<T>(props: GridSelectionAnnouncement
9696
}
9797

9898
lastSelection.current = selection;
99-
}, [selection]);
99+
});
100+
101+
useUpdateEffect(() => {
102+
if (state.selectionManager.isFocused) {
103+
announceSelectionChange();
104+
} else {
105+
// Wait a frame in case the collection is about to become focused (e.g. on mouse down).
106+
let raf = requestAnimationFrame(announceSelectionChange);
107+
return () => cancelAnimationFrame(raf);
108+
}
109+
}, [selection, state.selectionManager.isFocused]);
100110
}
101111

102112
function diffSelection(a: Selection, b: Selection): Set<Key> {

‎packages/@react-aria/interactions/src/useFocusVisible.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// See https://github.com/facebook/react/tree/cc7c1aece46a6b69b41958d731e0fd27c94bfc6c/packages/react-interactions
1717

1818
import {getOwnerDocument, getOwnerWindow, isMac, isVirtualClick} from '@react-aria/utils';
19+
import {ignoreFocusEvent} from './utils';
1920
import {useEffect, useState} from 'react';
2021
import {useIsSSR} from '@react-aria/ssr';
2122

@@ -92,7 +93,7 @@ function handleFocusEvent(e: FocusEvent) {
9293
// Firefox fires two extra focus events when the user first clicks into an iframe:
9394
// first on the window, then on the document. We ignore these events so they don't
9495
// cause keyboard focus rings to appear.
95-
if (e.target === window || e.target === document) {
96+
if (e.target === window || e.target === document || ignoreFocusEvent) {
9697
return;
9798
}
9899

@@ -108,6 +109,10 @@ function handleFocusEvent(e: FocusEvent) {
108109
}
109110

110111
function handleWindowBlur() {
112+
if (ignoreFocusEvent) {
113+
return;
114+
}
115+
111116
// When the window is blurred, reset state. This is necessary when tabbing out of the window,
112117
// for example, since a subsequent focus event won't be fired.
113118
hasEventBeforeFocus = false;

‎packages/@react-aria/interactions/src/useLongPress.ts‎

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

13-
import {DOMAttributes, LongPressEvent} from '@react-types/shared';
14-
import {mergeProps, useDescription, useGlobalListeners} from '@react-aria/utils';
13+
import {DOMAttributes, FocusableElement, LongPressEvent} from '@react-types/shared';
14+
import {focusWithoutScrolling, getOwnerDocument, mergeProps, useDescription, useGlobalListeners} from '@react-aria/utils';
1515
import {usePress} from './usePress';
1616
import {useRef} from 'react';
1717

@@ -81,6 +81,12 @@ export function useLongPress(props: LongPressProps): LongPressResult {
8181
timeRef.current = setTimeout(() => {
8282
// Prevent other usePress handlers from also handling this event.
8383
e.target.dispatchEvent(new PointerEvent('pointercancel', {bubbles: true}));
84+
85+
// Ensure target is focused. On touch devices, browsers typically focus on pointer up.
86+
if (getOwnerDocument(e.target).activeElement !== e.target) {
87+
focusWithoutScrolling(e.target as FocusableElement);
88+
}
89+
8490
if (onLongPress) {
8591
onLongPress({
8692
...e,

0 commit comments

Comments
 (0)