New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StackNavigator keyboard aware #3951

Merged
merged 4 commits into from Apr 20, 2018

Conversation

Projects
None yet
6 participants
@skevy
Member

skevy commented Apr 11, 2018

One thing that has always annoyed me in React Navigation is the handling of the keyboard. When a keyboard is visible on screen and a navigation action occurs (either by tapping a button or using a gesture), the keyboard tends to stay on screen until the transition completes. This feels janky and broken. On native iOS, for instance, the keyboard hides immediately when the navigation starts, and if the transition is cancelled (say, when the user releases the gesture), the keyboard reappears.

This PR introduces a "KeyboardAwareNavigator" higher order component that is enabled on the StackNavigator, unless a disableKeyboardHandling prop is passed into the StackNavigator's configuration.

@brentvatne @ericvicenti please let me know if you'd like a different API for this (aka whether or not this behavior should be available by default, or if you'd like a different name for the disableKeyboardHandling prop). Happy to change whatever, or you can feel free to edit this PR yourself if landing it is time sensitive.

Hope this is helpful!

@react-navigation-ci

This comment has been minimized.

@skevy skevy requested a review from brentvatne Apr 11, 2018

@react-navigation-ci

This comment has been minimized.

_handleGestureBegin = () => {
this._previouslyFocusedTextInput =
TextInput.State.currentlyFocusedField() || null;

This comment has been minimized.

@vonovak

vonovak Apr 11, 2018

Collaborator

minor thing: || null does not seem necessary

This comment has been minimized.

@skevy

skevy Apr 12, 2018

Member

Hmm yah why did I do this? lol will fix

@skevy skevy force-pushed the @skevy/master.make-stacknavigator-keyboard branch from dcb61af to 252b554 Apr 12, 2018

@react-navigation-ci

This comment has been minimized.

const currentField = TextInput.State.currentlyFocusedField();
if (currentField) {
TextInput.State.blurTextInput(currentField);
}

This comment has been minimized.

@ericvicenti

ericvicenti Apr 16, 2018

Contributor

We should call this.props.onTransitionStart here, to allow the user to continue to use the handlers. Same with the other events.

@ericvicenti

This is looking great!

The new handlers to StackView also look handy.

skevy and others added some commits Apr 11, 2018

Make StackNavigator keyboard aware
One thing that has always annoyed me in React Navigation is the handling of the keyboard. When a keyboard is visible on screen and a navigation action occurs (either by tapping a button or using a gesture), the keyboard tends to stay on screen until the transition completes. This feels janky and broken. On native iOS, for instance, the keyboard hides immediately when the navigation starts, and if the transition is cancelled (say, when the user releases the gesture), the keyboard reappears.

This PR introduces a "KeyboardAwareNavigator" higher order component that is enabled on the StackNavigator, unless a `disableKeyboardHandling` prop is passed into the StackNavigator's configuration.

@brentvatne brentvatne force-pushed the @skevy/master.make-stacknavigator-keyboard branch from 252b554 to 7b34b95 Apr 20, 2018

@react-navigation-ci

This comment has been minimized.

@react-navigation-ci

This comment has been minimized.

@brentvatne brentvatne merged commit 0890896 into master Apr 20, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@skevy

This comment has been minimized.

Member

skevy commented Apr 22, 2018

haha sorry @brentvatne for making you fix @ericvicenti's comment! I don't check my Github notifications nearly enough :)

@angly-cat

This comment has been minimized.

Contributor

angly-cat commented May 5, 2018

Hi!

This is a nice PR.

I wonder if SwitchNavigator and TabNavigator should be keyboard-aware by default as well?

Also it'll be great if this keyboard-awareness would be described in docs.

@brentvatne

This comment has been minimized.

Member

brentvatne commented May 5, 2018

@angly-cat - please create a feature request for that if you want it. and an issue on the docs repo. comment on closed stuff is not the right place ;)

@react-navigation react-navigation locked and limited conversation to collaborators May 5, 2018

@brentvatne brentvatne deleted the @skevy/master.make-stacknavigator-keyboard branch May 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.