[Select] Fix touch devices bugs #2939
Conversation
vladmoroz
requested review from
StephenHaney,
lucasmotta and
hadihallak
as code owners
|
|
||
| // Open on click when using a touch or pen device | ||
| if (pointerTypeRef.current !== 'mouse') { | ||
| handleOpen(event); | ||
| } |
There was a problem hiding this comment.
Open on click for non-mouse devices
For posterity, about pen devices in this PR:
- Using an Apple Pencil on iOS yields
pointerType: 'pen', but behaves exactly like touch (e.g. opens the Select on pointerdown when intending to scroll the page) - Using a Wacom tablets on macOS also yields
pointerType: 'pen', but behaves exactly like mouse (which don't need pointerdown to scroll, so the current behaviour isn't a problem in this case).
Since we can't disambiguate the two, I'm being a bit more defensive here and assuming pen is equivalent to touch
| // but not when the control key is pressed (avoiding MacOS right click); also not for touch | ||
| // devices because that would open the menu on scroll. (pen devices behave as touch on iOS). | ||
| if (event.button === 0 && event.ctrlKey === false && event.pointerType === 'mouse') { | ||
| handleOpen(event); |
There was a problem hiding this comment.
Retain handle open callback on pointerdown for mouse devices
| } else if (pointerTypeRef.current === 'mouse') { | ||
| // even though safari doesn't support this option, it's acceptable | ||
| // as it only means it might scroll a few pixels when using the pointer. | ||
| event.currentTarget.focus({ preventScroll: true }); |
There was a problem hiding this comment.
Check for a mouse pointer is added because on iOS gliding through the items doesn't quite work the same way—focus gets stuck on the first item that received focus on pointer move and won't budge
| onClick={composeEventHandlers(itemProps.onClick, () => { | ||
| // Open on click when using a touch or pen device | ||
| if (pointerTypeRef.current !== 'mouse') handleSelect(); | ||
| })} | ||
| onPointerUp={composeEventHandlers(itemProps.onPointerUp, () => { | ||
| // Using a mouse you should be able to do pointer down, move through | ||
| // the list, and release the pointer over the item to select it. | ||
| if (pointerTypeRef.current === 'mouse') handleSelect(); | ||
| })} |
There was a problem hiding this comment.
In case of #1658, what appears to happen when you click an item on a touch device is that touch and pointer events are fired on the Select Item correctly, but then also a click event is emulated, but by that time pointerup has fired and the select is closed, so the simulated click lands on the item behind
Moving the handler that closes the select to onclick avoids that issue, but we keep pointerup for mouse devices to retain the ability to glide through the items without releasing the pointer (which has never worked on iOS anyway).
DropdownMenu.Triggerreacts to scrolling on mobile devices #1912)The dropdown menu and potentially a couple other primitives would need similar fixes up next (can be done separately)
Touch, before:
Screen.Recording.2024-06-05.at.11.51.59.mov
Touch, after:
Screen.Recording.2024-06-05.at.11.52.19.mov
Behaviour with mouse devices is preserved:
Screen.Recording.2024-06-05.at.11.50.43.mov
Tested on real devices as well