Skip to content

Commit

Permalink
Improve touchable debugging
Browse files Browse the repository at this point in the history
Summary:Set `Touchable.TOUCH_TARGET_DEBUG` to see colored borders/text to all touchables.

Different touchable types are color-coded differently.

If there is `hitSlop`, it will be rendered with an extra view with a dashed border of the same color (not visible on
Android because `overflow: 'hidden'`).

`Text` with `onPress` directly set is just colored.

Added some extra checks to `TouchableWithoutFeedback` since it could silently break if the child is not a native
component.

Also added better error output for `ensureComponentIsNative` so it's easier to track down issues. I really wish there
was a cleaner way to get the component and owner names consistently, it would help make good debug messages way easier
to write.

Reviewed By: ericvicenti

Differential Revision: D3149865

fb-gh-sync-id: 602fc3474ae7636e32af529eb7ac52ac5b858030
fbshipit-source-id: 602fc3474ae7636e32af529eb7ac52ac5b858030
  • Loading branch information
sahrens authored and Facebook Github Bot 7 committed Apr 14, 2016
1 parent e02d400 commit 5c9b46c
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 22 deletions.
56 changes: 50 additions & 6 deletions Libraries/Components/Touchable/Touchable.js
Original file line number Original file line Diff line number Diff line change
@@ -1,15 +1,25 @@
/** /**
* Copyright (c) 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule Touchable * @providesModule Touchable
*/ */


'use strict'; 'use strict';


var BoundingDimensions = require('BoundingDimensions'); const BoundingDimensions = require('BoundingDimensions');
var Position = require('Position'); const Position = require('Position');
var TouchEventUtils = require('fbjs/lib/TouchEventUtils'); const React = require('React'); // eslint-disable-line no-unused-vars
const TouchEventUtils = require('fbjs/lib/TouchEventUtils');
const View = require('View');


var keyMirror = require('fbjs/lib/keyMirror'); const keyMirror = require('fbjs/lib/keyMirror');
var queryLayoutByID = require('queryLayoutByID'); const normalizeColor = require('normalizeColor');
const queryLayoutByID = require('queryLayoutByID');


/** /**
* `Touchable`: Taps done right. * `Touchable`: Taps done right.
Expand Down Expand Up @@ -713,7 +723,41 @@ var TouchableMixin = {
}; };


var Touchable = { var Touchable = {
Mixin: TouchableMixin Mixin: TouchableMixin,
TOUCH_TARGET_DEBUG: false, // Set this locally to help debug touch targets.
/**
* Renders a debugging overlay to visualize touch target with hitSlop (might not work on Android).
*/
renderDebugView: ({color, hitSlop}) => {
if (!Touchable.TOUCH_TARGET_DEBUG) {
return null;
}
if (!__DEV__) {
throw Error('Touchable.TOUCH_TARGET_DEBUG should not be enabled in prod!');
}
const debugHitSlopStyle = {};
hitSlop = hitSlop || {top: 0, bottom: 0, left: 0, right: 0};
for (const key in hitSlop) {
debugHitSlopStyle[key] = -hitSlop[key];
}
const hexColor = '#' + ('00000000' + normalizeColor(color).toString(16)).substr(-8);
return (
<View
pointerEvents="none"
style={{
position: 'absolute',
borderColor: hexColor.slice(0, -2) + '55', // More opaque
borderWidth: 1,
borderStyle: 'dashed',
backgroundColor: hexColor.slice(0, -2) + '0F', // Less opaque
...debugHitSlopStyle
}}
/>
);
}
}; };
if (Touchable.TOUCH_TARGET_DEBUG) {
console.warn('Touchable.TOUCH_TARGET_DEBUG is enabled');
}


module.exports = Touchable; module.exports = Touchable;
1 change: 1 addition & 0 deletions Libraries/Components/Touchable/TouchableBounce.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ var TouchableBounce = React.createClass({
onResponderRelease={this.touchableHandleResponderRelease} onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}> onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children} {this.props.children}
{Touchable.renderDebugView({color: 'orange', hitSlop: this.props.hitSlop})}
</Animated.View> </Animated.View>
); );
} }
Expand Down
1 change: 1 addition & 0 deletions Libraries/Components/Touchable/TouchableHighlight.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ var TouchableHighlight = React.createClass({
ref: CHILD_REF, ref: CHILD_REF,
} }
)} )}
{Touchable.renderDebugView({color: 'green', hitSlop: this.props.hitSlop})}
</View> </View>
); );
} }
Expand Down
12 changes: 10 additions & 2 deletions Libraries/Components/Touchable/TouchableNativeFeedback.android.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
var PropTypes = require('ReactPropTypes'); var PropTypes = require('ReactPropTypes');
var React = require('React'); var React = require('React');
var ReactNative = require('ReactNative'); var ReactNative = require('ReactNative');
var ReactNativeViewAttributes = require('ReactNativeViewAttributes');
var Touchable = require('Touchable'); var Touchable = require('Touchable');
var TouchableWithoutFeedback = require('TouchableWithoutFeedback'); var TouchableWithoutFeedback = require('TouchableWithoutFeedback');
var UIManager = require('UIManager'); var UIManager = require('UIManager');
Expand Down Expand Up @@ -206,13 +205,22 @@ var TouchableNativeFeedback = React.createClass({
}, },


render: function() { render: function() {
const child = onlyChild(this.props.children);
let children = child.props.children;
if (Touchable.TOUCH_TARGET_DEBUG && child.type.displayName === 'View') {
if (!Array.isArray(children)) {
children = [children];
}
children.push(Touchable.renderDebugView({color: 'brown', hitSlop: this.props.hitSlop}));
}
var childProps = { var childProps = {
...onlyChild(this.props.children).props, ...child.props,
nativeBackgroundAndroid: this.props.background, nativeBackgroundAndroid: this.props.background,
accessible: this.props.accessible !== false, accessible: this.props.accessible !== false,
accessibilityLabel: this.props.accessibilityLabel, accessibilityLabel: this.props.accessibilityLabel,
accessibilityComponentType: this.props.accessibilityComponentType, accessibilityComponentType: this.props.accessibilityComponentType,
accessibilityTraits: this.props.accessibilityTraits, accessibilityTraits: this.props.accessibilityTraits,
children,
testID: this.props.testID, testID: this.props.testID,
onLayout: this.props.onLayout, onLayout: this.props.onLayout,
hitSlop: this.props.hitSlop, hitSlop: this.props.hitSlop,
Expand Down
1 change: 1 addition & 0 deletions Libraries/Components/Touchable/TouchableOpacity.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ var TouchableOpacity = React.createClass({
onResponderRelease={this.touchableHandleResponderRelease} onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}> onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children} {this.props.children}
{Touchable.renderDebugView({color: 'cyan', hitSlop: this.props.hitSlop})}
</Animated.View> </Animated.View>
); );
}, },
Expand Down
43 changes: 31 additions & 12 deletions Libraries/Components/Touchable/TouchableWithoutFeedback.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -12,18 +12,19 @@
*/ */
'use strict'; 'use strict';


var EdgeInsetsPropType = require('EdgeInsetsPropType'); const EdgeInsetsPropType = require('EdgeInsetsPropType');
var React = require('React'); const React = require('React');
var TimerMixin = require('react-timer-mixin'); const TimerMixin = require('react-timer-mixin');
var Touchable = require('Touchable'); const Touchable = require('Touchable');
var View = require('View'); const View = require('View');
var ensurePositiveDelayProps = require('ensurePositiveDelayProps');
var invariant = require('fbjs/lib/invariant'); const ensurePositiveDelayProps = require('ensurePositiveDelayProps');
var onlyChild = require('onlyChild'); const onlyChild = require('onlyChild');
const warning = require('warning');


type Event = Object; type Event = Object;


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


/** /**
* Do not use unless you have a very good reason. All the elements that * Do not use unless you have a very good reason. All the elements that
Expand All @@ -34,7 +35,7 @@ var PRESS_RETENTION_OFFSET = {top: 20, left: 20, right: 20, bottom: 30};
* > * >
* > If you wish to have several child components, wrap them in a View. * > If you wish to have several child components, wrap them in a View.
*/ */
var TouchableWithoutFeedback = React.createClass({ const TouchableWithoutFeedback = React.createClass({
mixins: [TimerMixin, Touchable.Mixin], mixins: [TimerMixin, Touchable.Mixin],


propTypes: { propTypes: {
Expand Down Expand Up @@ -150,7 +151,23 @@ var TouchableWithoutFeedback = React.createClass({


render: function(): ReactElement { render: function(): ReactElement {
// Note(avik): remove dynamic typecast once Flow has been upgraded // Note(avik): remove dynamic typecast once Flow has been upgraded
return (React: any).cloneElement(onlyChild(this.props.children), { const child = onlyChild(this.props.children);
let children = child.props.children;
warning(
!child.type || child.type.displayName !== 'Text',
'TouchableWithoutFeedback does not work well with Text children. Wrap children in a View instead. See ' +
child._owner.getName()
);
if (Touchable.TOUCH_TARGET_DEBUG && child.type && child.type.displayName === 'View') {
if (!Array.isArray(children)) {
children = [children];
}
children.push(Touchable.renderDebugView({color: 'red', hitSlop: this.props.hitSlop}));
}
const style = (Touchable.TOUCH_TARGET_DEBUG && child.type && child.type.displayName === 'Text') ?
[child.props.style, {color: 'red'}] :
child.props.style;
return (React: any).cloneElement(child, {
accessible: this.props.accessible !== false, accessible: this.props.accessible !== false,
accessibilityLabel: this.props.accessibilityLabel, accessibilityLabel: this.props.accessibilityLabel,
accessibilityComponentType: this.props.accessibilityComponentType, accessibilityComponentType: this.props.accessibilityComponentType,
Expand All @@ -163,7 +180,9 @@ var TouchableWithoutFeedback = React.createClass({
onResponderGrant: this.touchableHandleResponderGrant, onResponderGrant: this.touchableHandleResponderGrant,
onResponderMove: this.touchableHandleResponderMove, onResponderMove: this.touchableHandleResponderMove,
onResponderRelease: this.touchableHandleResponderRelease, onResponderRelease: this.touchableHandleResponderRelease,
onResponderTerminate: this.touchableHandleResponderTerminate onResponderTerminate: this.touchableHandleResponderTerminate,
style,
children,
}); });
} }
}); });
Expand Down
5 changes: 5 additions & 0 deletions Libraries/Inspector/InspectorPanel.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var Text = require('Text');
var View = require('View'); var View = require('View');
var ElementProperties = require('ElementProperties'); var ElementProperties = require('ElementProperties');
var PerformanceOverlay = require('PerformanceOverlay'); var PerformanceOverlay = require('PerformanceOverlay');
var Touchable = require('Touchable');
var TouchableHighlight = require('TouchableHighlight'); var TouchableHighlight = require('TouchableHighlight');


var PropTypes = React.PropTypes; var PropTypes = React.PropTypes;
Expand Down Expand Up @@ -70,6 +71,9 @@ class InspectorPanel extends React.Component {
onClick={this.props.setPerfing} onClick={this.props.setPerfing}
/> />
</View> </View>
<Text style={styles.buttonText}>
{'Touchable.TOUCH_TARGET_DEBUG is ' + Touchable.TOUCH_TARGET_DEBUG}
</Text>
</View> </View>
); );
} }
Expand Down Expand Up @@ -126,6 +130,7 @@ var styles = StyleSheet.create({
fontSize: 20, fontSize: 20,
textAlign: 'center', textAlign: 'center',
marginVertical: 20, marginVertical: 20,
color: 'white',
}, },
}); });


Expand Down
9 changes: 7 additions & 2 deletions Libraries/Text/Text.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
const NativeMethodsMixin = require('NativeMethodsMixin'); const NativeMethodsMixin = require('NativeMethodsMixin');
const Platform = require('Platform'); const Platform = require('Platform');
const React = require('React'); const React = require('React');
const ReactInstanceMap = require('ReactInstanceMap');
const ReactNativeViewAttributes = require('ReactNativeViewAttributes'); const ReactNativeViewAttributes = require('ReactNativeViewAttributes');
const StyleSheetPropType = require('StyleSheetPropType'); const StyleSheetPropType = require('StyleSheetPropType');
const TextStylePropTypes = require('TextStylePropTypes'); const TextStylePropTypes = require('TextStylePropTypes');
Expand Down Expand Up @@ -149,7 +148,7 @@ const Text = React.createClass({
if (setResponder && !this.touchableHandleActivePressIn) { if (setResponder && !this.touchableHandleActivePressIn) {
// Attach and bind all the other handlers only the first time a touch // Attach and bind all the other handlers only the first time a touch
// actually happens. // actually happens.
for (let key in Touchable.Mixin) { for (const key in Touchable.Mixin) {
if (typeof Touchable.Mixin[key] === 'function') { if (typeof Touchable.Mixin[key] === 'function') {
(this: any)[key] = Touchable.Mixin[key].bind(this); (this: any)[key] = Touchable.Mixin[key].bind(this);
} }
Expand Down Expand Up @@ -219,6 +218,12 @@ const Text = React.createClass({
isHighlighted: this.state.isHighlighted, isHighlighted: this.state.isHighlighted,
}; };
} }
if (Touchable.TOUCH_TARGET_DEBUG && newProps.onPress) {
newProps = {
...newProps,
style: [this.props.style, {color: 'magenta'}],
};
}
if (this.context.isInAParentText) { if (this.context.isInAParentText) {
return <RCTVirtualText {...newProps} />; return <RCTVirtualText {...newProps} />;
} else { } else {
Expand Down

0 comments on commit 5c9b46c

Please sign in to comment.