Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onValueChange not triggered on input based elements for touch events #932

Closed
mauron85 opened this issue May 6, 2018 · 9 comments
Closed

Comments

@mauron85
Copy link

mauron85 commented May 6, 2018

Do you want to request a feature or report a bug?
bug
What is the current behavior?
Elements like Switch, Input... doesn't fire onValueChange on mobile browsers and Chrome when switched to emulate touch events.

**If the current behavior is a bug, please provide the steps to reproduce and
Not sure if this is needed but issue can be easily reproduced on simple Switch component.

Switch onValueChange on mobile browsers and Chrome is not called.

How to reproduce:

  1. Create Switch component with onValueChange event handler
  2. In Chrome switch to emulate touch events
  3. Click on Switch

What is the expected behavior?
Switch onValueChange will be called

Environment (include versions). Did this work in previous versions?
Yes it did. Not sure about exact version number, but version used following createElement works.
I think problem is related to line: https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/exports/createElement/index.js#L56

  • OS: Ubuntu 16.04
  • Device:
  • Browser: Chrome 66
  • React Native for Web (version):0.6.1
  • React (version):16.3.2
@necolas
Copy link
Owner

necolas commented May 7, 2018

I can't reproduce this in the RNW storybook. Switch and onValueChange work as expected in Chrome on my mobile device and when emulating touch events in desktop Chrome on macOS.

@mauron85
Copy link
Author

mauron85 commented May 7, 2018

Seems to be cause by other plugin messing up with events. Will update if I find the real cause. Closing for now.

@mauron85 mauron85 closed this as completed May 7, 2018
@mauron85
Copy link
Author

mauron85 commented May 7, 2018

Sorry for false report.

@mauron85
Copy link
Author

mauron85 commented May 7, 2018

I've found conflicting library. It's nativebase. Inputs doesn't trigger onValueChange when inside nativebase List.

Create repo, which demonstrates the issue https://github.com/mauron85/rn-switch-glitch

@mauron85
Copy link
Author

mauron85 commented May 7, 2018

Reported issue GeekyAnts/NativeBase#1874

@mauron85
Copy link
Author

mauron85 commented May 8, 2018

@necolas I did some additional research and it might be issue with react-native-web after all.

The problem seems to occur when Switch component is wrapped with touchable eg. TouchableHighlight.

Example:

<TouchableHighlight>
  <Switch value={this.state.switch2Enabled} onValueChange={this.toggleSwitch2}/>
</TouchableHighlight>

This only works with mouse events. Touch event is not propagated correctly.

Interestly wrapping touchable in another touchable works:

<TouchableHighlight>
  <TouchableHighlight onPress={() => console.log('btn pressed')}>
    <View style={styles.btn}>
      <Text>I can be touched</Text>
    </View>
  </TouchableHighlight>
</TouchableHighlight>

I've created example you can reproduce this issue: https://grateful-stem.glitch.me/
Source: https://glitch.com/edit/#!/grateful-stem

@mauron85 mauron85 reopened this May 8, 2018
@mauron85
Copy link
Author

mauron85 commented May 8, 2018

Reverting createElement to https://raw.githubusercontent.com/mauron85/react-native-web/extended/src/modules/createElement/index.js fixes the problem. So it really seems to be related to this commit 893963a

EDIT: But it's not as easy as just reverting, because then touchables will be called twice.

necolas added a commit that referenced this issue May 8, 2018
Browsers dispatch mouse events after touch events:
https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent

There have been several attempts to avoid this behaviour affecting the
ResponderEvent system. The previous approach of cancelling the event in
the `onResponderRelease` event handler can end up cancelling other
events that are expected, e.g., `focus`.

Instead, this patch changes the `ResponderEventPlugin.extractEvents`
function to filter the mouse events that occur a short time after a
touch event. (It's assumed that people will not be clicking a mouse
within a few hundred ms of performing a touch.) This allows the
ResponderEvent system to function as expected and leaves other callbacks
to fire as they would be expected to in React DOM, i.e., both
'onTouchStart' and 'onMouseDown' will be called following a touch start.

Fix #932
Ref #802
@necolas
Copy link
Owner

necolas commented May 8, 2018

Try this branch https://github.com/necolas/react-native-web/tree/react-native-web/mouse-event-filter

@mauron85
Copy link
Author

mauron85 commented May 9, 2018

Hi @necolas. After reading commit comment and previous issues, this seems like kind of hard to solve problem. It's easy to break something else. Anyway I've just tried mouse-event-filter branch and it's seems like it fixes this particular issue.

necolas added a commit that referenced this issue May 18, 2018
Browsers dispatch mouse events after touch events:
https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent

There have been several attempts to avoid this behaviour affecting the
ResponderEvent system. The previous approach of cancelling the event in
the `onResponderRelease` event handler can end up cancelling other
events that are expected, e.g., `focus`.

Instead, this patch changes the `ResponderEventPlugin.extractEvents`
function to filter the mouse events that occur a short time after a
touch event. (It's assumed that people will not be clicking a mouse
within a few hundred ms of performing a touch.) This allows the
ResponderEvent system to function as expected and leaves other callbacks
to fire as they would be expected to in React DOM, i.e., both
`onTouchStart` and `onMouseDown` will be called following a touch start.

Fix #932
Close #938
Ref #802
necolas added a commit that referenced this issue May 18, 2018
Browsers dispatch mouse events after touch events:
https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent

There have been several attempts to avoid this behaviour affecting the
ResponderEvent system. The previous approach of cancelling the event in
the `onResponderRelease` event handler can end up cancelling other
events that are expected, e.g., `focus`.

Instead, this patch changes the `ResponderEventPlugin.extractEvents`
function to filter the mouse events that occur a short time after a
touch event. (It's assumed that people will not be clicking a mouse
within a few hundred ms of performing a touch.) This allows the
ResponderEvent system to function as expected and leaves other callbacks
to fire as they would be expected to in React DOM, i.e., both
`onTouchStart` and `onMouseDown` will be called following a touch start.

Fix #835
Fix #888
Fix #932
Close #938
Ref #802
muratakbal pushed a commit to mdcollab/react-native-web that referenced this issue Aug 1, 2018
Browsers dispatch mouse events after touch events:
https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent

There have been several attempts to avoid this behaviour affecting the
ResponderEvent system. The previous approach of cancelling the event in
the `onResponderRelease` event handler can end up cancelling other
events that are expected, e.g., `focus`.

Instead, this patch changes the `ResponderEventPlugin.extractEvents`
function to filter the mouse events that occur a short time after a
touch event. (It's assumed that people will not be clicking a mouse
within a few hundred ms of performing a touch.) This allows the
ResponderEvent system to function as expected and leaves other callbacks
to fire as they would be expected to in React DOM, i.e., both
`onTouchStart` and `onMouseDown` will be called following a touch start.

Fix necolas#835
Fix necolas#888
Fix necolas#932
Close necolas#938
Ref necolas#802
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants