adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.91k stars 1.12k forks source link

Touch device press incorrectly detected as a virtual via usePress hook #3945

Closed pawelblaszczyk5 closed 1 year ago

pawelblaszczyk5 commented 1 year ago

๐Ÿ› Bug Report

When you use usePress hook it has specific internals for detecting pointerType of the press event that is currently happening. On my mobile device it doesn't seem to work correctly.

๐Ÿค” Expected Behavior

The pointerType should be properly detected as a touch

๐Ÿ˜ฏ Current Behavior

The pointerType is detected as a virtual

https://user-images.githubusercontent.com/54995479/213936336-e9d27b99-54e4-4ea9-8a99-ce1463cc8729.mp4

๐Ÿ’ Possible Solution

I've tried to debug the issue and I've come to few findings.

That's the condition that my mobile device is passing and why it's treated as a virtual: https://github.com/adobe/react-spectrum/blob/38a57d3360268fb0cb55c6b42b9a5f6f13bb57d6/packages/%40react-aria/interactions/src/usePress.ts#L321-L324

To be specific my device seems to be emitting event with width and height set to 0: https://github.com/adobe/react-spectrum/blob/38a57d3360268fb0cb55c6b42b9a5f6f13bb57d6/packages/%40react-aria/utils/src/isVirtualEvent.ts#L42-L50

Unfortunately, I'm not too familiar with screen readers detection to provide a potential fix

๐Ÿ”ฆ Context

Parts of the React Aria codes are ported to SolidJS headless library named Kobalte and I've found this to be the cause of one of the issues there. https://github.com/fabien-ml/kobalte/issues/69

๐Ÿ’ป Code Sample

I've basically copied the usePress docs example here.

๐ŸŒ Your Environment

Software Version(s)
react-aria ^3.5.0
Browser Chrome 109.0.5414.85
Operating System Android 12 (Xiaomi Poco F2 Pro)

๐Ÿงข Your Company/Team

Currently, I'm just using it for an open source project ๐Ÿ˜„

๐Ÿ•ท Tracking Issue (optional)

https://github.com/fabien-ml/kobalte/issues/69

LFDanLu commented 1 year ago

Thanks for filing this issue, unfortunately I can't reproduce this on my Samsung phone (Android 13, Chrome 109.05414.86). If possible, do you mind attaching what information the pointer event returns for your device other than width/height being 0?

pawelblaszczyk5 commented 1 year ago

Thanks a lot for talking a look into it ๐Ÿ˜Š I also can't reproduce it on all of my devices, although other Xiaomi device that I own seems to have the same issue. This is the whole event object:

{
  isTrusted: true,
  altKey: false,
  altitudeAngle: 1.5707963267948966,
  azimuthAngle: 0,
  bubbles: true,
  button: 0,
  buttons: 1,
  cancelBubble: false,
  cancelable: true,
  clientX: 206.59091186523438,
  clientY: 488.68182373046875,
  composed: true,
  ctrlKey: false,
  defaultPrevented: false,
  detail: 0,
  eventPhase: 0,
  fromElement: null,
  height: 0,
  isPrimary: true,
  layerX: 206,
  layerY: 1331,
  metaKey: false,
  movementX: 0,
  movementY: 0,
  offsetX: 60.85227584838867,
  offsetY: 23.22727394104004,
  pageX: 206.59091186523438,
  pageY: 1384.6818237304688,
  pointerId: 6,
  pointerType: "touch",
  pressure: 1,
  relatedTarget: null,
  returnValue: true,
  screenX: 206.59091186523438,
  screenY: 573.772705078125,
  shiftKey: false,
  sourceCapabilities: null,
  srcElement: button.kb - button - primary,
  tangentialPressure: 0,
  target: button.kb - button - primary,
  tiltX: 0,
  tiltY: 0,
  timeStamp: 337758.5,
  toElement: null,
  twist: 0,
  type: "pointerdown",
  which: 1,
  width: 0,
  x: 206.59091186523438,
  y: 488.68182373046875,
};
LFDanLu commented 1 year ago

Note: we'll want to revisit the places in which we use this virtual pointer event detection to see if we really need these checks. In general we should move away from relying on this check since there are so many combinations of devices/browsers that this may not work on.

deltasierra96 commented 1 year ago

This issue appears to be happening on my Samsung Tab S7 FE too. Touch events are being registered as virtual events in Chrome 114. Functionality works fine on my S10 Plus just fine and provides correct feedback in terms of hover and pressed states.

However the tablet fires the onClick/onPress events but doesn't apply correct styling based on isPressed boolean state.

weijiangan commented 1 year ago

I can concur this also happens on Galaxy Tab S6 Lite. Curiously it doesn't happen for Tab S7 unless if I use the stylus

Update: On the Galaxy Tab S6 Lite, I observed that the pointer event width height are always 0 in the beginning and then increases to 1 in subsequent events, but at that point it's already too late as usePress already store in the state that it is virtual.

devongovett commented 1 year ago

If you're having issues, please go to this page on your device, tap the button, and paste the output you get. Also please include as much info as possible about your device, including hardware, Android version, Chrome version, etc. https://codepen.io/devongovett/pen/XWyLEvy

jguddas commented 1 year ago

iOS safari fires pointer events from VoiceOver with incorrect coordinates/target

I just tried the normal voiceover doubble tab click and only got an on click event on iOS 16.6.

So this case is never reached since pointer down event never got triggered for me.

https://github.com/adobe/react-spectrum/blob/be501251a2012b2c1401e79940cbb39151c84d9d/packages/%40react-aria/interactions/src/usePress.ts#L344-L351

devongovett commented 1 year ago

Based on reports in https://github.com/nextui-org/nextui/issues/1349, it looks like some devices are firing pointer events for touch interactions with width and height of zero, which is being picked up by this check:

https://github.com/adobe/react-spectrum/blob/09a258f098147f7e67a0430b593b08168220ec4a/packages/%40react-aria/utils/src/isVirtualEvent.ts#L50

That was originally intended to detect iOS VoiceOver. We could potentially make this check specific to iOS devices by adding an isIOS() condition. However we might need to test other screen readers like e.g. on desktop (Mac VO, NVDA, etc) to see what events they fire. Alternatively we could do the inverse and check if !isAndroid() there.

yihuiliao commented 1 year ago

Hey everyone! If you were having issues, could you please go to this page on the docs, scroll to the button that says "Press me!", and report what it says? Also please include as much info as possible about your device, including hardware, Android version, Chrome version, etc. This should include an update to fix the issue but unfortunately, our team doesn't have the devices which are causing it so your help would be much appreciated!

LFDanLu commented 1 year ago

Just wanted to chime in with the following bug filed against Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1271444. The check we use in React Aria relies on several attributes of the click event, but is still quite brittle especially since Talkback sometimes issues events with pointerType: touch and height/width: 0. Ideally, attributes like isPrimary would be false for a TalkBack click.