-
-
Notifications
You must be signed in to change notification settings - Fork 598
feat(iOS): add support for using native behavior in hideNavigationBar, obscureBackground props in SearchBar
#3211
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
Conversation
| - (void)setObscureBackground:(RNSOptionalBoolean)obscureBackground | ||
| { | ||
| [_controller setObscuresBackgroundDuringPresentation:obscureBackground]; | ||
| switch (obscureBackground) { | ||
| case RNSOptionalBooleanTrue: | ||
| [_controller setObscuresBackgroundDuringPresentation:YES]; | ||
| _isObscureBackgroundSet = YES; | ||
| break; | ||
|
|
||
| case RNSOptionalBooleanFalse: | ||
| [_controller setObscuresBackgroundDuringPresentation:NO]; | ||
| _isObscureBackgroundSet = YES; | ||
| break; | ||
|
|
||
| default: | ||
| if (_isObscureBackgroundSet) { | ||
| RCTLogWarn(@"[RNScreens] Dynamically restoring obscureBackground to default native behavior is unsupported."); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| - (void)setHideNavigationBar:(BOOL)hideNavigationBar | ||
| - (void)setHideNavigationBar:(RNSOptionalBoolean)hideNavigationBar | ||
| { | ||
| [_controller setHidesNavigationBarDuringPresentation:hideNavigationBar]; | ||
| switch (hideNavigationBar) { | ||
| case RNSOptionalBooleanTrue: | ||
| [_controller setHidesNavigationBarDuringPresentation:YES]; | ||
| _isHideNavigationBarSet = YES; | ||
| break; | ||
|
|
||
| case RNSOptionalBooleanFalse: | ||
| [_controller setHidesNavigationBarDuringPresentation:NO]; | ||
| _isHideNavigationBarSet = YES; | ||
| break; | ||
|
|
||
| default: | ||
| if (_isHideNavigationBarSet) { | ||
| RCTLogWarn(@"[RNScreens] Dynamically restoring hideNavigationBar to default native behavior is unsupported."); | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be relatively easy to refactor out to some helper function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed internally, we decided to leave it as it is for now to maintain readability. If we use RNSOptionalBoolean more in the future, we can consider creating a macro.
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside the fact, that we could maybe refactor out some parts in RNSSearchBar implementation, this PR looks really good. Thank you! Let's create follow-up for react-navigation.
Description
Follow-up to #3199.
Adds support for using native behavior in
hideNavigationBar,obscureBackgroundprops in SearchBar.This will require changes in
react-navigationdocs - ticket for this: https://github.com/software-mansion/react-native-screens-labs/issues/384.Values table
obscureBackgroundJS prop (initial)undefinedRNSOptionalBooleanUndefinedtrueRNSOptionalBooleanTruetruefalseRNSOptionalBooleanFalsefalseChanges from
true/falsetoundefinedin runtime are unsupported.Native behavior
obscureBackgroundhideNavigationBarCloses https://github.com/software-mansion/react-native-screens-labs/issues/415.
Changes
obscureBackgroundandhideNavigationBarboolean | undefinedto'undefined' | 'true' | 'false'RNSOptionalBooleanenum and conversions for Paper and FabricundefinedTest code and steps to reproduce
Run
Test758. You can comment outobscureBackground,hideNavigationBarprops before running the test.Checklist