Create a MostVisited cell for ContentSuggestions

ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_item.mm

+// 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/content_suggestions/cells/content_suggestions_most_visited_item.h"
+
+#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited.h"
+#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_tile.h"
+#import "ios/chrome/browser/ui/favicon/favicon_attributes.h"
+#import "ios/chrome/browser/ui/favicon/favicon_view.h"
+#include "ios/chrome/browser/ui/ui_util.h"
+#import "ios/chrome/browser/ui/uikit_ui_util.h"
+#include "url/gurl.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+const CGFloat kSpacingIPhone = 16;
+const CGFloat kSpacingIPad = 24;
+}
+
+#pragma mark - ContentSuggestionsMostVisitedItem
+
+@implementation ContentSuggestionsMostVisitedItem
+
+@synthesize suggestionIdentifier = _suggestionIdentifier;
+@synthesize mostVisiteds = _mostVisiteds;
+
+- (instancetype)initWithType:(NSInteger)type {
+  self = [super initWithType:type];
+  if (self) {
+    self.cellClass = [ContentSuggestionsMostVisitedCell class];
+  }
+  return self;
+}
+
+- (void)configureCell:(ContentSuggestionsMostVisitedCell*)cell {
+  [super configureCell:cell];
+  [cell setMostVisiteds:self.mostVisiteds];
+}
+
+@end
+
+#pragma mark - ContentSuggestionsMostVisitedCell
+
+@interface ContentSuggestionsMostVisitedCell ()
+
+// The Most Visited tiles to be displayed.
+@property(nonatomic, strong)
+    NSMutableArray<ContentSuggestionsMostVisitedTile*>* tiles;

[lpromero, 2017/04/07 12:46:57]

Maybe add new lines between the property, as it is not easily readable
(especially in the review tool).

[gambard, 2017/04/10 11:49:16]

Done.

+// The first line of Most Visited tiles.
+@property(nonatomic, strong) UIStackView* firstLine;
+// The second line of Most Visited tiles, displayed below the first one.
+@property(nonatomic, strong) UIStackView* secondLine;
+// Contains both stack views.
+@property(nonatomic, copy) NSArray<UIStackView*>* stackViews;
+// Superview for the stack views, used to center them.
+@property(nonatomic, strong) UIView* stackContainer;
+// Width of the |stackContainer|, allowing resizing.
+@property(nonatomic, strong) NSLayoutConstraint* containerWidth;
+// Number of tiles per line during the previous layout.
+@property(nonatomic, assign) NSUInteger oldNumberOfTilesPerLine;

[lpromero, 2017/04/07 12:46:57]

Optional nit: s/old/previous

[gambard, 2017/04/10 11:49:16]

Done.

+
+@end
+
+@implementation ContentSuggestionsMostVisitedCell : MDCCollectionViewCell
+
+@synthesize tiles = _tiles;
+@synthesize firstLine = _firstLine;
+@synthesize secondLine = _secondLine;
+@synthesize stackViews = _stackViews;
+@synthesize stackContainer = _stackContainer;
+@synthesize containerWidth = _containerWidth;
+@synthesize oldNumberOfTilesPerLine = _oldNumberOfTilesPerLine;
+
+#pragma mark - Public
+
+- (instancetype)initWithFrame:(CGRect)frame {

[stkhapugin, 2017/04/07 12:17:58]

optional: add a comment here or above @implementation explaining that this cell:
1. displays two rows of content suggestions
2. uses a vertical stack of two horizontal stacks - one for each line
3. varies number of items per row depending on its width

This might be obvious know, but if this class grows with animations or some
other logic, it might start being confusing.

[gambard, 2017/04/10 11:49:16]

Done.

+  self = [super initWithFrame:frame];
+  if (self) {
+    _tiles = [NSMutableArray array];
+    _firstLine = [[UIStackView alloc] init];
+    _secondLine = [[UIStackView alloc] init];
+    _stackViews = @[ _firstLine, _secondLine ];
+    _stackContainer = [[UIView alloc] init];
+    _oldNumberOfTilesPerLine = 0;
+
+    for (UIStackView* row in self.stackViews) {
+      row.axis = UILayoutConstraintAxisHorizontal;
+      row.spacing = [self spacing];
+      row.translatesAutoresizingMaskIntoConstraints = NO;
+      [_stackContainer addSubview:row];
+      row.layoutMarginsRelativeArrangement = YES;
+    }
+
+    _stackContainer.translatesAutoresizingMaskIntoConstraints = NO;
+
+    [self.contentView addSubview:_stackContainer];
+
+    [_stackContainer.centerXAnchor
+        constraintEqualToAnchor:self.contentView.centerXAnchor]
+        .active = YES;
+    _containerWidth = [_stackContainer.widthAnchor constraintEqualToConstant:0];
+    _containerWidth.active = YES;
+
+    ApplyVisualConstraints(@[ @"V:|[container]|" ],
+                           @{ @"container" : _stackContainer });
+    ApplyVisualConstraints(
+        @[
+          @"V:|[first][second]|", @"H:|[first]-(>=0)-|",
+          @"H:|[second]-(>=0)-|"
+        ],
+        @{ @"first" : _firstLine,
+           @"second" : _secondLine });
+  }
+  return self;
+}
+
+- (void)setMostVisiteds:(NSArray<ContentSuggestionsMostVisited*>*)mostVisiteds {
+  [self.tiles removeAllObjects];
+  for (ContentSuggestionsMostVisited* mostVisited : mostVisiteds) {

[lpromero, 2017/04/07 12:46:57]

s/:/in

[gambard, 2017/04/10 11:49:16]

Done.

+    [self.tiles addObject:[ContentSuggestionsMostVisitedTile
+                              tileWithTitle:mostVisited.title
+                                 attributes:mostVisited.attributes]];
+  }
+}
+
+#pragma mark - UIView
+
+- (void)layoutSubviews {

[lpromero, 2017/04/07 12:46:57]

Please add the comment we use usually to describe the two-passes layout.

[gambard, 2017/04/10 11:49:16]

Done.

+  [super layoutSubviews];
+
+  [self applyLayout];
+
+  [super layoutSubviews];
+}
+
+#pragma mark - Private
+
+// Layouts the tiles. The view is modified only if the number of tiles per line
+// changes.
+- (void)applyLayout {
+  NSUInteger numberOfTilesPerLine = [self numberOfTilesPerLine];
+
+  if (numberOfTilesPerLine == self.oldNumberOfTilesPerLine) {
+    return;
+  }
+  self.oldNumberOfTilesPerLine = numberOfTilesPerLine;
+
+  for (UIStackView* row in self.stackViews) {
+    while (row.arrangedSubviews.count > 0) {
+      UIView* view = row.arrangedSubviews.firstObject;
+      [row removeArrangedSubview:view];
+      [view removeFromSuperview];
+    }
+  }
+
+  NSUInteger numberOfTilesfirstLine =

[lpromero, 2017/04/07 12:46:57]

s/numberOfTilesfirstLine/numberOfTilesFirstLine

[gambard, 2017/04/10 11:49:16]

Done.

+      MIN(numberOfTilesPerLine, self.tiles.count);
+  for (NSUInteger i = 0; i < numberOfTilesfirstLine; i++) {
+    [self.firstLine addArrangedSubview:self.tiles[i]];
+  }
+  if (self.tiles.count > numberOfTilesPerLine) {
+    NSUInteger totalNumberOfTiles =
+        MIN(2 * numberOfTilesPerLine, self.tiles.count);
+    for (NSUInteger i = numberOfTilesPerLine; i < totalNumberOfTiles; i++) {
+      [self.secondLine addArrangedSubview:self.tiles[i]];
+    }
+  }
+
+  self.containerWidth.constant =
+      [self widthForNumberOfItem:numberOfTilesPerLine];
+}
+
+// Returns the maximum number of tiles per line, limited to 4.
+- (NSUInteger)numberOfTilesPerLine {

[stkhapugin, 2017/04/07 12:17:58]

optional: it'd be nice to have a few tests for this part of layout logic,
specifically:

- on iPhone 7, in portrait, the number of tiles is 4
- on iPhone 7 Plus, in portrait, it's still 4
- in landscape it's 4
- on iPhone 5 it's 3 

or something similar, with a few important cases that we actually need. 

[gambard, 2017/04/10 11:49:16]

Done.

+  CGFloat availableWidth = self.contentView.bounds.size.width;
+
+  if (availableWidth > [self widthForNumberOfItem:4])
+    return 4;
+  if (availableWidth > [self widthForNumberOfItem:3])
+    return 3;
+  if (availableWidth > [self widthForNumberOfItem:2])
+    return 2;
+
+  NOTREACHED();
+  return 2;
+}
+
+// Returns the spacing of between tiles, based on the device.

[lpromero, 2017/04/07 12:46:57]

s/between/middle or interior or inner?

Or maybe is it just about removing "of"?

[gambard, 2017/04/10 11:49:16]

Done.

+- (CGFloat)spacing {
+  return IsIPadIdiom() ? kSpacingIPad : kSpacingIPhone;
+}
+
+// Returns the width necessary to fit |numberOfItem| items.
+- (CGFloat)widthForNumberOfItem:(NSUInteger)numberOfItem {
+  return (numberOfItem - 1) * [self spacing] +
+         numberOfItem * [ContentSuggestionsMostVisitedTile width];
+}
+
+@end