refactor: No longer preventDefault in usePress and allow browser to manage focus #7542
Conversation
| export function isFocusable(element: HTMLElement) { | ||
| return element.matches(FOCUSABLE_ELEMENT_SELECTOR); | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved to @react-aria/utils to avoid creating a circular dependency when I import it in @react-aria/interactions.
| // 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) |
There was a problem hiding this comment.
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.
| @@ -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. | |||
There was a problem hiding this comment.
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).
| checkSelection(onSelectionChange, ['Foo 10']); | ||
| // screen reader automatically handles this one | ||
| expect(announce).not.toHaveBeenCalled(); | ||
| expect(announce).toHaveBeenCalledWith('Foo 10 selected.'); |
There was a problem hiding this comment.
This test was wrong before. The announcement was not fired because the table never had focus.
There was a problem hiding this comment.
Tested the various cases mentioned in various comments in usePress and the other hooks in Android Chrome, iPad Safari and desktop Chrome/Firefox/Safari. Noted some small differences in behavior but otherwise things like DnD/long press/press/etc seem to work well. 🤞
| useUpdateEffect(() => { | ||
| if (state.selectionManager.isFocused) { | ||
| announceSelectionChange(); | ||
| } else { | ||
| // Wait a frame in case the collection is about to become focused (e.g. on mouse down). | ||
| let raf = requestAnimationFrame(announceSelectionChange); | ||
| return () => cancelAnimationFrame(raf); | ||
| } | ||
| }, [selection, state.selectionManager.isFocused]); |
There was a problem hiding this comment.
Not necessarily related to this change (but spurred due to the comment of mouse down here) but now that the Table's cell is focused on click rather than the row, the announcement of row selection seems to be interrupted a lot more, at least on Safari 17 + VoiceOver.
| if (getOwnerDocument(e.target).activeElement !== e.target) { | ||
| focusWithoutScrolling(e.target as FocusableElement); | ||
| } |
There was a problem hiding this comment.
Perhaps not a blocker, but just noticed that long pressing on "Two" in "TableView: static" story w/ highlight selection and multiple selection via Chrome Android causes a focus ring to appear on the "One" cell instead of "Two". Doesn't seem to happen on my iPad though
| // 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) |
| // Chrome and Firefox on touch Windows devices require mouse down events | ||
| // to be canceled in addition to pointer events, or an extra asynchronous | ||
| // focus event will be fired. | ||
| if (shouldPreventDefaultDown(e.currentTarget as Element)) { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
note: was unable to confirm this case still, will need someone with a touch Windows device
…refactor # Conflicts: # packages/@react-aria/focus/src/FocusScope.tsx # packages/@react-aria/test-utils/src/table.ts
## 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
} |
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
usePressto 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
tabIndexset. WebKit/WebKit#12743 We previously worked around this by callingpreventDefaultand manually managing focus. However, becausepreventDefaultis not granular this had many unintended side effects.preventDefaultduringonPointerDownprevents 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).onMouseDownevent #5004preventDefaultduringonMouseDownprevents the:activeCSS pseudo class in Firefox:activeselector withuseButton#4302preventDefaultduringonMouseDownprevents drag and drop. We had some special cases for this but not all of them.atag link draggable with usePress #7448preventDefaultto also prevent their behaviors in addition to the browser ones.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
onPressStartbecauseonFocusis fired by the browser afteronMouseDown. On touch devices, this will happen after the user lifts their finger. This change should fix these issues:onPointerDowncould cause the software keyboard to disappear, resulting in a layout shift that caused the user to tap on the wrong element.:focus-visibleselector to applyWhen
preventFocusOnPressis 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
onPressuntil theonClickevent instead of firing it duringonPointerUp. This fixes many issues:onPointerUpandonClick, the element behind it will be clicked instead.onClickbeing fired for a variety of purposes:We will need to evaluate how much of a breaking change this is. Outside
usePressitself, 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 beforeonPressStart, such as inuseMenuTriggerwhich now usespreventFocusOnPressand manually focuses its trigger prior to opening the popover so that it can be restored properly. This only happens for mouse interactions.