Commit 84957eeb authored by sczs's avatar sczs Committed by Chromium LUCI CQ

[ios] Saves Feed scrolling offset for refactored NTP.

- Sets the NTPViewController to the ntp_home_mediator so it can
save/load the Feed scrolling offset for the refactored feed.
- Adds setContentOffset to ntp_view_controller to set the offset to the
saved value.

Bug: 1114792
Change-Id: Ib4256a3cb4d65a8e7e6e6f00ec017a13a1447250
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628316
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarAdam Trudeau-Arcaro <adamta@google.com>
Cr-Commit-Position: refs/heads/master@{#843392}
parent 78055f09
...@@ -26,8 +26,8 @@ class WebState; ...@@ -26,8 +26,8 @@ class WebState;
@property(nonatomic, weak) id<NewTabPageControllerDelegate> toolbarDelegate; @property(nonatomic, weak) id<NewTabPageControllerDelegate> toolbarDelegate;
// Whether the Suggestions UI is displayed. If this is true, start is a no-op. // YES if the coordinator has started. If YES, start is a no-op.
@property(nonatomic, readonly) BOOL visible; @property(nonatomic, readonly) BOOL started;
@property(nonatomic, strong, readonly) @property(nonatomic, strong, readonly)
ContentSuggestionsHeaderViewController* headerController; ContentSuggestionsHeaderViewController* headerController;
......
...@@ -137,12 +137,12 @@ ...@@ -137,12 +137,12 @@
- (void)start { - (void)start {
DCHECK(self.browser); DCHECK(self.browser);
DCHECK(self.ntpMediator); DCHECK(self.ntpMediator);
if (self.visible) { if (self.started) {
// Prevent this coordinator from being started twice in a row // Prevent this coordinator from being started twice in a row
return; return;
} }
_visible = YES; _started = YES;
// Make sure that the omnibox is unfocused to prevent having it visually // Make sure that the omnibox is unfocused to prevent having it visually
// focused while the NTP is just created (with the fakebox visible). // focused while the NTP is just created (with the fakebox visible).
...@@ -349,7 +349,7 @@ ...@@ -349,7 +349,7 @@
->RemoveFeedViewController(self.discoverFeedViewController); ->RemoveFeedViewController(self.discoverFeedViewController);
} }
self.contentSuggestionsExpanded = nil; self.contentSuggestionsExpanded = nil;
_visible = NO; _started = NO;
} }
- (UIViewController*)viewController { - (UIViewController*)viewController {
......
...@@ -33,6 +33,7 @@ class Browser; ...@@ -33,6 +33,7 @@ class Browser;
@class ContentSuggestionsMetricsRecorder; @class ContentSuggestionsMetricsRecorder;
@class ContentSuggestionsViewController; @class ContentSuggestionsViewController;
@protocol LogoVendor; @protocol LogoVendor;
@class NewTabPageViewController;
@protocol NTPHomeConsumer; @protocol NTPHomeConsumer;
@class NTPHomeMetrics; @class NTPHomeMetrics;
@class DiscoverFeedMetricsRecorder; @class DiscoverFeedMetricsRecorder;
...@@ -74,9 +75,6 @@ class VoiceSearchAvailability; ...@@ -74,9 +75,6 @@ class VoiceSearchAvailability;
@property(nonatomic, strong) NTPHomeMetrics* NTPMetrics; @property(nonatomic, strong) NTPHomeMetrics* NTPMetrics;
// Recorder for the metrics related to the Discover feed. // Recorder for the metrics related to the Discover feed.
@property(nonatomic, strong) DiscoverFeedMetricsRecorder* discoverFeedMetrics; @property(nonatomic, strong) DiscoverFeedMetricsRecorder* discoverFeedMetrics;
// View Controller displaying the suggestions.
@property(nonatomic, weak)
ContentSuggestionsViewController* suggestionsViewController;
// Primary collection view controller that receives scroll events. // Primary collection view controller that receives scroll events.
// In the refactored NTP, the Discover feed collection view behaves as the // In the refactored NTP, the Discover feed collection view behaves as the
// primary NTP scroll view. Otherwise, the content suggestions collection view // primary NTP scroll view. Otherwise, the content suggestions collection view
...@@ -85,6 +83,21 @@ class VoiceSearchAvailability; ...@@ -85,6 +83,21 @@ class VoiceSearchAvailability;
// refactored NTP. // refactored NTP.
@property(nonatomic, weak) id<ContentSuggestionsCollectionControlling> @property(nonatomic, weak) id<ContentSuggestionsCollectionControlling>
primaryViewController; primaryViewController;
// View Controller for the NTP if using the non refactored NTP or the Feed is
// not visible.
// TODO(crbug.com/1114792): Create a protocol to avoid duplication and update
// comment.
@property(nonatomic, weak)
ContentSuggestionsViewController* suggestionsViewController;
// View Controller forthe NTP if using the refactored NTP and the Feed is
// visible.
// TODO(crbug.com/1114792): Create a protocol to avoid duplication and update
// comment.
@property(nonatomic, weak) NewTabPageViewController* ntpViewController;
// TODO(crbug.com/1114792): Update this comment to remove "refactored" when the
// NTP refactors launches.
@property(nonatomic, assign, getter=isRefactoredFeedVisible)
BOOL refactoredFeedVisible;
@property(nonatomic, weak) @property(nonatomic, weak)
ContentSuggestionsHeaderSynchronizer* headerCollectionInteractionHandler; ContentSuggestionsHeaderSynchronizer* headerCollectionInteractionHandler;
// Mediator for the ContentSuggestions. // Mediator for the ContentSuggestions.
......
...@@ -42,8 +42,10 @@ ...@@ -42,8 +42,10 @@
#import "ios/chrome/browser/ui/content_suggestions/ntp_home_consumer.h" #import "ios/chrome/browser/ui/content_suggestions/ntp_home_consumer.h"
#import "ios/chrome/browser/ui/content_suggestions/ntp_home_metrics.h" #import "ios/chrome/browser/ui/content_suggestions/ntp_home_metrics.h"
#import "ios/chrome/browser/ui/content_suggestions/user_account_image_update_delegate.h" #import "ios/chrome/browser/ui/content_suggestions/user_account_image_update_delegate.h"
#import "ios/chrome/browser/ui/ntp/discover_feed_wrapper_view_controller.h"
#include "ios/chrome/browser/ui/ntp/metrics.h" #include "ios/chrome/browser/ui/ntp/metrics.h"
#import "ios/chrome/browser/ui/ntp/new_tab_page_header_constants.h" #import "ios/chrome/browser/ui/ntp/new_tab_page_header_constants.h"
#import "ios/chrome/browser/ui/ntp/new_tab_page_view_controller.h"
#import "ios/chrome/browser/ui/ntp/notification_promo_whats_new.h" #import "ios/chrome/browser/ui/ntp/notification_promo_whats_new.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/browser/url_loading/url_loading_browser_agent.h" #import "ios/chrome/browser/url_loading/url_loading_browser_agent.h"
...@@ -642,10 +644,17 @@ const char kNTPHelpURL[] = ...@@ -642,10 +644,17 @@ const char kNTPHelpURL[] =
web::NavigationManager* manager = webState->GetNavigationManager(); web::NavigationManager* manager = webState->GetNavigationManager();
web::NavigationItem* item = manager->GetLastCommittedItem(); web::NavigationItem* item = manager->GetLastCommittedItem();
web::PageDisplayState displayState; web::PageDisplayState displayState;
UIEdgeInsets contentInset =
self.suggestionsViewController.collectionView.contentInset; // TODO(crbug.com/1114792): Create a protocol to stop having references to
CGPoint contentOffset = // both of these ViewControllers directly.
self.suggestionsViewController.collectionView.contentOffset; UICollectionView* collectionView =
self.refactoredFeedVisible
? self.ntpViewController.discoverFeedWrapperViewController
.feedCollectionView
: self.suggestionsViewController.collectionView;
UIEdgeInsets contentInset = collectionView.contentInset;
CGPoint contentOffset = collectionView.contentOffset;
contentOffset.y -= contentOffset.y -=
self.headerCollectionInteractionHandler.collectionShiftingOffset; self.headerCollectionInteractionHandler.collectionShiftingOffset;
displayState.scroll_state() = displayState.scroll_state() =
...@@ -662,8 +671,15 @@ const char kNTPHelpURL[] = ...@@ -662,8 +671,15 @@ const char kNTPHelpURL[] =
web::NavigationItem* item = navigationManager->GetVisibleItem(); web::NavigationItem* item = navigationManager->GetVisibleItem();
CGFloat offset = CGFloat offset =
item ? item->GetPageDisplayState().scroll_state().content_offset().y : 0; item ? item->GetPageDisplayState().scroll_state().content_offset().y : 0;
if (offset > 0) if (offset > 0) {
[self.suggestionsViewController setContentOffset:offset]; // TODO(crbug.com/1114792): Create a protocol to stop having references to
// both of these ViewControllers directly.
if ([self isRefactoredFeedVisible]) {
[self.ntpViewController setContentOffset:offset];
} else {
[self.suggestionsViewController setContentOffset:offset];
}
}
} }
// Fetches and update user's avatar on NTP, or use default avatar if user is // Fetches and update user's avatar on NTP, or use default avatar if user is
......
...@@ -20,6 +20,16 @@ ...@@ -20,6 +20,16 @@
// TODO(crbug.com/1114792): Handle case where feed is disabled, replacing it // TODO(crbug.com/1114792): Handle case where feed is disabled, replacing it
// with a regular scroll view. // with a regular scroll view.
_discoverFeed = discoverFeed; _discoverFeed = discoverFeed;
// Iterates through subviews to find collection view containing feed
// articles.
// TODO(crbug.com/1085419): Once the CollectionView is cleanly exposed,
// remove this loop.
for (UIView* view in _discoverFeed.view.subviews) {
if ([view isKindOfClass:[UICollectionView class]]) {
_feedCollectionView = static_cast<UICollectionView*>(view);
}
}
} }
return self; return self;
} }
...@@ -28,16 +38,6 @@ ...@@ -28,16 +38,6 @@
[super viewDidLoad]; [super viewDidLoad];
UIView* discoverView = self.discoverFeed.view; UIView* discoverView = self.discoverFeed.view;
// Iterates through subviews to find collection view containing feed articles.
// TODO(crbug.com/1085419): Once the CollectionView is cleanly exposed, remove
// this loop.
for (UIView* view in discoverView.subviews) {
if ([view isKindOfClass:[UICollectionView class]]) {
_feedCollectionView = static_cast<UICollectionView*>(view);
}
}
[self.discoverFeed willMoveToParentViewController:self]; [self.discoverFeed willMoveToParentViewController:self];
[self addChildViewController:self.discoverFeed]; [self addChildViewController:self.discoverFeed];
[self.view addSubview:discoverView]; [self.view addSubview:discoverView];
......
...@@ -168,10 +168,12 @@ ...@@ -168,10 +168,12 @@
[self.contentSuggestionsCoordinator start]; [self.contentSuggestionsCoordinator start];
self.ntpMediator.refactoredFeedVisible = [self isNTPRefactoredAndFeedVisible];
if ([self isNTPRefactoredAndFeedVisible]) { if ([self isNTPRefactoredAndFeedVisible]) {
self.ntpViewController = [[NewTabPageViewController alloc] self.ntpViewController = [[NewTabPageViewController alloc]
initWithContentSuggestionsViewController: initWithContentSuggestionsViewController:
self.contentSuggestionsCoordinator.viewController]; self.contentSuggestionsCoordinator.viewController];
self.ntpMediator.ntpViewController = self.ntpViewController;
UIViewController* discoverFeedViewController = UIViewController* discoverFeedViewController =
ios::GetChromeBrowserProvider() ios::GetChromeBrowserProvider()
...@@ -297,7 +299,8 @@ ...@@ -297,7 +299,8 @@
} }
- (void)willUpdateSnapshot { - (void)willUpdateSnapshot {
if ([self isNTPRefactoredAndFeedVisible]) { if (self.contentSuggestionsCoordinator.started &&
[self isNTPRefactoredAndFeedVisible]) {
[self.ntpViewController willUpdateSnapshot]; [self.ntpViewController willUpdateSnapshot];
} else { } else {
[self.contentSuggestionsCoordinator willUpdateSnapshot]; [self.contentSuggestionsCoordinator willUpdateSnapshot];
...@@ -436,6 +439,9 @@ ...@@ -436,6 +439,9 @@
// YES if we're using the refactored NTP and the Discover Feed is visible. // YES if we're using the refactored NTP and the Discover Feed is visible.
- (BOOL)isNTPRefactoredAndFeedVisible { - (BOOL)isNTPRefactoredAndFeedVisible {
// Make sure we call this only if self.contentSuggestionsCoordinator has been
// started.
DCHECK(self.contentSuggestionsCoordinator.started);
return IsRefactoredNTP() && return IsRefactoredNTP() &&
[self.contentSuggestionsCoordinator isDiscoverFeedVisible]; [self.contentSuggestionsCoordinator isDiscoverFeedVisible];
} }
......
...@@ -50,6 +50,10 @@ ...@@ -50,6 +50,10 @@
// Called when a snapshot of the content will be taken. // Called when a snapshot of the content will be taken.
- (void)willUpdateSnapshot; - (void)willUpdateSnapshot;
// Sets the collection contentOffset to |offset|, or caches the value and
// applies it after the first layout.
- (void)setContentOffset:(CGFloat)offset;
@end @end
#endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_VIEW_CONTROLLER_H_ #endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_VIEW_CONTROLLER_H_
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#import "ios/chrome/browser/ui/ntp/discover_feed_wrapper_view_controller.h" #import "ios/chrome/browser/ui/ntp/discover_feed_wrapper_view_controller.h"
#import "ios/chrome/browser/ui/ntp/new_tab_page_content_delegate.h" #import "ios/chrome/browser/ui/ntp/new_tab_page_content_delegate.h"
#import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h" #import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h"
#import "ios/chrome/browser/ui/toolbar/public/toolbar_utils.h"
#import "ios/chrome/browser/ui/util/named_guide.h" #import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/common/ui/util/constraints_ui_util.h" #import "ios/chrome/common/ui/util/constraints_ui_util.h"
...@@ -50,6 +51,9 @@ const CGFloat kOffsetToPinOmnibox = 100; ...@@ -50,6 +51,9 @@ const CGFloat kOffsetToPinOmnibox = 100;
// view. // view.
@property(nonatomic, weak) ContentSuggestionsLayout* contentSuggestionsLayout; @property(nonatomic, weak) ContentSuggestionsLayout* contentSuggestionsLayout;
// Initial scrolling offset, used to restore the previous scrolling position.
@property(nonatomic, assign) CGFloat initialContentOffset;
@end @end
@implementation NewTabPageViewController @implementation NewTabPageViewController
...@@ -66,6 +70,7 @@ const CGFloat kOffsetToPinOmnibox = 100; ...@@ -66,6 +70,7 @@ const CGFloat kOffsetToPinOmnibox = 100;
// TODO(crbug.com/1114792): Instantiate this depending on the initial scroll // TODO(crbug.com/1114792): Instantiate this depending on the initial scroll
// position. // position.
_scrolledIntoFeed = NO; _scrolledIntoFeed = NO;
_initialContentOffset = NAN;
} }
return self; return self;
...@@ -142,6 +147,8 @@ const CGFloat kOffsetToPinOmnibox = 100; ...@@ -142,6 +147,8 @@ const CGFloat kOffsetToPinOmnibox = 100;
self.discoverFeedWrapperViewController.feedCollectionView.contentInset = self.discoverFeedWrapperViewController.feedCollectionView.contentInset =
UIEdgeInsetsMake(collectionView.contentSize.height, 0, 0, 0); UIEdgeInsetsMake(collectionView.contentSize.height, 0, 0, 0);
self.headerSynchronizer.additionalOffset = collectionView.contentSize.height; self.headerSynchronizer.additionalOffset = collectionView.contentSize.height;
[self applyContentOffset];
} }
- (void)viewWillAppear:(BOOL)animated { - (void)viewWillAppear:(BOOL)animated {
...@@ -202,10 +209,20 @@ const CGFloat kOffsetToPinOmnibox = 100; ...@@ -202,10 +209,20 @@ const CGFloat kOffsetToPinOmnibox = 100;
[self updateOverscrollActionsState]; [self updateOverscrollActionsState];
} }
#pragma mark - Public
- (void)willUpdateSnapshot { - (void)willUpdateSnapshot {
[self.overscrollActionsController clear]; [self.overscrollActionsController clear];
} }
- (void)setContentOffset:(CGFloat)offset {
self.initialContentOffset = offset;
if (self.isViewLoaded && self.collectionView.window &&
self.collectionView.contentSize.height != 0) {
[self applyContentOffset];
}
}
#pragma mark - UIScrollViewDelegate #pragma mark - UIScrollViewDelegate
- (void)scrollViewDidScroll:(UIScrollView*)scrollView { - (void)scrollViewDidScroll:(UIScrollView*)scrollView {
...@@ -326,6 +343,28 @@ const CGFloat kOffsetToPinOmnibox = 100; ...@@ -326,6 +343,28 @@ const CGFloat kOffsetToPinOmnibox = 100;
[self.ntpContentDelegate reloadContentSuggestions]; [self.ntpContentDelegate reloadContentSuggestions];
} }
// Sets the collectionView's contentOffset if |_initialContentOffset| is set.
- (void)applyContentOffset {
if (!isnan(self.initialContentOffset)) {
UICollectionView* collection =
self.discoverFeedWrapperViewController.feedCollectionView;
// Don't set the offset such as the content of the collection is smaller
// than the part of the collection which should be displayed with that
// offset, taking into account the size of the toolbar.
CGFloat offset = MAX(
0, MIN(self.initialContentOffset,
collection.contentSize.height - collection.bounds.size.height -
ToolbarExpandedHeight(
self.traitCollection.preferredContentSizeCategory) +
collection.contentInset.bottom));
if (collection.contentOffset.y != offset) {
collection.contentOffset = CGPointMake(0, offset);
// TODO(crbug.com/1114792): Add the FakeOmnibox if necessary.
}
}
self.initialContentOffset = NAN;
}
#pragma mark - Setters #pragma mark - Setters
// Sets whether or not the NTP is scrolled into the feed and notifies the // Sets whether or not the NTP is scrolled into the feed and notifies the
......
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