Touchable issues

[necolas]

The problem

The overall issue appears to be that a) events don't always occurs in the same order, which means the responder is not transferred correctly, b) mouse move events during a touch can cause the responder to be terminated, and c) scroll events are not always fired in situations where they are expected in React Native.

There are problems with the responder event system setup that are covered by these issues:

#693

https://glitch.com/edit/#!/statuesque-bedbug

locationX is relative to the element on which the native event occurred but not the element which the responder event is attached.

#731

https://glitch.com/edit/#!/cultured-bracket

In React Native, ScrollView will trigger onScroll events if you're dragging down when at the top of the scroll view - this allows the responder system to transfer the responder grant from the touchable back to the scroll view. But the web stops dispatching scroll events when scrollTop === 0, so the responder system leaves the touchable as the responder. The reported issue goes away when ScrollView dispatches a DOM scroll event after it receives a touchmove that occurs when scrollTop === 0.

#829

There are 2 significant issues with using Touchable* components within an app that relies on the document.body (rather than ScrollView) being the root scroller. I believe the root cause of both issues is that the body is not part of the responder system, which Touchable and ScrollView rely on to coordinate transfer of the responder grant.

#1164

https://codesandbox.io/s/6njwzqojnr

TouchableOpacity will trigger another component at the same location when changing views (if second component is an "" or "")

#1124

Across multiple browsers, if you hold the mouse very still onPress triggers, but if the mouse is in motion onPress does not work.

Context

Part of the solution may related to producing the correct events (as can be inferred from React Native). This might involve changes to the event filtering logic in the REP, and the scroll event logic in ScrollView.

• Responder Event Plugin
• ScrollResponder
• ScrollView

Related incomplete patches

Attempts to patch these problems.

• https://github.com/necolas/react-native-web/pull/997/files
• rxrossi@1e3eb7f

[tafelito]

@necolas what's missing on the 2 solutions you mentioned above?

I don't see this happening at twitter, is this an issue from the latest versions of RNW, are you doing anything different there?

[paularmstrong]

@tafelito: Twitter doesn't use ScrollView and has a bunch of hacky workarounds in order to avoid using it (because we rely on body-scrolling, for various reasons). In the long run, this isn't great and we'd like a path to move away from our workaround.

[tafelito]

For us this also happens outside a ScrollView. We are avoiding it in some places just using preventDefault but that just also a hack and it doesn't even work in every case

…

________________________________ From: Paul Armstrong <notifications@github.com> Sent: Wednesday, January 16, 2019 18:24 To: necolas/react-native-web Cc: tafelito; Mention Subject: Re: [necolas/react-native-web] Responder Event System issues (#1219) @tafelito<https://github.com/tafelito>: Twitter doesn't use ScrollView and has a bunch of hacky workarounds in order to avoid using it (because we rely on body-scrolling, for various reasons). In the long run, this isn't great and we'd like a path to move away from our workaround. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#1219 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFsmlCJS0V7tKkrZCZxIwIKrZVcNS2b0ks5vD7SVgaJpZM4ZoyPr>.

[viggyfresh]

Hi @necolas,

Just wanted to follow up on @aligg's comment in #1124 (see #1124 (comment)) - we ended up solving our issue! For posterity: we had inadvertently mixed our metaphors and wrapped a bunch of RNW components inside a div with overflow-y set to scroll; when we replaced this with a RNW ScrollView, all of our problems magically went away! I'm not at all familiar with the responder event system, but I presume this particular blend of web and RNW components isn't supported (or wise).

[RichardLindhout]

@necolas Maybe we can listen to a normal click event of the browser instead of onTouchStart etc.

[EyMaddis]

@RichardLindhout
I use custom touchables right now and it works by using something like this:

<TouchableOpacity
          ...
          {...(Platform.OS === 'web'
            ? {
                // When scrolling the document body, the touchables might be triggered
                // see  https://github.com/necolas/react-native-web/issues/1219
                onClick: this.onPress,
              }
            : {
                onPress: this.onPress,
              })}
/>

Edit: This currently might not work if you use accessibilityRole="link", see #1348.

[loudwinston]

@EyMaddis good tip! We applied this as a monkey-patch for Touchable components by using webpack NormalModuleReplacementPlugin so we don't have to apply this ternary all over our app

webpack.config.js

plugins: [
  new webpack.NormalModuleReplacementPlugin(
    /^react-native$/,
    path.resolve(__dirname, './src/web/react-native-web.tsx')
  )
]

react-native-web.tsx

import React from 'react'
import * as ReactNativeWeb from 'react-native-web'

const createTouchable = (Touchable) => (props: any) => {
  const { onPress, ...restProps } = props
  return (
    <Touchable {...restProps} onClick={onPress} />
  )
}

export * from 'react-native-web'

export const TouchableOpacity = createTouchable(ReactNativeWeb.TouchableOpacity)
export const TouchableWithoutFeedback = createTouchable(ReactNativeWeb.TouchableWithoutFeedback)
// etc

[tafelito]

one problem I see with that solution is that you manually need to handle propagation. If you have a Touchable inside another touchable, the onPress only gets called once. Using onClick you have to stop propagation depending on your use case it might just work adding in to the onClick event

[EyMaddis]

The touchable problem with touchables triggering on scrollY=0 boils down to this line:

https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/modules/ScrollResponder/index.js#L336 (scrollResponderIsAnimating)
as it controls scrollResponderHandleStartShouldSetResponderCapture.

If I understand it correctly the problem is that the touchstart event fires first and then scroll. But touchstart was already triggered and grants the handling to the touchable...
Once the browser triggers scroll, the touchable handling should be terminated and the scrollview should seize control.

Edit: I cannot find a way to send signals to the responder system. I would like to trigger a termination to other touchables once a browser scroll was registered. Where is the responder system actually implemented? Does it occur in RNW, in vendor code or from react, @necolas ? If so, do you have any hints?

[stoicskyline]

Noticed that on certain mobile browsers like iOS Safari, scrolling will accidentally click when gesture is released. Is this the same issue that you're describing @EyMaddis ?

[EyMaddis]

@Jorgechen Yes, see #731

[andreassavva]

About #1124 it only happens when I have scrolled down below approximately 50 pixels. If I am at the top of the page, the onPress event is being called as it should. If I first scroll below 50 pixels or more, click down inside the TouchableOpacity and then move the mouse (still inside the TouchableOpacity component), then the press event is being released and the onPress event is not called when I release the mouse click.

Anyone that can shed some light on where in react-native-web this is happening? It would help me search for a way to fix this

[andreassavva]

I fixed #1124 . In Touchable/index.js the touch from the native event is extracted in the touchableHandleResponderMove function and pageX and pageY are received from it. These two attributes give the x and y positions in the whole page, which means that the scrolled pixels are added to them.

Getting clientX and clientY from touch instead of pageX and pageY fixes this issue for me.

[brunolemos]

Is there any workaround for the touchable press getting triggered if I scroll and release the finger on top of the touchable? This is hurting the ux of my app.

cc @MoOx

EDIT: I just realized (after reading this comment) that the issue only happens because my TouchableOpacity is inside react-window. If I use the built-in FlatList the issue doesn't happen. Any workaround for that?

[stoicskyline]

@brunolemos Have you tried the method suggested by @EyMaddis ? That solution has worked for me until an official fix.

By the way, good work on DevHub and hope you continue writing dev articles. Obrigado.

[brunolemos]

I ended up using a different workaround. I have a Touchable.tsx component and I check the pageX and pageY values from the press events, and if the user moved the finger I assume it was a scroll and cancel the press event. Seems to work great, I'm happy with the result.

const pressInPagePointRef = useRef({ x: 0, y: 0 })

const onPressIn = useCallback<NonNullable<TouchableProps['onPressIn']>>(
  e => {
    pressInPagePointRef.current = {
      x: e.nativeEvent.pageX,
      y: e.nativeEvent.pageY,
    }

    if (_onPressIn) _onPressIn(e)
  },
  [_onPressIn],
)

const onPressOut = useCallback<NonNullable<TouchableProps['onPressOut']>>(
  e => {
    if (_onPressOut) _onPressOut(e)

    const [x, y] = [e.nativeEvent.pageX, e.nativeEvent.pageY]
    if (
      Math.abs(pressInPagePointRef.current.x - x) > 1 ||
      Math.abs(pressInPagePointRef.current.y - y) > 1
    ) {
      e.preventDefault()
    }
  },
  [_onPressOut],
)

const onPress = useCallback<NonNullable<TouchableProps['onPress']>>(
  e => {
    if (e.isDefaultPrevented()) return
    if (_onPress) _onPress(e)
  },
  [_onPress],
)

[jfrolich]

@necolas: Do you have a recommendation how to handle the current issue with window scrolling being broken? Are you open to have a temporary workaround in the stable RNW, before the responder system rewrite that fixes the scrolling issue has landed. (Perhaps a fix that does not handle all cases, but makes Touchables usable in a window scroll environment). Or do you foresee the new responder system landing soon-ish? Or would you recommend to do a patch-package for now?

[EyMaddis]

@jfrolich Could you try this PR? #1472

[jfrolich]

@EyMaddis: Will try! Looks like a good solution! I see that it's conflicting now with master?

[necolas]

Please try version 0.0.0-33f8505d6 (a canary release) and let me know if the problem is resolved. See #1538

[rxb]

I had the same / similar problem as @jfrolich which brought me to this issue. Version 0.0.0-33f8505d6 totally fixed it for me. Thanks for all your work on this!

[jfrolich]

I tried different solutions, and they didn't really solve the problem. The new fix by @necolas works perfectly however! Only now reanimated/react-native-gesture-handler are breaking, probably because this is such a big change in the internals.

[necolas]

Are they breaking on findNodeHandle calls? That API is going away in React Native/DOM as it doesn't work with functional components or concurrent mode

[jfrolich]

Are they breaking on findNodeHandle calls? That API is going away in React Native/DOM as it doesn't work with functional components or concurrent mode

Yes precisely that. What API is replacing that?

[necolas]

React refs to host nodes should be used. Facebook and React Native itself have mostly migrated off those APIs already. https://reactjs.org/docs/react-dom.html#finddomnode

[jfrolich]

React refs to host nodes should be used. Facebook and React Native itself have mostly migrated off those APIs already. https://reactjs.org/docs/react-dom.html#finddomnode

Gotcha, that's what I thought as well 👍

[Maker-Mark]

Still seeing this issue!