Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

feat: add keyboardHidesTabBarAnimationConfig props #195

Conversation

adrianha
Copy link
Contributor

@adrianha adrianha commented Oct 14, 2019

Motivation

Fixes #149
Add keyboardHidesTabBarAnimationConfig on TabBarOptions to modify keyboard animation

Test plan

Use this config as TabBarOptions:

tabBarOptions: {
  keyboardHidesTabBar: true,
  keyboardHidesTabBarAnimationConfig: {
    show: {
      animation: 'timing' | 'spring',
      config?: TimingKeyboardAnimationConfig | SpringKeyboardAnimationConfig
    },
    hide: {
      animation: 'timing' | 'spring',
      config?: TimingKeyboardAnimationConfig | SpringKeyboardAnimationConfig
    }
  }
}

@satya164
Copy link
Member

satya164 commented Oct 16, 2019

Thanks for the PR.

I'd rather have something like keyboardHidesTabBarAnimationConfig which provides a custom animation e.g. { animation: timing, config: { duration: 0 } } so user can also adjust the animation.

@adrianha
Copy link
Contributor Author

Noted. So i think there will be 2 configs for animations show and hide. What do you think @satya164 ?

@satya164
Copy link
Member

satya164 commented Oct 16, 2019

I think we can make it an object with show/hide properties. We could have default value for both and shallow merge.

@adrianha
Copy link
Contributor Author

Yeah, will update the PR

@adrianha
Copy link
Contributor Author

adrianha commented Oct 16, 2019

just updated the config, but its only support timing animation for now. Any advice how to define types if i want to add decay or spring animation? since the config is different, eg: decay need velocity and doesnt have duration as a config. cc @satya164

edit:
is it okay to add lodash dependency to use merge to merge the config?

@adrianha
Copy link
Contributor Author

after giving a thought, i guess timing is the only possible animation type for keyboard show/hide animation. Could we use decay or spring animation to handle this? i think custom duration should be enough. maybe something like this:

keyboardHidesTabBarAnimationConfig: {
  show: {
    duration: 150
  },
  hide: {
    duration: 100
  }
}

What do you think @satya164 ?

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I was on holidays. Yeah we should support other animations as well (for example spring is a valid animation), but in that case, duration isn't a valid config. So we'd need to check the type of animation and add default duration only for timing.

The type definitions also need to be updated.

package.json Outdated Show resolved Hide resolved
@adrianha
Copy link
Contributor Author

adrianha commented Oct 21, 2019

@satya164 Just updated the PR

  • Update type definitions; now support timing and spring animation
  • Remove lodash

@adrianha adrianha changed the title feat: add keyboardHidesTabBarAnimated props feat: add keyboardHidesTabBarAnimationConfig props Oct 23, 2019
@adrianha
Copy link
Contributor Author

Hi @satya164 , any suggestion or concern regarding the new updates?

@satya164
Copy link
Member

Thanks for the updates!

@satya164 satya164 merged commit 08a6268 into react-navigation:master Oct 31, 2019
@adrianha adrianha deleted the feat/add-keyboardHidesTabBarAnimated-config branch October 31, 2019 15:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keyboardHidesTabBar should instantly hide tab bar
3 participants