[V2][iOS] Fix bottomTabs’s animate option #4465
Conversation
Signed-off-by: wilson <wilson@waveo.com>
|
Can you add example GIF showing this feature? |
|
@henrikra added |
|
Nice! And can you also share what configuration you passed from JavaScript? |
|
aligned with the doc Navigation.mergeOptions(componentId, {
bottomTabs: { visible: false, animate: true },
}); |
|
Ahaa so you are listening the scroll event in your code and when you are going down you will call this? Navigation.mergeOptions(componentId, {
bottomTabs: { visible: false, animate: true },
}); |
|
yup :) |
|
You have to fix tests before this gets merged though :) |
|
can you give me some guidance? |
lib/ios/RNNTabBarPresenter.m
Outdated
| @@ -16,7 +16,7 @@ - (void)applyOptions:(RNNNavigationOptions *)options { | |||
| [tabBarController rnn_setTabBarTranslucent:[options.bottomTabs.translucent getWithDefaultValue:NO]]; | |||
| [tabBarController rnn_setTabBarHideShadow:[options.bottomTabs.hideShadow getWithDefaultValue:NO]]; | |||
| [tabBarController rnn_setTabBarStyle:[RCTConvert UIBarStyle:[options.bottomTabs.barStyle getWithDefaultValue:@"default"]]]; | |||
| [tabBarController rnn_setTabBarVisible:[options.bottomTabs.visible getWithDefaultValue:YES]]; | |||
| [tabBarController rnn_setTabBarVisible:[options.bottomTabs.visible getWithDefaultValue:YES] animated:[options.bottomTabs.animate getWithDefaultValue:YES]]; | |||
There was a problem hiding this comment.
This is a breaking change because it does not animate in existing implementations.
I think that it is better to default to NO.
Signed-off-by: wilson <wilson@waveo.com>
Signed-off-by: wilson <wilson@waveo.com>
lib/ios/RNNTabBarPresenter.m
Outdated
| @@ -54,8 +54,8 @@ - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavig | |||
| [tabBarController rnn_setTabBarHideShadow:newOptions.bottomTabs.hideShadow.get]; | |||
| } | |||
|
|
|||
| if (newOptions.bottomTabs.visible.hasValue) { | |||
| [tabBarController rnn_setTabBarVisible:newOptions.bottomTabs.visible.get]; | |||
| if (newOptions.bottomTabs.visible.hasValue && newOptions.bottomTabs.animate.hasValue) { | |||
There was a problem hiding this comment.
If visible has a value and animate doesn't, rnn_setTabBarVisible won't be called - This is a breaking change/bug.
I suggest we handle visibility change it two different ways:
- Keep current behaviour as is - Visible has value and animate doesn't
- Both visible and animate have values
Once you make this change we can move forward and approve this PR. Thanks
Signed-off-by: wilson <wilson@waveo.com>
|
This would be really useful |
|
@yogevbd Any status on merging this ? |
|
Doesn’t look like there’s any reason not to merge this? |
|
hi @yogevbd , is there anything I need to change or fix? thanks |
* [V2][iOS] Fix bottomTabs’s animate option Signed-off-by: wilson <wilson@waveo.com> * change animated default value to false Signed-off-by: wilson <wilson@waveo.com> * fix self.tabBar.hidden value Signed-off-by: wilson <wilson@waveo.com> * Fix tests Signed-off-by: wilson <wilson@waveo.com> * fix value check Signed-off-by: wilson <wilson@waveo.com>
Resolve #4385