[Tabs] fix TabBarViewControllerExample to respect safe area #4949
Conversation
| TBVCSampleViewController *vc = | ||
| [TBVCSampleViewController sampleWithTitle:@"Push&Hide" color:UIColor.grayColor]; | ||
| vc.colorScheme = self.colorScheme; | ||
| vc.typographyScheme = self.typographyScheme; |
There was a problem hiding this comment.
Please use strongSelf here and above.
There was a problem hiding this comment.
Good catch! Addressed in a46864d.
| action:@selector(toggleTabBar) | ||
| forControlEvents:UIControlEventTouchUpInside]; | ||
|
|
||
| self.selectedViewController = [self.viewControllers firstObject]; |
There was a problem hiding this comment.
Nit: Please use dot notation for property access.
self.selectedViewController = self.viewControllers.firstObject;There was a problem hiding this comment.
Good catch. Addressed in a46864d.
| - (void)addMDCButtonWithFrame:(CGRect)frame | ||
| buttonScheme:(id<MDCButtonScheming>)buttonScheme | ||
| title:(NSString *)title | ||
| actionBlock:(MDCButtonActionBlock)actionBlock; |
There was a problem hiding this comment.
Please add nullability specifiers for these arguments.
There was a problem hiding this comment.
Addressed in a46864d.
| @property(nonatomic) MDCAppBarViewController *appBarViewController; | ||
| @property(nonatomic) UILabel *titleLabel; | ||
| @property(nonatomic) CGRect buttonFrame; |
There was a problem hiding this comment.
Please document that this is the "target" frame desired by the calling code. Reading the property, I thought it was going to be just a copy of self.button.frame.
There was a problem hiding this comment.
Good point! Addressed in a46864d.
| @property(nonatomic) MDCAppBarViewController *appBarViewController; | ||
| @property(nonatomic) UILabel *titleLabel; | ||
| @property(nonatomic) CGRect buttonFrame; | ||
| @property(nonatomic) UIButton *button; |
There was a problem hiding this comment.
Please make this MDCButton * since we cannot apply an <MDCButtonScheming> to a UIButton.
There was a problem hiding this comment.
Addressed in a46864d.
| @@ -78,6 +84,17 @@ - (void)viewWillAppear:(BOOL)animated { | |||
| - (void)viewDidLayoutSubviews { | |||
| [super viewDidLayoutSubviews]; | |||
| _titleLabel.center = self.view.center; | |||
| UIEdgeInsets edgeInsets = UIEdgeInsetsZero; | |||
There was a problem hiding this comment.
Nit/nameing: safeAreaInsets
There was a problem hiding this comment.
Addressed in a46864d.
| edgeInsets = self.view.safeAreaInsets; | ||
| } | ||
| CGRect buttonFrame = self.buttonFrame; | ||
| self.button.frame = CGRectMake(buttonFrame.origin.x + edgeInsets.left, |
There was a problem hiding this comment.
Please consider using CGRectOffset, since I believe that's what you're going for here.
There was a problem hiding this comment.
Addressed in a46864d.
| [MDCContainedButtonThemer applyScheme:buttonScheme toButton:button]; | ||
| [self.view addSubview:button]; | ||
| self.button = button; | ||
| self.buttonFrame = frame; |
There was a problem hiding this comment.
Please consider calling CGRectStandardize before assigning the property.
There was a problem hiding this comment.
Addressed in a46864d.
|
|
||
| + (nonnull instancetype)sampleWithTitle:(nonnull NSString *)title color:(nonnull UIColor *)color; | ||
|
|
||
| - (void)addMDCButtonWithFrame:(CGRect)frame |
There was a problem hiding this comment.
Naming/nit: Please make this setMDCButtonWithFrame:... since there can only be one. :highlander:
There was a problem hiding this comment.
Addressed in a46864d.
| title:@"Toggle Tab Bar" | ||
| actionBlock:^{ | ||
| TabBarViewControllerExample *strongSelf = weakSelf; | ||
| [strongSelf setTabBarHidden:!strongSelf.tabBarHidden animated:YES]; |
There was a problem hiding this comment.
Since this is a property, please use the dot syntax.
strongSelf.tabBarHidden = !self.isTabBarHidden;There was a problem hiding this comment.
I kept the original behavior since it wanted to animate out the tab bar. Would it be okay if I change the behavior?
closes #3728
Before
After: