From 5c9b46c15e62d269d8eb6e9b3891f8626c51f1e5 Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Thu, 14 Apr 2016 14:27:35 -0700 Subject: [PATCH] Improve touchable debugging 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 --- Libraries/Components/Touchable/Touchable.js | 56 +++++++++++++++++-- .../Components/Touchable/TouchableBounce.js | 1 + .../Touchable/TouchableHighlight.js | 1 + .../TouchableNativeFeedback.android.js | 12 +++- .../Components/Touchable/TouchableOpacity.js | 1 + .../Touchable/TouchableWithoutFeedback.js | 43 ++++++++++---- Libraries/Inspector/InspectorPanel.js | 5 ++ Libraries/Text/Text.js | 9 ++- 8 files changed, 106 insertions(+), 22 deletions(-) diff --git a/Libraries/Components/Touchable/Touchable.js b/Libraries/Components/Touchable/Touchable.js index 55be8e0dc..6bf9b976f 100644 --- a/Libraries/Components/Touchable/Touchable.js +++ b/Libraries/Components/Touchable/Touchable.js @@ -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 */ 'use strict'; -var BoundingDimensions = require('BoundingDimensions'); -var Position = require('Position'); -var TouchEventUtils = require('fbjs/lib/TouchEventUtils'); +const BoundingDimensions = require('BoundingDimensions'); +const Position = require('Position'); +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'); -var queryLayoutByID = require('queryLayoutByID'); +const keyMirror = require('fbjs/lib/keyMirror'); +const normalizeColor = require('normalizeColor'); +const queryLayoutByID = require('queryLayoutByID'); /** * `Touchable`: Taps done right. @@ -713,7 +723,41 @@ var TouchableMixin = { }; 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 ( + + ); + } }; +if (Touchable.TOUCH_TARGET_DEBUG) { + console.warn('Touchable.TOUCH_TARGET_DEBUG is enabled'); +} module.exports = Touchable; diff --git a/Libraries/Components/Touchable/TouchableBounce.js b/Libraries/Components/Touchable/TouchableBounce.js index 5ea367ceb..c06cbd7d9 100644 --- a/Libraries/Components/Touchable/TouchableBounce.js +++ b/Libraries/Components/Touchable/TouchableBounce.js @@ -142,6 +142,7 @@ var TouchableBounce = React.createClass({ onResponderRelease={this.touchableHandleResponderRelease} onResponderTerminate={this.touchableHandleResponderTerminate}> {this.props.children} + {Touchable.renderDebugView({color: 'orange', hitSlop: this.props.hitSlop})} ); } diff --git a/Libraries/Components/Touchable/TouchableHighlight.js b/Libraries/Components/Touchable/TouchableHighlight.js index 63314e408..2e7f3ffbd 100644 --- a/Libraries/Components/Touchable/TouchableHighlight.js +++ b/Libraries/Components/Touchable/TouchableHighlight.js @@ -248,6 +248,7 @@ var TouchableHighlight = React.createClass({ ref: CHILD_REF, } )} + {Touchable.renderDebugView({color: 'green', hitSlop: this.props.hitSlop})} ); } diff --git a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js index 17a70652c..4fe522732 100644 --- a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js +++ b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js @@ -13,7 +13,6 @@ var PropTypes = require('ReactPropTypes'); var React = require('React'); var ReactNative = require('ReactNative'); -var ReactNativeViewAttributes = require('ReactNativeViewAttributes'); var Touchable = require('Touchable'); var TouchableWithoutFeedback = require('TouchableWithoutFeedback'); var UIManager = require('UIManager'); @@ -206,13 +205,22 @@ var TouchableNativeFeedback = React.createClass({ }, 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 = { - ...onlyChild(this.props.children).props, + ...child.props, nativeBackgroundAndroid: this.props.background, accessible: this.props.accessible !== false, accessibilityLabel: this.props.accessibilityLabel, accessibilityComponentType: this.props.accessibilityComponentType, accessibilityTraits: this.props.accessibilityTraits, + children, testID: this.props.testID, onLayout: this.props.onLayout, hitSlop: this.props.hitSlop, diff --git a/Libraries/Components/Touchable/TouchableOpacity.js b/Libraries/Components/Touchable/TouchableOpacity.js index f6ac619f4..52e0579e4 100644 --- a/Libraries/Components/Touchable/TouchableOpacity.js +++ b/Libraries/Components/Touchable/TouchableOpacity.js @@ -175,6 +175,7 @@ var TouchableOpacity = React.createClass({ onResponderRelease={this.touchableHandleResponderRelease} onResponderTerminate={this.touchableHandleResponderTerminate}> {this.props.children} + {Touchable.renderDebugView({color: 'cyan', hitSlop: this.props.hitSlop})} ); }, diff --git a/Libraries/Components/Touchable/TouchableWithoutFeedback.js b/Libraries/Components/Touchable/TouchableWithoutFeedback.js index d48e15856..42c01e476 100755 --- a/Libraries/Components/Touchable/TouchableWithoutFeedback.js +++ b/Libraries/Components/Touchable/TouchableWithoutFeedback.js @@ -12,18 +12,19 @@ */ 'use strict'; -var EdgeInsetsPropType = require('EdgeInsetsPropType'); -var React = require('React'); -var TimerMixin = require('react-timer-mixin'); -var Touchable = require('Touchable'); -var View = require('View'); -var ensurePositiveDelayProps = require('ensurePositiveDelayProps'); -var invariant = require('fbjs/lib/invariant'); -var onlyChild = require('onlyChild'); +const EdgeInsetsPropType = require('EdgeInsetsPropType'); +const React = require('React'); +const TimerMixin = require('react-timer-mixin'); +const Touchable = require('Touchable'); +const View = require('View'); + +const ensurePositiveDelayProps = require('ensurePositiveDelayProps'); +const onlyChild = require('onlyChild'); +const warning = require('warning'); 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 @@ -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. */ -var TouchableWithoutFeedback = React.createClass({ +const TouchableWithoutFeedback = React.createClass({ mixins: [TimerMixin, Touchable.Mixin], propTypes: { @@ -150,7 +151,23 @@ var TouchableWithoutFeedback = React.createClass({ render: function(): ReactElement { // 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, accessibilityLabel: this.props.accessibilityLabel, accessibilityComponentType: this.props.accessibilityComponentType, @@ -163,7 +180,9 @@ var TouchableWithoutFeedback = React.createClass({ onResponderGrant: this.touchableHandleResponderGrant, onResponderMove: this.touchableHandleResponderMove, onResponderRelease: this.touchableHandleResponderRelease, - onResponderTerminate: this.touchableHandleResponderTerminate + onResponderTerminate: this.touchableHandleResponderTerminate, + style, + children, }); } }); diff --git a/Libraries/Inspector/InspectorPanel.js b/Libraries/Inspector/InspectorPanel.js index b33d973b4..2009aa213 100644 --- a/Libraries/Inspector/InspectorPanel.js +++ b/Libraries/Inspector/InspectorPanel.js @@ -17,6 +17,7 @@ var Text = require('Text'); var View = require('View'); var ElementProperties = require('ElementProperties'); var PerformanceOverlay = require('PerformanceOverlay'); +var Touchable = require('Touchable'); var TouchableHighlight = require('TouchableHighlight'); var PropTypes = React.PropTypes; @@ -70,6 +71,9 @@ class InspectorPanel extends React.Component { onClick={this.props.setPerfing} /> + + {'Touchable.TOUCH_TARGET_DEBUG is ' + Touchable.TOUCH_TARGET_DEBUG} + ); } @@ -126,6 +130,7 @@ var styles = StyleSheet.create({ fontSize: 20, textAlign: 'center', marginVertical: 20, + color: 'white', }, }); diff --git a/Libraries/Text/Text.js b/Libraries/Text/Text.js index eeb88c2dd..d4914b60d 100644 --- a/Libraries/Text/Text.js +++ b/Libraries/Text/Text.js @@ -14,7 +14,6 @@ const NativeMethodsMixin = require('NativeMethodsMixin'); const Platform = require('Platform'); const React = require('React'); -const ReactInstanceMap = require('ReactInstanceMap'); const ReactNativeViewAttributes = require('ReactNativeViewAttributes'); const StyleSheetPropType = require('StyleSheetPropType'); const TextStylePropTypes = require('TextStylePropTypes'); @@ -149,7 +148,7 @@ const Text = React.createClass({ if (setResponder && !this.touchableHandleActivePressIn) { // Attach and bind all the other handlers only the first time a touch // actually happens. - for (let key in Touchable.Mixin) { + for (const key in Touchable.Mixin) { if (typeof Touchable.Mixin[key] === 'function') { (this: any)[key] = Touchable.Mixin[key].bind(this); } @@ -219,6 +218,12 @@ const Text = React.createClass({ isHighlighted: this.state.isHighlighted, }; } + if (Touchable.TOUCH_TARGET_DEBUG && newProps.onPress) { + newProps = { + ...newProps, + style: [this.props.style, {color: 'magenta'}], + }; + } if (this.context.isInAParentText) { return ; } else {