From ee8a7b48272b0bea3d77864d3552345f31a9d8b0 Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Wed, 13 Dec 2017 17:21:20 -0800 Subject: [PATCH] Some TouchableHighlight cleanup Reviewed By: TheSavior Differential Revision: D6494579 fbshipit-source-id: 02bbfc571e53f698cc943375800ad3bec4405495 --- .../Touchable/TouchableHighlight.js | 139 ++++++++---------- .../TouchableHighlight-test.js.snap | 3 +- Libraries/StyleSheet/StyleSheet.js | 14 ++ 3 files changed, 75 insertions(+), 81 deletions(-) diff --git a/Libraries/Components/Touchable/TouchableHighlight.js b/Libraries/Components/Touchable/TouchableHighlight.js index 133c13daa..173d229aa 100644 --- a/Libraries/Components/Touchable/TouchableHighlight.js +++ b/Libraries/Components/Touchable/TouchableHighlight.js @@ -8,6 +8,7 @@ * * @providesModule TouchableHighlight * @flow + * @format */ 'use strict'; @@ -17,28 +18,19 @@ const PropTypes = require('prop-types'); const React = require('React'); const ReactNativeViewAttributes = require('ReactNativeViewAttributes'); const StyleSheet = require('StyleSheet'); -/* $FlowFixMe(>=0.54.0 site=react_native_oss) This comment suppresses an error - * found when Flow v0.54 was deployed. To see the error delete this comment and - * run Flow. */ -const TimerMixin = require('react-timer-mixin'); const Touchable = require('Touchable'); const TouchableWithoutFeedback = require('TouchableWithoutFeedback'); const View = require('View'); const ViewPropTypes = require('ViewPropTypes'); const createReactClass = require('create-react-class'); -const ensureComponentIsNative = require('ensureComponentIsNative'); const ensurePositiveDelayProps = require('ensurePositiveDelayProps'); -/* $FlowFixMe(>=0.54.0 site=react_native_oss) This comment suppresses an error - * found when Flow v0.54 was deployed. To see the error delete this comment and - * run Flow. */ -const keyOf = require('fbjs/lib/keyOf'); -const merge = require('merge'); import type {Event} from 'TouchableWithoutFeedback'; const DEFAULT_PROPS = { activeOpacity: 0.85, + delayPressOut: 100, underlayColor: 'black', }; @@ -141,7 +133,7 @@ const PRESS_RETENTION_OFFSET = {top: 20, left: 20, right: 20, bottom: 30}; * */ -var TouchableHighlight = createReactClass({ +const TouchableHighlight = createReactClass({ displayName: 'TouchableHighlight', propTypes: { ...TouchableWithoutFeedback.propTypes, @@ -155,6 +147,10 @@ var TouchableHighlight = createReactClass({ * active. */ underlayColor: ColorPropType, + /** + * Style to apply to the container/underlay. Most commonly used to make sure + * rounded corners match the wrapped component. + */ style: ViewPropTypes.style, /** * Called immediately after the underlay is shown @@ -184,64 +180,36 @@ var TouchableHighlight = createReactClass({ tvParallaxProperties: PropTypes.object, }, - mixins: [NativeMethodsMixin, TimerMixin, Touchable.Mixin], + mixins: [NativeMethodsMixin, Touchable.Mixin], getDefaultProps: () => DEFAULT_PROPS, - // Performance optimization to avoid constantly re-generating these objects. - _computeSyntheticState: function(props) { - return { - activeProps: { - style: { - opacity: props.activeOpacity, - } - }, - activeUnderlayProps: { - style: { - backgroundColor: props.underlayColor, - } - }, - underlayStyle: [ - INACTIVE_UNDERLAY_PROPS.style, - props.style, - ], - hasTVPreferredFocus: props.hasTVPreferredFocus - }; - }, - getInitialState: function() { this._isMounted = false; - return merge( - this.touchableGetInitialState(), this._computeSyntheticState(this.props) - ); + return { + ...this.touchableGetInitialState(), + extraChildStyle: null, + extraUnderlayStyle: styles.inactiveUnderlay, + }; }, componentDidMount: function() { this._isMounted = true; ensurePositiveDelayProps(this.props); - ensureComponentIsNative(this.refs[CHILD_REF]); }, componentWillUnmount: function() { this._isMounted = false; - }, - - componentDidUpdate: function() { - ensureComponentIsNative(this.refs[CHILD_REF]); + clearTimeout(this._hideTimeout); }, componentWillReceiveProps: function(nextProps) { ensurePositiveDelayProps(nextProps); - if (nextProps.activeOpacity !== this.props.activeOpacity || - nextProps.underlayColor !== this.props.underlayColor || - nextProps.style !== this.props.style) { - this.setState(this._computeSyntheticState(nextProps)); - } }, viewConfig: { uiViewClassName: 'RCTView', - validAttributes: ReactNativeViewAttributes.RCTView + validAttributes: ReactNativeViewAttributes.RCTView, }, /** @@ -249,7 +217,7 @@ var TouchableHighlight = createReactClass({ * defined on your component. */ touchableHandleActivePressIn: function(e: Event) { - this.clearTimeout(this._hideTimeout); + clearTimeout(this._hideTimeout); this._hideTimeout = null; this._showUnderlay(); this.props.onPressIn && this.props.onPressIn(e); @@ -263,10 +231,12 @@ var TouchableHighlight = createReactClass({ }, touchableHandlePress: function(e: Event) { - this.clearTimeout(this._hideTimeout); + clearTimeout(this._hideTimeout); this._showUnderlay(); - this._hideTimeout = this.setTimeout(this._hideUnderlay, - this.props.delayPressOut || 100); + this._hideTimeout = setTimeout( + this._hideUnderlay, + this.props.delayPressOut, + ); this.props.onPress && this.props.onPress(e); }, @@ -298,20 +268,24 @@ var TouchableHighlight = createReactClass({ if (!this._isMounted || !this._hasPressHandler()) { return; } - - this.refs[UNDERLAY_REF].setNativeProps(this.state.activeUnderlayProps); - this.refs[CHILD_REF].setNativeProps(this.state.activeProps); + this.setState({ + extraChildStyle: { + opacity: this.props.activeOpacity, + }, + extraUnderlayStyle: { + backgroundColor: this.props.underlayColor, + }, + }); this.props.onShowUnderlay && this.props.onShowUnderlay(); }, _hideUnderlay: function() { - this.clearTimeout(this._hideTimeout); + clearTimeout(this._hideTimeout); this._hideTimeout = null; - if (this._hasPressHandler() && this.refs[UNDERLAY_REF]) { - this.refs[CHILD_REF].setNativeProps(INACTIVE_CHILD_PROPS); - this.refs[UNDERLAY_REF].setNativeProps({ - ...INACTIVE_UNDERLAY_PROPS, - style: this.state.underlayStyle, + if (this._hasPressHandler()) { + this.setState({ + extraChildStyle: null, + extraUnderlayStyle: styles.inactiveUnderlay, }); this.props.onHideUnderlay && this.props.onHideUnderlay(); } @@ -327,46 +301,51 @@ var TouchableHighlight = createReactClass({ }, render: function() { + const child = React.Children.only(this.props.children); return ( - {React.cloneElement( - React.Children.only(this.props.children), - { - ref: CHILD_REF, - } - )} - {Touchable.renderDebugView({color: 'green', hitSlop: this.props.hitSlop})} + {React.cloneElement(child, { + style: StyleSheet.compose( + child.props.style, + this.state.extraChildStyle, + ), + })} + {Touchable.renderDebugView({ + color: 'green', + hitSlop: this.props.hitSlop, + })} ); - } + }, }); -var CHILD_REF = keyOf({childRef: null}); -var UNDERLAY_REF = keyOf({underlayRef: null}); -var INACTIVE_CHILD_PROPS = { - style: StyleSheet.create({x: {opacity: 1.0}}).x, -}; -var INACTIVE_UNDERLAY_PROPS = { - style: StyleSheet.create({x: {backgroundColor: 'transparent'}}).x, -}; +const styles = StyleSheet.create({ + inactiveUnderlay: { + backgroundColor: 'transparent', + }, +}); module.exports = TouchableHighlight; diff --git a/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap b/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap index 2bd2f08e3..ffd03d18e 100644 --- a/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap +++ b/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap @@ -19,10 +19,10 @@ exports[`TouchableHighlight renders correctly 1`] = ` onStartShouldSetResponder={[Function]} style={ Array [ + Object {}, Object { "backgroundColor": "transparent", }, - Object {}, ] } testID={undefined} @@ -32,6 +32,7 @@ exports[`TouchableHighlight renders correctly 1`] = ` accessible={true} allowFontScaling={true} ellipsizeMode="tail" + style={null} > Touchable diff --git a/Libraries/StyleSheet/StyleSheet.js b/Libraries/StyleSheet/StyleSheet.js index ae845cab0..c32d1a9b6 100644 --- a/Libraries/StyleSheet/StyleSheet.js +++ b/Libraries/StyleSheet/StyleSheet.js @@ -134,6 +134,20 @@ module.exports = { */ absoluteFillObject, + /** + * Combines two styles such that `style2` will override any styles in `style1`. + * If either style is falsy, the other one is returned without allocating an + * array, saving allocations and maintaining reference equality for + * PureComponent checks. + */ + compose(style1: ?StyleProp, style2: ?StyleProp): ?StyleProp { + if (style1 && style2) { + return [style1, style2]; + } else { + return style1 || style2; + } + }, + /** * Flattens an array of style objects, into one aggregated style object. * Alternatively, this method can be used to lookup IDs, returned by