Suggestions UI - favicon item

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

Index: ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm b/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm
new file mode 100644
index 0000000000000000000000000000000000000000..36d7fd0523abd6ffeda01376baf37c64bc4bdb07
--- /dev/null
+++ b/ios/chrome/browser/ui/suggestions/suggestions_favicon_item.mm
@@ -0,0 +1,133 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h"
+
+#import <UIKit/UIKit.h>
+
+#include "base/logging.h"
+#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_internal_cell.h"
+#import "ios/chrome/browser/ui/uikit_ui_util.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+// Space between two cells in the internal collection view.
+const CGFloat kInternalCellSpacing = 40;
+// Width of the cells in the internal collection view.
+const CGFloat kInternalCellWidth = 70;
+// Height of the cells in the internal collection view.
+const CGFloat kInternalCellHeight = 120;
+// Left inset of the internal collection view.
+const CGFloat kInternalLeftSpacing = 16;

[lpromero, 2017/01/18 09:34:38]

s/left/leading, idem in the comment.

[gambard, 2017/01/18 12:37:58]

Done.

+}  // namespace
+
+#pragma mark - SugggestionsFaviconItem
+
+// The item is the data source of the inner collection view.
+@interface SuggestionsFaviconItem ()<UICollectionViewDataSource> {
+  NSMutableArray<NSString*>* _faviconTitles;
+  NSMutableArray<UIImage*>* _faviconImages;
+}
+
+@end
+
+@implementation SuggestionsFaviconItem
+
+@synthesize delegate = _delegate;
+
+- (instancetype)initWithType:(NSInteger)type {
+  self = [super initWithType:type];
+  if (self) {
+    self.cellClass = [SuggestionsFaviconCell class];
+    _faviconTitles = [NSMutableArray array];
+    _faviconImages = [NSMutableArray array];
+  }
+  return self;
+}
+
+- (void)addFavicon:(UIImage*)favicon withTitle:(NSString*)title {
+  [_faviconImages addObject:favicon];
+  [_faviconTitles addObject:title];
+}
+
+#pragma mark - UICollectionViewDataSource
+
+- (UICollectionViewCell*)collectionView:(UICollectionView*)collectionView
+                 cellForItemAtIndexPath:(NSIndexPath*)indexPath {
+  SuggestionsFaviconInternalCell* cell = [collectionView
+      dequeueReusableCellWithReuseIdentifier:[SuggestionsFaviconInternalCell
+                                                 reuseIdentifier]
+                                forIndexPath:indexPath];
+  cell.faviconView.image = [_faviconImages objectAtIndex:indexPath.item];
+  cell.title.text = [_faviconTitles objectAtIndex:indexPath.item];
+  return cell;
+}
+
+- (NSInteger)collectionView:(UICollectionView*)collectionView
+     numberOfItemsInSection:(NSInteger)section {
+  DCHECK([_faviconImages count] == [_faviconTitles count]);
+  return [_faviconImages count];
+}
+
+#pragma mark - CollectionViewItem
+
+- (void)configureCell:(SuggestionsFaviconCell*)cell {
+  [super configureCell:cell];
+  cell.collectionView.dataSource = self;
+  cell.delegate = self.delegate;

[lpromero, 2017/01/18 09:34:38]

Add a prepareForReuse to the cell, where these are unset. Also reset the
faviconView.image and title.text.

[gambard, 2017/01/18 12:37:58]

Done.
To reset the image and the title, I have added it to the inner cells.
I have not used -prepareForReuse on any other cell. Should I?

[lpromero, 2017/01/18 13:14:02]

Usually I reset everything I can have set in configureCell: and
-collectionView:cellForItemAtIndexPath:.

[gambard, 2017/01/18 14:19:08]

Acknowledged.

+}
+
+@end
+
+#pragma mark - SuggestionsFaviconCell
+
+// The cell is the delegate of the inner collection view.
+@interface SuggestionsFaviconCell ()<UICollectionViewDelegate>
+
+@end
+
+@implementation SuggestionsFaviconCell
+
+@synthesize collectionView = _collectionView;
+@synthesize delegate = _delegate;
+
+- (instancetype)initWithFrame:(CGRect)frame {
+  self = [super initWithFrame:frame];
+  if (self) {
+    UICollectionViewFlowLayout* layout =
+        [[UICollectionViewFlowLayout alloc] init];
+    layout.scrollDirection = UICollectionViewScrollDirectionHorizontal;
+    layout.minimumInteritemSpacing = kInternalCellSpacing;
+    layout.itemSize = CGSizeMake(kInternalCellWidth, kInternalCellHeight);
+    layout.sectionInset = UIEdgeInsetsMake(0, kInternalLeftSpacing, 0, 0);
+
+    _collectionView = [[UICollectionView alloc] initWithFrame:CGRectZero
+                                         collectionViewLayout:layout];
+    [_collectionView registerClass:[SuggestionsFaviconInternalCell class]
+        forCellWithReuseIdentifier:[SuggestionsFaviconInternalCell
+                                       reuseIdentifier]];
+    _collectionView.backgroundColor = [UIColor clearColor];
+
+    _collectionView.delegate = self;
+
+    [self.contentView addSubview:_collectionView];
+    _collectionView.translatesAutoresizingMaskIntoConstraints = NO;

[lpromero, 2017/01/18 09:34:38]

You might need to add that before adding as subview? I find it cleaner and
avoids the system to resize the view to satisfy the autoresizing masks
constraints when the view is added, for in fine removing the constraints.

As a general question, do you get unsatisfiable constraints errors in the
console when you display this?

[gambard, 2017/01/18 12:37:58]

Done.
I don't unsatisfiable constraints. I have some ambiguous constraints on other
type of cell (SuggestionsArticleCell).
In lot of places it is done after adding it to the view, so if it would create
conflicts we would have seen it before.

[lpromero, 2017/01/18 13:14:02]

What kind of ambiguous constraints log do you get?

[gambard, 2017/01/18 14:19:08]

The title and description of the article are pinned to the bottom of the
article, which must be lower than the image bottom. If the image is bigger than
the text, one of the label must be expanded, which one is ambiguous.
I was aware of this, I will fix it in a future CL.

+    AddSameSizeConstraint(_collectionView, self.contentView);
+    [_collectionView.heightAnchor constraintEqualToConstant:kInternalCellHeight]
+        .active = YES;
+  }
+  return self;
+}
+
+#pragma mark - UICollectionViewDelegate
+
+- (void)collectionView:(UICollectionView*)collectionView
+    didSelectItemAtIndexPath:(NSIndexPath*)indexPath {
+  [self.delegate openFaviconAtIndexPath:indexPath];
+}
+
+@end