[ios] Fix issue with SetUpList expand animation and scrolling

(cherry picked from commit 18df644) Fixed: 1449320 Change-Id: I7fc7cd8f2bcc6903b0af36e61ac0416b4536cdf6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571679 Commit-Queue: Scott Yoder <scottyoder@google.com> Reviewed-by: Chris Lu <thegreenfrog@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1150004} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571951 Cr-Commit-Position: refs/branch-heads/5790@{#132} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · May 30, 2023 · 803348b · 803348b
1 parent 7b85211
commit 803348b
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 10 deletions.
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm
@@ -61,6 +61,7 @@
 #import "ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view.h"
 #import "ios/chrome/browser/ui/menu/browser_action_factory.h"
 #import "ios/chrome/browser/ui/menu/menu_histograms.h"
+#import "ios/chrome/browser/ui/ntp/feed_delegate.h"
 #import "ios/chrome/browser/ui/ntp/new_tab_page_constants.h"
 #import "ios/chrome/browser/ui/ntp/new_tab_page_delegate.h"
 #import "ios/chrome/browser/ui/ntp/new_tab_page_feature.h"
@@ -415,6 +416,10 @@ - (void)showSetUpListMenuWithButton:(UIButton*)button {
   [_actionSheetCoordinator start];
 }
 
+- (void)setUpListViewHeightDidChange {
+  [self.feedDelegate contentSuggestionsWasUpdated];
+}
+
 #pragma mark - SetUpList Helpers
 
 // Shows the Default Browser Promo.

diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
@@ -501,7 +501,8 @@ - (void)showSetUpListWithItems:(NSArray<SetUpListItemViewData*>*)items {
     }
 
   } else {
-    SetUpListView* setUpListView = [[SetUpListView alloc] initWithItems:items];
+    SetUpListView* setUpListView =
+        [[SetUpListView alloc] initWithItems:items rootView:self.view];
     setUpListView.delegate = self.setUpListViewDelegate;
     self.setUpListView = setUpListView;
     [self.verticalStackView insertArrangedSubview:setUpListView atIndex:index];

diff --git a/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view.h b/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view.h
@@ -21,14 +21,19 @@ enum class SetUpListItemType;
 // Called when the user selects the Set Up List menu.
 - (void)showSetUpListMenuWithButton:(UIButton*)button;
 
+// Called when the height of the SetUpListView did change.
+- (void)setUpListViewHeightDidChange;
+
 @end
 
 // A view that displays the Set Up List, a list of tasks a new user may want
 // to complete to set up the app.
 @interface SetUpListView : UIView
 
-// Initializes the SetUpListView, with the given items.
-- (instancetype)initWithItems:(NSArray<SetUpListItemViewData*>*)items;
+// Initializes the SetUpListView, with the given `items`, and the given
+// `rootView`.
+- (instancetype)initWithItems:(NSArray<SetUpListItemViewData*>*)items
+                     rootView:(UIView*)rootView;
 
 // The object that should handle delegate events.
 @property(nonatomic, weak) id<SetUpListViewDelegate> delegate;

diff --git a/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view.mm b/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view.mm
@@ -70,6 +70,9 @@ @implementation SetUpListView {
   // The array of item data given to the initializer.
   NSArray<SetUpListItemViewData*>* _itemsData;
 
+  // The view that needs layout if SetUpListView's height changes.
+  UIView* _rootView;
+
   // The array of SetUpListItemViews.
   NSMutableArray<SetUpListItemView*>* _items;
 
@@ -86,11 +89,13 @@ @implementation SetUpListView {
   UIButton* _menuButton;
 }
 
-- (instancetype)initWithItems:(NSArray<SetUpListItemViewData*>*)items {
+- (instancetype)initWithItems:(NSArray<SetUpListItemViewData*>*)items
+                     rootView:(UIView*)rootView {
   self = [super init];
   if (self) {
     CHECK_GT(items.count, 0ul);
     _itemsData = items;
+    _rootView = rootView;
   }
   return self;
 }
@@ -433,7 +438,12 @@ - (void)expandItemsStack {
     index++;
   }
   _itemsStack.accessibilityElements = _items;
+  // Layout the newly added (but still hidden) items, so that the animation is
+  // correct.
+  [self setNeedsLayout];
+  [self layoutIfNeeded];
 
+  __weak __typeof(self) weakSelf = self;
   __weak __typeof(_expandButton) weakExpandButton = _expandButton;
   [UIView animateWithDuration:kExpandAnimationDuration.InSecondsF()
                    animations:^{
@@ -444,6 +454,7 @@ - (void)expandItemsStack {
                      // Flip the expand button to point up;
                      weakExpandButton.transform = CGAffineTransformScale(
                          CGAffineTransformIdentity, 1, -1);
+                     [weakSelf heightDidChange];
                    }];
 }
 
@@ -455,6 +466,7 @@ - (void)unexpandItemsStack {
   _expandButton.accessibilityLabel =
       l10n_util::GetNSString(IDS_IOS_SET_UP_LIST_EXPAND);
 
+  __weak __typeof(self) weakSelf = self;
   __weak __typeof(_expandButton) weakExpandButton = _expandButton;
   [UIView animateWithDuration:kExpandAnimationDuration.InSecondsF()
       animations:^{
@@ -465,6 +477,7 @@ - (void)unexpandItemsStack {
         // Flip the expand button to point down again;
         weakExpandButton.transform =
             CGAffineTransformScale(CGAffineTransformIdentity, 1, 1);
+        [weakSelf heightDidChange];
       }
       completion:^(BOOL finished) {
         for (SetUpListItemView* item in items) {
@@ -473,6 +486,14 @@ - (void)unexpandItemsStack {
       }];
 }
 
+// Tells the root view to re-layout and tells the delegate that the height
+// changed.
+- (void)heightDidChange {
+  [_rootView setNeedsLayout];
+  [_rootView layoutIfNeeded];
+  [self.delegate setUpListViewHeightDidChange];
+}
+
 #pragma mark Private methods (All Set view)
 
 // Returns `YES` if all items are marked complete.

diff --git a/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view_unittest.mm b/ios/chrome/browser/ui/content_suggestions/set_up_list/set_up_list_view_unittest.mm
@@ -112,7 +112,8 @@ void ExpectSubviewCount(int count, Class klass) {
 // expand / collapse button functions as it should. Also verifies that
 // the items that are expected to be in the list are there at each step.
 TEST_F(SetUpListViewTest, ExpandCollapse) {
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData
+                                                    rootView:nil];
   [_superview addSubview:view];
 
   // It should initially display two items.
@@ -142,7 +143,9 @@ void ExpectSubviewCount(int count, Class klass) {
 // Tests that a touch on a SetUpListItemView results in a call to the view\
 // delegate.
 TEST_F(SetUpListViewTest, TouchSetUpListItemView) {
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData
+                                                    rootView:nil];
+
   [_superview addSubview:view];
 
   SetUpListItemView* item_view =
@@ -167,7 +170,8 @@ void ExpectSubviewCount(int count, Class klass) {
 
 // Tests that a tap on the menu button results in a call to the view delegate.
 TEST_F(SetUpListViewTest, MenuButton) {
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData
+                                                    rootView:nil];
   [_superview addSubview:view];
 
   id view_delegate = OCMProtocolMock(@protocol(SetUpListViewDelegate));
@@ -181,7 +185,8 @@ void ExpectSubviewCount(int count, Class klass) {
 
 // Tests that a SetUpListItemView can be marked complete.
 TEST_F(SetUpListViewTest, SetUpListItemViewMarkComplete) {
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData
+                                                    rootView:nil];
   [_superview addSubview:view];
 
   SetUpListItemView* item_view =
@@ -200,7 +205,9 @@ void ExpectSubviewCount(int count, Class klass) {
 TEST_F(SetUpListViewTest, NoExpandButton) {
   NSArray<SetUpListItemViewData*>* first_two_items =
       [_itemsData subarrayWithRange:NSMakeRange(0, 2)];
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:first_two_items];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:first_two_items
+                                                    rootView:nil];
+
   [_superview addSubview:view];
 
   SetUpListItemView* expand_button =
@@ -210,7 +217,8 @@ void ExpectSubviewCount(int count, Class klass) {
 
 // Tests that the "All Set" can be shown.
 TEST_F(SetUpListViewTest, AllSetView) {
-  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData];
+  SetUpListView* view = [[SetUpListView alloc] initWithItems:_itemsData
+                                                    rootView:nil];
   [_superview addSubview:view];
 
   ExpectSubview(@"kSetUpListAllSetID", false);