Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-23684] iOS: Expose properties to hide the navbar #8160

Merged
merged 10 commits into from
Jul 27, 2016
Merged

[TIMOB-23684] iOS: Expose properties to hide the navbar #8160

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e88eb02
Added properties to hide navbar
Jul 26, 2016
5e7c827
Ensure type
Jul 27, 2016
6a9eaf6
Update per property
Jul 27, 2016
1f91020
Update per property - callers
Jul 27, 2016
2c29ee6
Only call setters
Jul 27, 2016
0a35071
Added documentations
Jul 27, 2016
272d318
Added osver since
Jul 27, 2016
b36132c
Multiline in apidco
Jul 27, 2016
d39fded
Remove duplicate full-stop
Jul 27, 2016
bd69f67
min version iOS in apidoc
Jul 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions iphone/Classes/TiUIWindowProxy.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ - (void)viewWillAppear:(BOOL)animated; // Called when the view is about to ma
- (void)viewDidAppear:(BOOL)animated; // Called when the view has been fully transitioned onto the screen. Default does nothing
{
[self updateTitleView];
[self updateHidesBars];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

I think someone broke the indentation with tabs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

tl;dr:
It's a legacy part of the code, where tabs have been used.

The way to go here is to use the pattern that is already there. All new sources have spaces by default, but some old one have tabs. The only reason we don't run a script to fix it to spaces everywhere is, to not loose the ability to git-blame the source, because it sometimes can be very important to see why a certain change was made back in the days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so tab ;)

[super viewDidAppear:animated];
}

Expand Down Expand Up @@ -920,6 +921,45 @@ -(void)setToolbar:(id)items withObject:(id)properties

}

-(void)setHidesBarsOnSwipe:(id)value
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We need validation, please add

ENSURE_TYPE(value, NSNumber);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhm, it's a bool!

Copy link
Collaborator

Choose a reason for hiding this comment

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

They come to the proxies as NSNumbers (because of the JavaScriptCore below it). So usually this is what you do in boolean-setters:

- (void)setHidesBarsOnSwipe:(id)value
{
    ENSURE_TYPE(value, NSNumber);

    [myView setWhatever:[TiUtils boolValue:value def:YES]];
    [self replaceValue:value forKey:@"hidesBarsOnSwipe" notification:NO];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect! Thank you!

[self replaceValue:value forKey:@"hidesBarsOnSwipe" notification:NO];
[self updateHidesBars];
}

-(void)setHidesBarsOnTap:(id)value
{
[self replaceValue:value forKey:@"hidesBarsOnTap" notification:NO];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, add it for all please

[self updateHidesBars];
}

-(void)setHidesBarsWhenVerticallyCompact:(id)value
{
[self replaceValue:value forKey:@"hidesBarsWhenVerticallyCompact" notification:NO];
[self updateHidesBars];
}

-(void)setHidesBarsWhenKeyboardAppears:(id)value
{
[self replaceValue:value forKey:@"hidesBarsWhenKeyboardAppears" notification:NO];
[self updateHidesBars];
}

-(void)updateHidesBars
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we call all of them together as oppose as each to their own setter ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a better code organization, nope?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's more about performance. Imagine, you use all provided properties. Then 4 setters will be called and 16 native calls will be made, since it calls the [self updateHideBars] every time. The only reason this way would make sense is, when one property depends on another one and they need to be updated as soon as another one is. If that is not the case, updating it in the setter is the way to go.

If they require main-thread, use ENSURE_MAIN_THREAD(methodName, argName) to ensure that the method runs on the main-thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I keep TiThreadPerformOnMainThread ?

{
if ([TiUtils isIOS8OrGreater]) {
TiThreadPerformOnMainThread(^{
if ((controller != nil) && ([controller navigationController] != nil)) {
UINavigationController *ourNC = [controller navigationController];
ourNC.hidesBarsOnSwipe = [TiUtils boolValue:[self valueForUndefinedKey:@"hidesBarsOnSwipe"] def:NO];
ourNC.hidesBarsOnTap = [TiUtils boolValue:[self valueForUndefinedKey:@"hidesBarsOnTap"] def:NO];
ourNC.hidesBarsWhenVerticallyCompact = [TiUtils boolValue:[self valueForUndefinedKey:@"hidesBarsWhenVerticallyCompact"] def:NO];
ourNC.hidesBarsWhenKeyboardAppears = [TiUtils boolValue:[self valueForUndefinedKey:@"hidesBarsWhenKeyboardAppears"] def:NO];
}
}, NO);
}
}


#define SETPROP(m,x) \
{\
Expand Down Expand Up @@ -970,6 +1010,10 @@ -(void)setupWindowDecorations
SETPROP(@"navTintColor",setNavTintColor);
SETPROP(@"translucent",setTranslucent);
SETPROP(@"tabBarHidden",setTabBarHidden);
SETPROP(@"hidesBarsOnSwipe",setHidesBarsOnSwipe);
SETPROP(@"hidesBarsOnTap", setHidesBarsOnTap);
SETPROP(@"hidesBarsWhenVerticallyCompact",setHidesBarsWhenVerticallyCompact);
SETPROP(@"hidesBarsWhenKeyboardAppears", setHidesBarsWhenKeyboardAppears);
SETPROPOBJ(@"toolbar",setToolbar);
[self updateBarImage];
[self updateNavButtons];
Expand Down