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

[change] ResponderEventPlugin filters browser emulated mouse events #938

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions packages/react-native-web/src/exports/createElement/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,6 @@ const adjustProps = domProps => {
if (isEventHandler) {
if (isButtonRole && isDisabled) {
domProps[propName] = undefined;
} else if (propName === 'onResponderRelease') {
// Browsers fire mouse events after touch events. This causes the
// 'onResponderRelease' handler to be called twice for Touchables.
// Auto-fix this issue by calling 'preventDefault' to cancel the mouse
// events.
domProps[propName] = e => {
if (e.cancelable && !e.isDefaultPrevented()) {
e.preventDefault();
}
return prop(e);
};
} else {
// TODO: move this out of the render path
domProps[propName] = e => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,29 @@ ResponderEventPlugin.eventTypes.selectionChangeShouldSetResponder.dependencies =
ResponderEventPlugin.eventTypes.scrollShouldSetResponder.dependencies = [topScroll];
ResponderEventPlugin.eventTypes.startShouldSetResponder.dependencies = startDependencies;

let lastActiveTouchTimestamp = null;

const originalExtractEvents = ResponderEventPlugin.extractEvents;
ResponderEventPlugin.extractEvents = (topLevelType, targetInst, nativeEvent, nativeEventTarget) => {
const hasActiveTouches = ResponderTouchHistoryStore.touchHistory.numberActiveTouches > 0;

let shouldSkipMouseAfterTouch = false;
if (nativeEvent.type.indexOf('touch') > -1) {
lastActiveTouchTimestamp = Date.now();
} else if (lastActiveTouchTimestamp && nativeEvent.type.indexOf('mouse') > -1) {
const now = Date.now();
shouldSkipMouseAfterTouch = lastActiveTouchTimestamp - now < 250;
Copy link

@doxiaodong doxiaodong Nov 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work when there is a window.alert call @necolas

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that mean?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it means if you call window.alert in onPress , and then shouldSkipMouseAfterTouch is not work right

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Maybe don't call alert

Copy link

@doxiaodong doxiaodong Nov 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, But people may not understand this. Is there a way to record timestamp after onPress called tick

}

if (
// Filter out mousemove and mouseup events when a touch hasn't started yet
((topLevelType === topMouseMove || topLevelType === topMouseUp) && !hasActiveTouches) ||
// Filter out events from wheel/middle and right click.
(nativeEvent.button === 1 || nativeEvent.button === 2)
(nativeEvent.button === 1 || nativeEvent.button === 2) ||
// Filter out mouse events that browsers dispatch immediately after touch events end
// Prevents the REP from calling handlers twice for touch interactions.
// See #802 and #932.
shouldSkipMouseAfterTouch
) {
return;
}
Expand Down
10 changes: 9 additions & 1 deletion website/storybook/1-components/Switch/SwitchScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import PropOnValueChange from './examples/PropOnValueChange';
import PropThumbColor from './examples/PropThumbColor';
import PropTrackColor from './examples/PropTrackColor';
import PropValue from './examples/PropValue';
import TouchableWrapper from './examples/TouchableWrapper';
import React from 'react';
import UIExplorer, {
AppText,
Expand Down Expand Up @@ -127,12 +128,19 @@ const SwitchScreen = () => (

<Section title="More examples">
<DocItem
description="Custom sizes can be created using styles"
description="Custom sizes can be created using styles."
example={{
code: '<Switch style={{ height: 30 }} />',
render: () => <CustomSize />
}}
/>

<DocItem
description="Wrapped in a Touchable."
example={{
render: () => <TouchableWrapper />
}}
/>
</Section>
</UIExplorer>
);
Expand Down
39 changes: 39 additions & 0 deletions website/storybook/1-components/Switch/examples/TouchableWrapper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* eslint-disable react/jsx-no-bind */
/**
* @flow
*/

import React from 'react';
import { Switch, TouchableHighlight, View } from 'react-native';

class TouchableWrapperExample extends React.PureComponent {
state = {
on: false
};

render() {
const { on } = this.state;

return (
<View>
<TouchableHighlight onPress={() => {}} style={style} underlayColor="#eee">
<Switch onValueChange={this._handleChange} value={on} />
</TouchableHighlight>
</View>
);
}

_handleChange = value => {
this.setState({ on: value });
};
}

const style = {
alignSelf: 'flex-start',
borderWidth: 1,
borderColor: '#ddd',
paddingHorizontal: 50,
paddingVertical: 20
};

export default TouchableWrapperExample;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ export default class TouchableWrapper extends React.Component {

_handlePress = () => {
if (this._input) {
this._input.focus();
setTimeout(() => {
this._input.focus();
}, 0);
}
};

Expand Down