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.

Sign up for GitHub

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

[flow-strict] Flow TouchableNativeFeedback.android.js #22176

Closed
wants to merge 9 commits into from
Closed

[flow-strict] Flow TouchableNativeFeedback.android.js #22176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bd8c741
[flow] Flow TouchableNativeFeedback.android.js
mottox2 Nov 6, 2018
d5d4426
[flow-strict] Apply flow-strict
mottox2 Nov 6, 2018
6d44637
Apply flow to Libraries/Components/Touchable/Touchable.js
mottox2 Nov 6, 2018
1a03b06
Fix
mottox2 Nov 6, 2018
b455f5f
Fix typo
mottox2 Nov 6, 2018
3fb851b
Fix styles
mottox2 Nov 7, 2018
4623d51
Remove $FlowFixMe
mottox2 Nov 7, 2018
245a510
Merge branch 'master' of https://github.com/facebook/react-native int…
mottox2 Nov 7, 2018
68f335f
Check color
mottox2 Nov 7, 2018
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
100 changes: 83 additions & 17 deletions Libraries/Components/Touchable/Touchable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
* @format
*/

Expand All @@ -23,6 +24,9 @@ const View = require('View');
const keyMirror = require('fbjs/lib/keyMirror');
const normalizeColor = require('normalizeColor');

import type {PressEvent, SyntheticEvent, LayoutEvent} from 'CoreEventTypes';
import type {EdgeInsetsProp} from 'EdgeInsetsPropType';

/**
* `Touchable`: Taps done right.
*
Expand Down Expand Up @@ -111,6 +115,7 @@ const normalizeColor = require('normalizeColor');
/**
* Touchable states.
*/

const States = keyMirror({
NOT_RESPONDER: null, // Not the responder
RESPONDER_INACTIVE_PRESS_IN: null, // Responder, inactive, in the `PressRect`
Expand All @@ -122,10 +127,33 @@ const States = keyMirror({
ERROR: null,
});

/**
type State =
| typeof States.NOT_RESPONDER
| typeof States.RESPONDER_INACTIVE_PRESS_IN
| typeof States.RESPONDER_INACTIVE_PRESS_OUT
| typeof States.RESPONDER_ACTIVE_PRESS_IN
| typeof States.RESPONDER_ACTIVE_PRESS_OUT
| typeof States.RESPONDER_ACTIVE_LONG_PRESS_IN
| typeof States.RESPONDER_ACTIVE_LONG_PRESS_OUT
| typeof States.ERROR;

/*
* Quick lookup map for states that are considered to be "active"
*/

const baseStatesConditions = {
NOT_RESPONDER: false,
RESPONDER_INACTIVE_PRESS_IN: false,
RESPONDER_INACTIVE_PRESS_OUT: false,
RESPONDER_ACTIVE_PRESS_IN: false,
RESPONDER_ACTIVE_PRESS_OUT: false,
RESPONDER_ACTIVE_LONG_PRESS_IN: false,
RESPONDER_ACTIVE_LONG_PRESS_OUT: false,
ERROR: false,
};

const IsActive = {
...baseStatesConditions,
RESPONDER_ACTIVE_PRESS_OUT: true,
RESPONDER_ACTIVE_PRESS_IN: true,
};
Expand All @@ -135,12 +163,14 @@ const IsActive = {
* therefore eligible to result in a "selection" if the press stops.
*/
const IsPressingIn = {
...baseStatesConditions,
RESPONDER_INACTIVE_PRESS_IN: true,
RESPONDER_ACTIVE_PRESS_IN: true,
RESPONDER_ACTIVE_LONG_PRESS_IN: true,
};

const IsLongPressingIn = {
...baseStatesConditions,
RESPONDER_ACTIVE_LONG_PRESS_IN: true,
};

Expand All @@ -157,6 +187,15 @@ const Signals = keyMirror({
LONG_PRESS_DETECTED: null,
});

type Signal =
| typeof Signals.DELAY
| typeof Signals.RESPONDER_GRANT
| typeof Signals.RESPONDER_RELEASE
| typeof Signals.RESPONDER_TERMINATED
| typeof Signals.ENTER_PRESS_RECT
| typeof Signals.LEAVE_PRESS_RECT
| typeof Signals.LONG_PRESS_DETECTED;

/**
* Mapping from States x Signals => States
*/
Expand Down Expand Up @@ -391,7 +430,7 @@ const TouchableMixin = {
* @param {SyntheticEvent} e Synthetic event from event system.
*
*/
touchableHandleResponderGrant: function(e) {
touchableHandleResponderGrant: function(e: SyntheticEvent<any>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function accepts a PressEvent.

const dispatchID = e.currentTarget;
// Since e is used in a callback invoked on another event loop
// (as in setTimeout etc), we need to call e.persist() on the
Expand Down Expand Up @@ -432,21 +471,21 @@ const TouchableMixin = {
/**
* Place as callback for a DOM element's `onResponderRelease` event.
*/
touchableHandleResponderRelease: function(e) {
touchableHandleResponderRelease: function(e: PressEvent) {
this._receiveSignal(Signals.RESPONDER_RELEASE, e);
},

/**
* Place as callback for a DOM element's `onResponderTerminate` event.
*/
touchableHandleResponderTerminate: function(e) {
touchableHandleResponderTerminate: function(e: PressEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You've typed this function as accepting a PressEvent, but later on in the function, we forward this event to _receiveSignal, which accepts a LayoutEvent. There's a contradiction here.

this._receiveSignal(Signals.RESPONDER_TERMINATED, e);
},

/**
* Place as callback for a DOM element's `onResponderMove` event.
*/
touchableHandleResponderMove: function(e) {
touchableHandleResponderMove: function(e: PressEvent) {
// Not enough time elapsed yet, wait for highlight -
// this is just a perf optimization.
if (
Expand Down Expand Up @@ -633,7 +672,14 @@ const TouchableMixin = {
UIManager.measure(tag, this._handleQueryLayout);
},

_handleQueryLayout: function(l, t, w, h, globalX, globalY) {
_handleQueryLayout: function(
l: number,
t: number,
w: number,
h: number,
globalX: number,
globalY: number,
) {
//don't do anything UIManager failed to measure node
if (!l && !t && !w && !h && !globalX && !globalY) {
return;
Expand All @@ -652,12 +698,12 @@ const TouchableMixin = {
);
},

_handleDelay: function(e) {
_handleDelay: function(e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit curious. How come you decided to type this with LayoutEvent?

this.touchableDelayTimeout = null;
this._receiveSignal(Signals.DELAY, e);
},

_handleLongDelay: function(e) {
_handleLongDelay: function(e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same deal here. Why is this a LayoutEvent and not a PressEvent? This function calls _receiveSignal, which I think should accept a PressEvent.

this.longPressDelayTimeout = null;
const curState = this.state.touchable.touchState;
if (
Expand Down Expand Up @@ -685,7 +731,7 @@ const TouchableMixin = {
* @throws Error if invalid state transition or unrecognized signal.
* @sideeffects
*/
_receiveSignal: function(signal, e) {
_receiveSignal: function(signal: Signal, e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This gets forwarded to _performSideEffectsForTransition. So, shouldn't e be typed as PressEvent?

const responderID = this.state.touchable.responderID;
const curState = this.state.touchable.touchState;
const nextState = Transitions[curState] && Transitions[curState][signal];
Expand Down Expand Up @@ -725,14 +771,14 @@ const TouchableMixin = {
this.longPressDelayTimeout = null;
},

_isHighlight: function(state) {
_isHighlight: function(state: State) {
return (
state === States.RESPONDER_ACTIVE_PRESS_IN ||
state === States.RESPONDER_ACTIVE_LONG_PRESS_IN
);
},

_savePressInLocation: function(e) {
_savePressInLocation: function(e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Later on in the function, we call TouchEventUtils.extractSingleTouch, which expects to be called with the native event of a PressEvent. So, I think this type should be PressEvent and not LayoutEvent.

const touch = TouchEventUtils.extractSingleTouch(e.nativeEvent);
const pageX = touch && touch.pageX;
const pageY = touch && touch.pageY;
Expand All @@ -741,7 +787,12 @@ const TouchableMixin = {
this.pressInLocation = {pageX, pageY, locationX, locationY};
},

_getDistanceBetweenPoints: function(aX, aY, bX, bY) {
_getDistanceBetweenPoints: function(
aX: number,
aY: number,
bX: number,
bY: number,
) {
const deltaX = aX - bX;
const deltaY = aY - bY;
return Math.sqrt(deltaX * deltaX + deltaY * deltaY);
Expand All @@ -758,7 +809,12 @@ const TouchableMixin = {
* @param {Event} e Native event.
* @sideeffects
*/
_performSideEffectsForTransition: function(curState, nextState, signal, e) {
_performSideEffectsForTransition: function(
curState: State,
nextState: State,
signal: Signal,
e: LayoutEvent,
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This event is forwarded to _startHighlight and _endHighlight. Shouldn't it therefore also be typed as a PressEvent?

const curIsHighlight = this._isHighlight(curState);
const newIsHighlight = this._isHighlight(nextState);

Expand Down Expand Up @@ -813,12 +869,12 @@ const TouchableMixin = {
UIManager.playTouchSound();
},

_startHighlight: function(e) {
_startHighlight: function(e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same deal as _endHighlight here.

this._savePressInLocation(e);
this.touchableHandleActivePressIn && this.touchableHandleActivePressIn(e);
},

_endHighlight: function(e) {
_endHighlight: function(e: LayoutEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method calls this.touchableHandleActivePressOut, which you typed as accepting a PressEvent inside TouchableNativeFeedback.android.js. So, shouldn't the type of e also be PressEvent?

if (this.touchableHandleActivePressOut) {
if (
this.touchableGetPressOutDelayMS &&
Expand All @@ -840,7 +896,13 @@ const Touchable = {
/**
* Renders a debugging overlay to visualize touch target with hitSlop (might not work on Android).
*/
renderDebugView: ({color, hitSlop}) => {
renderDebugView: ({
color,
hitSlop,
}: {
color: string | number,
hitSlop: EdgeInsetsProp,
}) => {
if (!Touchable.TOUCH_TARGET_DEBUG) {
return null;
}
Expand All @@ -854,8 +916,12 @@ const Touchable = {
for (const key in hitSlop) {
debugHitSlopStyle[key] = -hitSlop[key];
}
const normalizedColor = normalizeColor(color);
if (typeof normalizedColor !== 'number') {
return null;
}
const hexColor =
'#' + ('00000000' + normalizeColor(color).toString(16)).substr(-8);
'#' + ('00000000' + normalizedColor.toString(16)).substr(-8);
return (
<View
pointerEvents="none"
Expand Down
13 changes: 7 additions & 6 deletions Libraries/Components/Touchable/TouchableNativeFeedback.android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
*/

Expand All @@ -22,6 +23,8 @@ const createReactClass = require('create-react-class');
const ensurePositiveDelayProps = require('ensurePositiveDelayProps');
const processColor = require('processColor');

import type {PressEvent} from 'CoreEventTypes';

const rippleBackgroundPropType = PropTypes.shape({
type: PropTypes.oneOf(['RippleAndroid']),
color: PropTypes.number,
Expand All @@ -38,8 +41,6 @@ const backgroundPropType = PropTypes.oneOfType([
themeAttributeBackgroundPropType,
]);

type Event = Object;

const PRESS_RETENTION_OFFSET = {top: 20, left: 20, right: 20, bottom: 30};

/**
Expand Down Expand Up @@ -167,7 +168,7 @@ const TouchableNativeFeedback = createReactClass({
* `Touchable.Mixin` self callbacks. The mixin will invoke these if they are
* defined on your component.
*/
touchableHandleActivePressIn: function(e: Event) {
touchableHandleActivePressIn: function(e: PressEvent) {
this.props.onPressIn && this.props.onPressIn(e);
this._dispatchPressedStateChange(true);
if (this.pressInLocation) {
Expand All @@ -178,16 +179,16 @@ const TouchableNativeFeedback = createReactClass({
}
},

touchableHandleActivePressOut: function(e: Event) {
touchableHandleActivePressOut: function(e: PressEvent) {
this.props.onPressOut && this.props.onPressOut(e);
this._dispatchPressedStateChange(false);
},

touchableHandlePress: function(e: Event) {
touchableHandlePress: function(e: PressEvent) {
this.props.onPress && this.props.onPress(e);
},

touchableHandleLongPress: function(e: Event) {
touchableHandleLongPress: function(e: PressEvent) {
this.props.onLongPress && this.props.onLongPress(e);
},

Expand Down