Skip to content

Commit

Permalink
[ios] Fix Fake Omnibox bug when sticking to top of NTP
Browse files Browse the repository at this point in the history
The logic in NTPViewController's -offsetToStickOmnibox to determine
when to stick the fake omnibox to the top of the surface is not the
same as the one within the NewTabPageView that determines when the
fake omnibox expansion animation stops (which happens to be the same
position as where it should stick to the top of surface). This change
makes NewTabPageView's logic public to the NTPViewController.

Video: https://drive.google.com/file/d/1tmjMxIrzQjah1wlUzXfrerpYV-M8TKV2/view?usp=sharing

(cherry picked from commit 6053917)

Fixed: 1443603
Change-Id: I0aa81e8aceb77cae41068b08db70be08e2d2bda2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4569060
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: Scott Yoder <scottyoder@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1150978}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4581659
Auto-Submit: Chris Lu <thegreenfrog@chromium.org>
Commit-Queue: Scott Yoder <scottyoder@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#265}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Chris Lu authored and Chromium LUCI CQ committed Jun 2, 2023
1 parent 3e71f0d commit 85c13d6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 23 deletions.
3 changes: 3 additions & 0 deletions ios/chrome/browser/ui/ntp/new_tab_page_header_view.h
Expand Up @@ -52,6 +52,9 @@
- (CGFloat)searchFieldProgressForOffset:(CGFloat)offset
safeAreaInsets:(UIEdgeInsets)safeAreaInsets;

// The positive offset value to begin the fake omniobx expansion animation.
- (CGFloat)offsetToBeginFakeOmniboxExpansionForSplitMode;

// Changes the constraints of searchField based on its initialFrame and the
// scroll view's y `offset`. Also adjust the alpha values for `_searchBoxBorder`
// and `_shadow` and the constant values for the `constraints`. `screenWidth` is
Expand Down
18 changes: 6 additions & 12 deletions ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
Expand Up @@ -363,18 +363,7 @@ - (void)addSeparatorToSearchField:(UIView*)searchField {
- (CGFloat)searchFieldProgressForOffset:(CGFloat)offset
safeAreaInsets:(UIEdgeInsets)safeAreaInsets {
// The scroll offset at which point searchField's frame should stop growing.
CGFloat maxScaleOffset = self.frame.size.height - ToolbarHeight() -
ntp_header::kFakeOmniboxScrolledToTopMargin -
safeAreaInsets.top;
// If the Shrunk logo for the Start Surface is being shown, the searchField
// expansion should start later so that its background does not cut off the
// logo. This mainly impacts notched devices that have a large top Safe Area
// inset. Instead of ensuring the expansion finishes by the time the omnibox
// reaches the bottom of the toolbar, wait until the logo is in the safe area
// before expanding so it is out of view.
if (ShouldShrinkLogoForStartSurface()) {
maxScaleOffset += safeAreaInsets.top;
}
CGFloat maxScaleOffset = [self offsetToBeginFakeOmniboxExpansionForSplitMode];
// If it is not in SplitMode the search field should scroll under the toolbar.
if (!IsSplitToolbarMode(self)) {
maxScaleOffset += ToolbarHeight();
Expand All @@ -392,6 +381,11 @@ - (CGFloat)searchFieldProgressForOffset:(CGFloat)offset
return percent;
}

- (CGFloat)offsetToBeginFakeOmniboxExpansionForSplitMode {
return self.frame.size.height - ToolbarHeight() -
ntp_header::kFakeOmniboxScrolledToTopMargin;
}

- (void)updateSearchFieldWidth:(NSLayoutConstraint*)widthConstraint
height:(NSLayoutConstraint*)heightConstraint
topMargin:(NSLayoutConstraint*)topMarginConstraint
Expand Down
Expand Up @@ -110,6 +110,9 @@
// Update any dynamic constraints.
- (void)updateConstraints;

// The positive offset value to begin the fake omniobx expansion animation.
- (CGFloat)offsetToBeginFakeOmniboxExpansionForSplitMode;

@end

#endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_HEADER_VIEW_CONTROLLER_H_
Expand Up @@ -303,6 +303,10 @@ - (void)viewDidAppear:(BOOL)animated {
}
}

- (CGFloat)offsetToBeginFakeOmniboxExpansionForSplitMode {
return [self.headerView offsetToBeginFakeOmniboxExpansionForSplitMode];
}

#pragma mark - Private

// Initialize and add a search field tap target and a voice search button.
Expand Down
17 changes: 6 additions & 11 deletions ios/chrome/browser/ui/ntp/new_tab_page_view_controller.mm
Expand Up @@ -1480,17 +1480,12 @@ - (CGFloat)offsetWhenScrolledIntoFeed {
// The y-position content offset for when the fake omnibox
// should stick to the top of the NTP.
- (CGFloat)offsetToStickOmnibox {
CGFloat offset =
-(self.headerViewController.view.frame.size.height -
[self stickyOmniboxHeight] -
[self.feedHeaderViewController customSearchEngineViewHeight]);
if (IsSplitToolbarMode(self)) {
offset -= [self contentSuggestionsContentHeight];
}
if (self.feedTopSectionViewController) {
offset -= self.feedTopSectionViewController.view.frame.size.height;
}
return offset;
// Do not need to factor in safeAreaInsets.top because the fake omnibox sticks
// below it, so it is effectively just the scroll distance between top of
// NTPHeader and the top of the Fake Omnibox.
return -([self heightAboveFeed] -
[self.headerViewController
offsetToBeginFakeOmniboxExpansionForSplitMode]);
}

// Whether the collection view has attained its minimum height.
Expand Down

0 comments on commit 85c13d6

Please sign in to comment.