refactor: No longer preventDefault in usePress and allow browser to manage focus #7542

devongovett · 2024-12-20T23:38:29Z

Part of #1720

Fixes #5004, fixes #4302, fixes #6618, closes #7448, fixes #5643, fixes #7480, fixes #5522, fixes #4089, fixes #5833, fixes #6512, fixes #4355, fixes #5940, fixes #1926, fixes #1384, closes #1391

This is a major refactor to usePress to improve compatibility with other libraries, improve the way focus is managed, and fix many issues caused by our current approach. 🎄

This is mainly possible because of a change in Safari 17, which enabled buttons and other native inputs to be focused (both via mouse/touch and keyboard tabbing) only when they have an explicit tabIndex set. WebKit/WebKit#12743 We previously worked around this by calling preventDefault and manually managing focus. However, because preventDefault is not granular this had many unintended side effects.

preventDefault during onPointerDown prevents subsequent mouse and touch events from being fired by the browser entirely. This can break compatibility between React Aria and other libraries that rely on these events (e.g. Material UI button ripple effect).
- [RAC] - Table block onMouseDown event #5004
preventDefault during onMouseDown prevents the :active CSS pseudo class in Firefox
- Can't use :active selector with useButton #4302
preventDefault during onMouseDown prevents drag and drop. We had some special cases for this but not all of them.
- Unable to drag and drop links #6618
- fix: make a tag link draggable with usePress #7448
Libraries overload preventDefault to also prevent their behaviors in addition to the browser ones.
- React aria components are incompatible with Radix UI? #5643

Thanks to the Safari change, we can now let the browser handle focusing elements on press for us. This changes the order that things occur, so we'll need to test carefully. In particular, the element will no longer be focused before onPressStart because onFocus is fired by the browser after onMouseDown. On touch devices, this will happen after the user lifts their finger. This change should fix these issues:

Focusing during onPointerDown could cause the software keyboard to disappear, resulting in a layout shift that caused the user to tap on the wrong element.
- Button onPress event on iOS can select the wrong element when virtual keyboard is open #6512
- useButton: focus removes from previous element if touch starts #4355
Triggering an implicit form submission caused focus to move to the submit button
- Pressing enter while one of the form inputs is focused (i.e. submitting a form) moves focus to the submit button. #5940
useRadio and useCheckbox cause ui jumps on Safari through usePress and focusWithoutScrolling #1926
Programmatic focus causes the :focus-visible selector to apply
- useFocusable triggers keyboard interaction use case when only using mouse #4457
- React Aria useButton mouse click triggers :focus-visible styles #1247

When preventFocusOnPress is set, we allow the browser to focus, but handle the event at a window-level capturing listener, and immediately revert it and stop propagation to make the event non-observable to child elements.

We also now delay firing onPress until the onClick event instead of firing it during onPointerUp. This fixes many issues:

On Android, if you remove the original target between onPointerUp and onClick, the element behind it will be clicked instead.
- HTML element under the drawer is clicked when the button on the drawer is clicked #7480
- ComboBox triggers click on background elements on mobile #5522
Libraries and users rely on onClick being fired for a variety of purposes:
- Event handler captures incorrect event target for MenuTrigger button #4089
- stopPropagation not preventing event propagation to parent element using usePress #5833

We will need to evaluate how much of a breaking change this is. Outside usePress itself, the main changes were for tests, which now must actually trigger the click event and not just pointerdown/pointerup. Switching to user-event fixes this for the most part. We also have to deal with some assumptions that focus will occur before onPressStart, such as in useMenuTrigger which now uses preventFocusOnPress and manually focuses its trigger prior to opening the popover so that it can be restored properly. This only happens for mouse interactions.

…them

#5940

For #5833

rspbot · 2024-12-20T23:47:17Z

Build successful! 🎉

devongovett · 2024-12-21T00:34:17Z

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

-export function isFocusable(element: HTMLElement) {
-  return element.matches(FOCUSABLE_ELEMENT_SELECTOR);
-}
-


Moved to @react-aria/utils to avoid creating a circular dependency when I import it in @react-aria/interactions.

devongovett · 2024-12-21T00:35:38Z

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

          // If triggered from a screen reader or by using element.click(),
          // trigger as if it were a keyboard click.
-          if (!state.ignoreClickAfterPress && !state.ignoreEmulatedMouseEvents && !state.isPressed && (state.pointerType === 'virtual' || isVirtualClick(e.nativeEvent))) {
-            // Ensure the element receives focus (VoiceOver on iOS does not do this)


I can't reproduce the original issue anymore. iOS VO uses DOM focus now. Not sure if we need to worry about this elsewhere. If so, it should probably be done e.g. in dialog trigger instead of here so it doesn't affect other things like submitting a form.

devongovett · 2024-12-21T00:36:28Z

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

@@ -544,18 +536,15 @@ export function usePress(props: PressHookProps): PressResult {
        cancel(e);
      };
    } else {
+      // NOTE: this fallback branch is almost entirely used by unit tests.
+      // All browsers now support pointer events, but JSDOM still does not.


Next step will be to make this branch development only, and possibly get rid of the touch event handling entirely (since most people write their tests using mouse events).

devongovett · 2024-12-21T00:39:09Z

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

        checkSelection(onSelectionChange, ['Foo 10']);
-        // screen reader automatically handles this one
-        expect(announce).not.toHaveBeenCalled();
+        expect(announce).toHaveBeenCalledWith('Foo 10 selected.');


This test was wrong before. The announcement was not fired because the table never had focus.

rspbot · 2024-12-21T05:53:38Z

Build successful! 🎉

rspbot · 2024-12-21T05:53:39Z

## API Changes

@react-aria/focus

/@react-aria/focus:isFocusable

 isFocusable {
-  element: HTMLElement
+  element: Element
   returnVal: undefined
 }

@react-aria/test-utils

/@react-aria/test-utils:triggerLongPress

 triggerLongPress {
   opts: {
     element: HTMLElement
   advanceTimer: (number) => void | Promise<unknown>
-  pointerOpts?: {
-  
+  pointerOpts?: Record<string, any>
 }
-}
   returnVal: undefined
 }

@react-aria/utils

/@react-aria/utils:isFocusable

+isFocusable {
+  element: Element
+  returnVal: undefined
+}

/@react-aria/utils:isTabbable

+isTabbable {
+  element: Element
+  returnVal: undefined
+}

@react-spectrum/test-utils

/@react-spectrum/test-utils:triggerLongPress

 triggerLongPress {
   opts: {
     element: HTMLElement
   advanceTimer: (number) => void | Promise<unknown>
-  pointerOpts?: {
-  
+  pointerOpts?: Record<string, any>
 }
-}
   returnVal: undefined
 }

devongovett added 11 commits December 19, 2024 12:15

working first pass

fc45e2f

Update mouse and touch handlers as well

f9d3339

Always set a tabIndex on native buttons and inputs so Safari focuses …

53d8ed2

…them

lint

2340dba

Update tests

f5041d2

Make grid selection announcement more resilient to focus order

f0fd5d6

Remove focus during virtual clicks

7034dc9

#5940

Add test story for #7480

a30e14a

Add story for #6512

dac4320

Make sure to cancel press if onClick's propagation is stopped

2aa0612

For #5833

fix ts

4a6f3df

devongovett commented Dec 21, 2024

View reviewed changes

devongovett mentioned this pull request Dec 21, 2024

Dismissing a dialog by clicking outside inconsistently triggers links outside #7283

Open

Use focusWithoutScrolling when restoring focus

f7e4d9e

snowystinger mentioned this pull request Dec 23, 2024

Picker: Clicking outside changes the picklist value #7537

Open

snowystinger mentioned this pull request Jan 1, 2025

onClickCapture does not work as expected in mobile #7557

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: No longer preventDefault in usePress and allow browser to manage focus #7542

refactor: No longer preventDefault in usePress and allow browser to manage focus #7542

devongovett commented Dec 20, 2024 •

edited

Loading

rspbot commented Dec 20, 2024

devongovett Dec 21, 2024

devongovett Dec 21, 2024

devongovett Dec 21, 2024

devongovett Dec 21, 2024

rspbot commented Dec 21, 2024

rspbot commented Dec 21, 2024

refactor: No longer preventDefault in usePress and allow browser to manage focus #7542

Are you sure you want to change the base?

refactor: No longer preventDefault in usePress and allow browser to manage focus #7542

Conversation

devongovett commented Dec 20, 2024 • edited Loading

rspbot commented Dec 20, 2024

devongovett Dec 21, 2024

Choose a reason for hiding this comment

devongovett Dec 21, 2024

Choose a reason for hiding this comment

devongovett Dec 21, 2024

Choose a reason for hiding this comment

devongovett Dec 21, 2024

Choose a reason for hiding this comment

rspbot commented Dec 21, 2024

rspbot commented Dec 21, 2024

@react-aria/focus

/@react-aria/focus:isFocusable

@react-aria/test-utils

/@react-aria/test-utils:triggerLongPress

@react-aria/utils

/@react-aria/utils:isFocusable

/@react-aria/utils:isTabbable

@react-spectrum/test-utils

/@react-spectrum/test-utils:triggerLongPress

devongovett commented Dec 20, 2024 •

edited

Loading