From 276485b0c0d1937622d764106ed8adea2bd1cb32 Mon Sep 17 00:00:00 2001 From: Maxime Lapointe Date: Wed, 7 Dec 2016 21:39:54 -0800 Subject: [PATCH] Reworking keyboardShouldPersistTaps to have a middle ground Summary: Right now, the ScrollView's keyboard hiding behavior is either all or nothing: Hide the keyboard on any tap, or do nothing ever. This PR introduces a third mode to keyboardShouldPersistTaps which is much closer to what I consider should be the default. In the new behavior, the tap responding is done in the bubbling phase (instead of the capture phase like =true). As a result, a child can handle the tap. If no child does, then the ScrollView will receive the tap and will hide the keyboard. As a result, changing TextInput focus works as a user expects, with a single tap and without keyboard hiding. But taping on Text or on the empty part of the ScrollView hides the keyboard and removes the focus. You can view the behavior in a monkey patched ScrollView demo on rnplay: https://rnplay.org/apps/E90UYw https://rnplay.org/apps/UGzhKA In order to have a uniform props set, i added 3 values to the keyboardShouldPersistTaps: 'never' and 'always' are the same as false and true. 'handled' is the new behavior. I don't Closes https://github.com/facebook/react-native/pull/10628 Differential Revision: D4294945 Pulled By: ericvicenti fbshipit-source-id: 1a753014156cac1a23fabfa8e1faa9a768868ef2 --- Examples/Movies/SearchScreen.js | 2 +- .../UIExplorer/js/UIExplorerExampleList.js | 2 +- Examples/UIExplorer/js/UIExplorerPage.js | 4 +-- Libraries/Components/ScrollResponder.js | 27 ++++++++++++++++--- Libraries/Components/ScrollView/ScrollView.js | 18 ++++++++----- 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/Examples/Movies/SearchScreen.js b/Examples/Movies/SearchScreen.js index 057336649b38a0..d45ddf1a29e20e 100644 --- a/Examples/Movies/SearchScreen.js +++ b/Examples/Movies/SearchScreen.js @@ -299,7 +299,7 @@ var SearchScreen = React.createClass({ onEndReached={this.onEndReached} automaticallyAdjustContentInsets={false} keyboardDismissMode="on-drag" - keyboardShouldPersistTaps={true} + keyboardShouldPersistTaps="handled" showsVerticalScrollIndicator={false} />; diff --git a/Examples/UIExplorer/js/UIExplorerExampleList.js b/Examples/UIExplorer/js/UIExplorerExampleList.js index 47772a63f7f345..b9af139c34aaa6 100644 --- a/Examples/UIExplorer/js/UIExplorerExampleList.js +++ b/Examples/UIExplorer/js/UIExplorerExampleList.js @@ -82,7 +82,7 @@ class UIExplorerExampleList extends React.Component { renderRow={this._renderExampleRow.bind(this)} renderSectionHeader={this._renderSectionHeader} enableEmptySections={true} - keyboardShouldPersistTaps={true} + keyboardShouldPersistTaps="handled" automaticallyAdjustContentInsets={false} keyboardDismissMode="on-drag" /> diff --git a/Examples/UIExplorer/js/UIExplorerPage.js b/Examples/UIExplorer/js/UIExplorerPage.js index c29e5bfb61e9c3..c2f75858b0d3bc 100644 --- a/Examples/UIExplorer/js/UIExplorerPage.js +++ b/Examples/UIExplorer/js/UIExplorerPage.js @@ -35,13 +35,11 @@ var UIExplorerTitle = require('./UIExplorerTitle'); class UIExplorerPage extends React.Component { props: { - keyboardShouldPersistTaps?: boolean, noScroll?: boolean, noSpacer?: boolean, }; static propTypes = { - keyboardShouldPersistTaps: React.PropTypes.bool, noScroll: React.PropTypes.bool, noSpacer: React.PropTypes.bool, }; @@ -55,7 +53,7 @@ class UIExplorerPage extends React.Component { ContentWrapper = (ScrollView: ReactClass); // $FlowFixMe found when converting React.createClass to ES6 wrapperProps.automaticallyAdjustContentInsets = !this.props.title; - wrapperProps.keyboardShouldPersistTaps = true; + wrapperProps.keyboardShouldPersistTaps = 'handled'; wrapperProps.keyboardDismissMode = 'interactive'; } var title = this.props.title ? diff --git a/Libraries/Components/ScrollResponder.js b/Libraries/Components/ScrollResponder.js index e3ead883791694..6b739ebebdaf8e 100644 --- a/Libraries/Components/ScrollResponder.js +++ b/Libraries/Components/ScrollResponder.js @@ -18,6 +18,7 @@ var ReactNative = require('ReactNative'); var Subscribable = require('Subscribable'); var TextInputState = require('TextInputState'); var UIManager = require('UIManager'); +var warning = require('fbjs/lib/warning'); var { getInstanceFromNode } = require('ReactNativeComponentTree'); var { ScrollViewManager } = require('NativeModules'); @@ -134,7 +135,7 @@ var ScrollResponderMixin = { // - Determine if the scroll view has been scrolled and therefore should // refuse to give up its responder lock. // - Determine if releasing should dismiss the keyboard when we are in - // tap-to-dismiss mode (!this.props.keyboardShouldPersistTaps). + // tap-to-dismiss mode (this.props.keyboardShouldPersistTaps !== 'always'). observedScrollSinceBecomingResponder: false, becameResponderWhileAnimating: false, }; @@ -172,7 +173,14 @@ var ScrollResponderMixin = { * true. * */ - scrollResponderHandleStartShouldSetResponder: function(): boolean { + scrollResponderHandleStartShouldSetResponder: function(e: Event): boolean { + var currentlyFocusedTextInput = TextInputState.currentlyFocusedField(); + + if (this.props.keyboardShouldPersistTaps === 'handled' && + currentlyFocusedTextInput != null && + e.target !== currentlyFocusedTextInput) { + return true; + } return false; }, @@ -190,7 +198,10 @@ var ScrollResponderMixin = { scrollResponderHandleStartShouldSetResponderCapture: function(e: Event): boolean { // First see if we want to eat taps while the keyboard is up var currentlyFocusedTextInput = TextInputState.currentlyFocusedField(); - if (!this.props.keyboardShouldPersistTaps && + var {keyboardShouldPersistTaps} = this.props; + var keyboardNeverPersistTaps = !keyboardShouldPersistTaps || + keyboardShouldPersistTaps === 'never'; + if (keyboardNeverPersistTaps && currentlyFocusedTextInput != null && !isTagInstanceOfTextInput(e.target)) { return true; @@ -250,7 +261,8 @@ var ScrollResponderMixin = { // By default scroll views will unfocus a textField // if another touch occurs outside of it var currentlyFocusedTextInput = TextInputState.currentlyFocusedField(); - if (!this.props.keyboardShouldPersistTaps && + if (this.props.keyboardShouldPersistTaps !== true && + this.props.keyboardShouldPersistTaps !== 'always' && currentlyFocusedTextInput != null && e.target !== currentlyFocusedTextInput && !this.state.observedScrollSinceBecomingResponder && @@ -481,6 +493,13 @@ var ScrollResponderMixin = { * The `keyboardWillShow` is called before input focus. */ componentWillMount: function() { + var {keyboardShouldPersistTaps} = this.props; + warning( + typeof keyboardShouldPersistTaps !== 'boolean', + `'keyboardShouldPersistTaps={${keyboardShouldPersistTaps}}' is deprecated. ` + + `Use 'keyboardShouldPersistTaps="${keyboardShouldPersistTaps ? "always" : "never"}"' instead` + ); + this.keyboardWillOpenTo = null; this.additionalScrollOffset = 0; this.addListenerOn(Keyboard, 'keyboardWillShow', this.scrollResponderKeyboardWillShow); diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index 3f39492a4fd735..a53ff84ec9f6b6 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -192,12 +192,18 @@ const ScrollView = React.createClass({ 'on-drag', ]), /** - * When false, tapping outside of the focused text input when the keyboard - * is up dismisses the keyboard. When true, the keyboard will not dismiss - * automatically, and the scroll view will not catch taps, but children of - * the scroll view can catch taps. The default value is false. - */ - keyboardShouldPersistTaps: PropTypes.bool, + * Determines when the keyboard should stay visible after a tap. + * + * - 'never' (the default), tapping outside of the focused text input when the keyboard + * is up dismisses the keyboard. When this happens, children won't receive the tap. + * - 'always', the keyboard will not dismiss automatically, and the scroll view will not + * catch taps, but children of the scroll view can catch taps. + * - 'handled', the keyboard will not dismiss automatically when the tap was handled by + * a children, (or captured by an ancestor). + * - false, deprecated, use 'never' instead + * - true, deprecated, use 'always' instead + */ + keyboardShouldPersistTaps: PropTypes.oneOf(['always', 'never', 'handled', false, true]), /** * The maximum allowed zoom scale. The default value is 1.0. * @platform ios