Suggestions UI - stack item

ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm

Index: ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm b/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm
index 0940e8db9b2fe0f8a11dfa179326090f669d3c53..e3b7e5c106d6c44da2a20b32e2bec7b3ac35ba25 100644
--- a/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm
+++ b/ios/chrome/browser/ui/suggestions/suggestions_view_controller.mm
@@ -11,12 +11,15 @@
 #import "ios/chrome/browser/ui/suggestions/suggestions_collection_updater.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_commands.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_item_actions.h"
+#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h"
+#import "ios/chrome/browser/ui/suggestions/suggestions_stack_item_actions.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
-@interface SuggestionsViewController ()<SuggestionsItemActions>
+@interface SuggestionsViewController ()<SuggestionsItemActions,
+                                        SuggestionsStackItemActions>
 
 @property(nonatomic, strong) SuggestionsCollectionUpdater* collectionUpdater;
 
@@ -36,27 +39,22 @@
 - (void)viewDidLoad {
   [super viewDidLoad];
 
-  _collectionUpdater = [[SuggestionsCollectionUpdater alloc]
-      initWithCollectionViewController:self];
+  _collectionUpdater = [[SuggestionsCollectionUpdater alloc] init];
+  _collectionUpdater.collectionViewController = self;
 
   self.collectionView.delegate = self;
   self.styler.cellStyle = MDCCollectionViewCellStyleCard;
 }
 
-#pragma mark - MDCCollectionViewStylingDelegate
-
-- (CGFloat)collectionView:(UICollectionView*)collectionView
-    cellHeightAtIndexPath:(NSIndexPath*)indexPath {
-  CollectionViewItem* item =
-      [self.collectionViewModel itemAtIndexPath:indexPath];
-  UIEdgeInsets inset = [self collectionView:collectionView
-                                     layout:collectionView.collectionViewLayout
-                     insetForSectionAtIndex:indexPath.section];
+#pragma mark - UICollectionViewDelegate
 
-  return [MDCCollectionViewCell
-      cr_preferredHeightForWidth:CGRectGetWidth(collectionView.bounds) -
-                                 inset.left - inset.right
-                         forItem:item];
+- (void)collectionView:(UICollectionView*)collectionView
+    didSelectItemAtIndexPath:(NSIndexPath*)indexPath {
+  [super collectionView:collectionView didSelectItemAtIndexPath:indexPath];
+  if ([[self.collectionViewModel itemAtIndexPath:indexPath]
+          isKindOfClass:[SuggestionsStackItem class]]) {

[marq (ping after 24h), 2017/01/16 18:04:42]

I don't like the runtime class membership test. 

lpromero@: I think it would be good if a collection view could declare all of
its items to be members of some CollectionViewItem subclass, and then
[CollectionViewModel itemAtIndexPath:] can return an Item with some known
additional API that the collection view controller can query without relying on
RTTI. 

e.g, if by default a CollectionViewController's |collectionViewModel| property
is a CollectionViewModel<CollectionViewItem*>*, a subclass can declare their
property to instead be a  CollectionViewModel<SuggestionCollectionViewItem*>*. 

This is all doable with ObjC generic annotations, but the downside is that the
implementation of CollectionViewModel has to use id internally for the type of
the object it contains.

[lpromero, 2017/01/16 18:17:55]

This should test the itemType property of the item instead of its class.

[gambard, 2017/01/17 13:28:11]

I used the item type.

[gambard, 2017/01/17 13:28:11]

The ItemType property is unique per class for now but it might not always be
true.

+    [self.suggestionCommandHandler openReadingList];
+  }
 }
 
 #pragma mark - SuggestionsExpandableCellDelegate
@@ -85,6 +83,44 @@
                             toSection:inputSection];
 }
 
+#pragma mark - SuggestionsStackItemActions
+
+- (void)topOfStackPressed:(id)sender {
+  [self.suggestionCommandHandler openFirstPageOfReadingList];
+}
+
+#pragma mark - MDCCollectionViewStylingDelegate
+
+- (nullable UIColor*)collectionView:(nonnull UICollectionView*)collectionView

[marq (ping after 24h), 2017/01/16 18:04:42]

I think we don't want nullability annotations on some methods and not on others.

[gambard, 2017/01/17 13:28:11]

Done.

+     cellBackgroundColorAtIndexPath:(nonnull NSIndexPath*)indexPath {
+  if ([self.collectionUpdater isCustomStyling:indexPath.section]) {
+    return [UIColor clearColor];
+  }
+  return [UIColor whiteColor];
+}
+
+- (BOOL)collectionView:(nonnull UICollectionView*)collectionView
+    shouldHideItemBackgroundAtIndexPath:(nonnull NSIndexPath*)indexPath {
+  if ([self.collectionUpdater isCustomStyling:indexPath.section]) {
+    return YES;
+  }
+  return NO;
+}
+
+- (CGFloat)collectionView:(UICollectionView*)collectionView
+    cellHeightAtIndexPath:(NSIndexPath*)indexPath {
+  CollectionViewItem* item =
+      [self.collectionViewModel itemAtIndexPath:indexPath];
+  UIEdgeInsets inset = [self collectionView:collectionView
+                                     layout:collectionView.collectionViewLayout
+                     insetForSectionAtIndex:indexPath.section];
+
+  return [MDCCollectionViewCell
+      cr_preferredHeightForWidth:CGRectGetWidth(collectionView.bounds) -

[marq (ping after 24h), 2017/01/16 18:04:42]

Taking edge inset width into account for cell height seems like something the
superclass should do.

[lpromero, 2017/01/16 18:17:55]

This is dependent on the cellStyle this controller wants for the given cell,
i.e. because of line 46.
Which superclass did you mean: CollectionViewController, or
MDCCollectionViewCell?

[gambard, 2017/01/17 13:28:11]

Maybe we should add a method to CollectionViewController
-widthOfCellInSection:(NSInteger)section?

[marq (ping after 24h), 2017/01/17 14:36:06]

I meant CollectionViewController, since this is a method defined on that class,
but it doesn't get a superclass invocation so that won't really help.

+                                 inset.left - inset.right
+                         forItem:item];
+}
+
 #pragma mark - Private
 
 - (void)expand:(BOOL)expand cell:(UICollectionViewCell*)cell {