Skip to content

Commit

Permalink
Reworking keyboardShouldPersistTaps to have a middle ground
Browse files Browse the repository at this point in the history
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 #10628

Differential Revision: D4294945

Pulled By: ericvicenti

fbshipit-source-id: 1a753014156cac1a23fabfa8e1faa9a768868ef2
  • Loading branch information
MaxLap authored and Martin Konicek committed Dec 12, 2016
1 parent ee92b5c commit 276485b
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 15 deletions.
2 changes: 1 addition & 1 deletion Examples/Movies/SearchScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ var SearchScreen = React.createClass({
onEndReached={this.onEndReached}
automaticallyAdjustContentInsets={false}
keyboardDismissMode="on-drag"
keyboardShouldPersistTaps={true}
keyboardShouldPersistTaps="handled"
showsVerticalScrollIndicator={false}
/>;

Expand Down
2 changes: 1 addition & 1 deletion Examples/UIExplorer/js/UIExplorerExampleList.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
/>
Expand Down
4 changes: 1 addition & 3 deletions Examples/UIExplorer/js/UIExplorerPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -55,7 +53,7 @@ class UIExplorerPage extends React.Component {
ContentWrapper = (ScrollView: ReactClass<any>);
// $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 ?
Expand Down
27 changes: 23 additions & 4 deletions Libraries/Components/ScrollResponder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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;
},

Expand All @@ -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;
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 12 additions & 6 deletions Libraries/Components/ScrollView/ScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 276485b

Please sign in to comment.