Commit 65fd3130 authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

[ios] Fix Tabgrid UI imperfections

- Moves GridViewCntroller inset adjustments in -viewDidAppear and -willTransitionToSize to -viewDidlayoutSubviews. The tradeoffs is that -viewDidLayoutSubviews is called many more times than the other two, hence the original choice to stay away from it. However, the current approach gives a glitchy-like UI experience when rotating.
- Updates Remote Tabs insets with horizontal safe area of scroll view if not on the screen. Doesn't update insets if visible since it will then have the right safe area already. This is still a hack, but a more comprehensive coverage so that it never ends up encroaching the safe area.

Video: https://drive.google.com/open?id=1jzP0ClCOn0uQTZC9hJ2g0FUWsF-nSuqV

Bug: 868270, 868285, 868658
Change-Id: Ia50146e1be9dfdde74bbf3e7d6b6042168b2ce8a
Reviewed-on: https://chromium-review.googlesource.com/1153391
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579618}
parent 8f606faf
......@@ -305,14 +305,6 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
return NO;
}
#pragma mark - UIScrollViewDelegate
- (void)scrollViewDidChangeAdjustedContentInset:(UIScrollView*)scrollView {
// Adjust Content Inset changed. Force a re-layout of the CollectionView to
// visualize changes.
[self.collectionView.collectionViewLayout invalidateLayout];
}
#pragma mark - GridCellDelegate
- (void)closeButtonTappedForCell:(GridCell*)cell {
......
......@@ -180,8 +180,14 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
- (void)viewDidAppear:(BOOL)animated {
[super viewDidAppear:animated];
self.initialFrame = self.view.frame;
[self modifyChildViewControllerInsetsAndScrollViewOffets];
[self updatePageViewAccessibilityVisibility];
// Modify Remote Tabs Insets when page appears and during rotation.
[self setInsetForRemoteTabs];
}
- (void)viewDidLayoutSubviews {
[super viewDidLayoutSubviews];
// Modify Incognito and Regular Tabs Insets
[self setInsetForGridViews];
}
- (void)viewWillDisappear:(BOOL)animated {
......@@ -210,7 +216,8 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
};
auto completion =
^(id<UIViewControllerTransitionCoordinatorContext> context) {
[self modifyChildViewControllerInsetsAndScrollViewOffets];
// Modify Remote Tabs Insets when page appears and during rotation.
[self setInsetForRemoteTabs];
};
[coordinator animateAlongsideTransition:animate completion:completion];
}
......@@ -357,9 +364,9 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
#pragma mark - Private
// Updates the scroll view's offset by calling setter of _currentPage. Updates
// the insets for the Grid ViewControllers and Remote Tabs.
- (void)modifyChildViewControllerInsetsAndScrollViewOffets {
// Sets the proper insets for the Remote Tabs ViewController to accomodate for
// the safe area, toolbar, and status bar.
- (void)setInsetForRemoteTabs {
// Call the current page setter to sync the scroll view offset to the current
// page value, if the scroll view isn't scrolling. Don't animate this.
if (!self.scrollView.dragging && !self.scrollView.decelerating) {
......@@ -370,26 +377,21 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
CGFloat bottomInset = self.configuration == TabGridConfigurationBottomToolbar
? self.bottomToolbar.intrinsicContentSize.height
: 0;
UIEdgeInsets contentInset = UIEdgeInsetsMake(
UIEdgeInsets inset = UIEdgeInsetsMake(
self.topToolbar.intrinsicContentSize.height, 0, bottomInset, 0);
[self setInsetForRemoteTabs:contentInset];
[self setInsetForGridViews:contentInset];
}
// Sets the proper insets for the Remote Tabs ViewController to accomodate for
// the safe area, toolbar, and status bar.
- (void)setInsetForRemoteTabs:(UIEdgeInsets)inset {
if (@available(iOS 11, *)) {
// Left and right side could be missing correct safe area
// inset upon rotation. Manually correct it.
self.remoteTabsViewController.additionalSafeAreaInsets = UIEdgeInsetsZero;
UIEdgeInsets additionalSafeArea = inset;
UIEdgeInsets safeArea = self.scrollView.safeAreaInsets;
UIEdgeInsets remoteTabsSafeArea =
self.remoteTabsViewController.tableView.safeAreaInsets;
additionalSafeArea.right += safeArea.right - remoteTabsSafeArea.right;
additionalSafeArea.left += safeArea.left - remoteTabsSafeArea.left;
// If Remote Tabs isn't on the screen, it will not have the right safe area
// insets. Pass down the safe area insets of the scroll view.
if (self.currentPage != TabGridPageRemoteTabs) {
additionalSafeArea.right = safeArea.right;
additionalSafeArea.left = safeArea.left;
}
// Ensure that the View Controller doesn't have safe area inset that already
// covers the view's bounds.
DCHECK(!CGRectIsEmpty(UIEdgeInsetsInsetRect(
......@@ -405,7 +407,19 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
// Sets the proper insets for the Grid ViewControllers to accomodate for the
// safe area and toolbar.
- (void)setInsetForGridViews:(UIEdgeInsets)inset {
- (void)setInsetForGridViews {
// Call the current page setter to sync the scroll view offset to the current
// page value, if the scroll view isn't scrolling. Don't animate this.
if (!self.scrollView.dragging && !self.scrollView.decelerating) {
self.currentPage = _currentPage;
}
// The content inset of the tab grids must be modified so that the toolbars
// do not obscure the tabs. This may change depending on orientation.
CGFloat bottomInset = self.configuration == TabGridConfigurationBottomToolbar
? self.bottomToolbar.intrinsicContentSize.height
: 0;
UIEdgeInsets inset = UIEdgeInsetsMake(
self.topToolbar.intrinsicContentSize.height, 0, bottomInset, 0);
if (@available(iOS 11, *)) {
inset.left = self.scrollView.safeAreaInsets.left;
inset.right = self.scrollView.safeAreaInsets.right;
......@@ -601,32 +615,20 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
RecentTabsTableViewController* viewController = self.remoteTabsViewController;
viewController.view.translatesAutoresizingMaskIntoConstraints = NO;
[self addChildViewController:viewController];
// Inserting RecentTabs into a UIView helps prevent safe area unpredictability
// for the TableView when doing inset calculations in -setInsetForRemoteTabs.
UIView* parentView = [[UIView alloc] init];
parentView.translatesAutoresizingMaskIntoConstraints = NO;
[parentView addSubview:viewController.view];
[contentView addSubview:parentView];
[contentView addSubview:viewController.view];
[viewController didMoveToParentViewController:self];
NSArray* constraints = @[
[parentView.topAnchor constraintEqualToAnchor:contentView.topAnchor],
[parentView.bottomAnchor constraintEqualToAnchor:contentView.bottomAnchor],
[parentView.leadingAnchor
[viewController.view.topAnchor
constraintEqualToAnchor:contentView.topAnchor],
[viewController.view.bottomAnchor
constraintEqualToAnchor:contentView.bottomAnchor],
[viewController.view.leadingAnchor
constraintEqualToAnchor:self.regularTabsViewController.view
.trailingAnchor],
[parentView.trailingAnchor
[viewController.view.trailingAnchor
constraintEqualToAnchor:contentView.trailingAnchor],
[parentView.widthAnchor constraintEqualToAnchor:self.view.widthAnchor],
[viewController.view.widthAnchor
constraintEqualToAnchor:self.view.widthAnchor],
[viewController.view.topAnchor
constraintEqualToAnchor:parentView.topAnchor],
[viewController.view.bottomAnchor
constraintEqualToAnchor:parentView.bottomAnchor],
[viewController.view.leadingAnchor
constraintEqualToAnchor:parentView.leadingAnchor],
[viewController.view.trailingAnchor
constraintEqualToAnchor:parentView.trailingAnchor],
];
[NSLayoutConstraint activateConstraints:constraints];
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment