necolas / react-native-web

Cross-platform React UI packages
https://necolas.github.io/react-native-web
MIT License
21.62k stars 1.79k forks source link

Touchable onPress called twice #1078

Closed tafelito closed 6 years ago

tafelito commented 6 years ago

The problem I noticed that the Touchable onPress event is being fired twice. I found that you fixed this here but digging into the source code I found that you are assuming the mouse event is not the same if the time between the events are less than 250ms. The issue is that in my case, the time difference is sometimes greater than 250 (253 255 sometimes) so the mousedown event is not filtered after the touch

How to reproduce Not sure how to reproduce as a isolated case. I know the issue is the time difference, but I'm not sure if the issue is something different that is causing a delay greater than expected. I'm using react-router and I noticed this happening when navigating from one screen to another.

When the touch event is fired, Route A is unmounted, route B is mounted, then mouse event is fired. So I have a link component that is present in both routes. If I don't include the Link component in route B, then the event is not fired, even tho the time difference is greater than 250ms

Expected behavior Touchable onPress event to fired once Environment (include versions). Did this work in previous versions?

Could something else be causing a delay on the event getting fired? It doesn't happen all the time

Ivorfason commented 6 years ago

I found a temporary solution:

window.addEventListener('mouseup', function(e) {
    e.stopPropagation();           
}, true);

I have proposed the Problem before As you described. Maybe I haven't described clearly... Event mouseup excuted after the event 'keyup' which caused the event 'onPress' excuted twice. Only found it on Android Platform...

hushicai commented 6 years ago

Also some IOS browser #1082

hushicai commented 6 years ago

related pull request #938

tafelito commented 6 years ago

If the Touchable is used as a link, using accessibilityRole="link" solves this issue . Related to #970

dharmilsan commented 6 years ago

Just had this issue of presses getting dispatched twice on an old Mi device

noeljackson commented 6 years ago

@tafelito I've got the exact same problem and I don't see a really clear workaround.

necolas commented 6 years ago

Yes I also think this is caused by the arbitrary time delay for removing the mouse events. It could be that if the work related to the press event takes a long enough time (i.e., CPU intensive, old device, etc.) the browser fires the sympathetic mouse event much later after the touch. The reason why it won't happen if the Link doesn't exist in both screens is because React has removed the element from the DOM when the route changed.

I'd guess that this problem you describe is more likely than the possibility of someone trying to use the mouse within 1s of performing a touch. So I think the time threshold between the touch and mouse events can safely be extended to at least 1000ms. But if one day the current approach is a problem, another way might be to exclude all mouse events while the app is in "touch mode".

It's important to do this work in the Responder Event Plugin injection code to avoid potentially breaking other DOM modules in your app that listen to mouse events. It also still allows people to respond directly to the React onMouse* events during touch interaction, if they have a use case.

jpstrikesback commented 6 years ago

Thanks for adding the extended delay (in c3cbd53) @necolas! Any chance you could cut a release just for this?

necolas commented 6 years ago

Let me know if any of you who experienced this issue are still seeing it with the latest version of RNW.

tafelito commented 6 years ago

It seems to be working fine now. Thanks for the fix

bcpugh commented 5 years ago

On mobile Safari, when I fire a touchable opacity normally I get a and a touchstart and touchend event. However, if the next view (triggered by the onPress) has a touchable in the same place on the screen it immediately triggers an additional: mouseover, mousemove, mousedown, mouse up, and (2x)click events, causing the next touchable to be triggered as well.

E.g., a touchable opacity A that filled the entire screen (onPress navigates to the next view), touchable opacity B that only filled the right hand side of the screen. Clicking on the left side of touchable opacity A behaves as expected, clicking on the right side of touchable opacity A triggers the additional events, causing both touchable opacity A and subsequently B (almost immediately) to be triggered.

I'm afraid I might be in a bit over my head here-- please direct me on how I can help diagnose more specifically.

RNW: 0.9.6 RN: 0.57.4 Browser: Safari (iOS12, iPhone 7)

necolas commented 5 years ago

@bcpugh please create a new issue

pnkapadia6 commented 5 years ago

In the latest versions, the issue happens when in chrome's responsive view but not in the normal view.

vipulrawat commented 5 years ago

For me, it's happening in the normal view too and very frequently.

jottenlips commented 5 years ago

@pnkapadia6 experiencing this as well

sirpy commented 5 years ago

This happens to me if a button rerenders the screen such as a route transfer and there exists a button in the new screen at the same place. If the button position is moved it doesnt happen

mohsinulhaq commented 5 years ago

@necolas do we have any issue created for https://github.com/necolas/react-native-web/issues/1078#issuecomment-491576811 I am facing the same issue. If not, I will create one.

oste commented 4 years ago

I am seeing this as well. I thought it could be related to position absolute but it sounds more generic with how I press needs to be handled. https://github.com/callstack/react-native-paper/issues/1546

oste commented 4 years ago

Can this be reopened? I don't see any benefit to creating a new issue with all of the context here. @necolas

actuallymentor commented 4 years ago

I notice this issue in react-native-web as well. An onPress triggering both touchend and mouseup.

Gregoor commented 1 year ago

I'm also experiencing this when using onStartShouldSetResponder and its related methods. It only happens on touch Safari for me though, not in Chrome's touch emulation (have not tried a real Android yet).