Add favicon view to ContentSuggestionsArticle

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

Index: ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
diff --git a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
index e92e1b9523f140975443dff1985f956c056f29f8..7eb95688377f4a2b1e8fec34c47f5b0e23590a70 100644
--- a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
+++ b/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
@@ -6,6 +6,8 @@
 
 #include "base/time/time.h"
 #import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h"
+#import "ios/chrome/browser/ui/favicon/favicon_attributes.h"
+#import "ios/chrome/browser/ui/favicon/favicon_view.h"
 #import "ios/chrome/browser/ui/uikit_ui_util.h"
 #import "ios/chrome/browser/ui/util/i18n_string.h"
 #import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h"
@@ -20,6 +22,10 @@ namespace {
 const CGFloat kImageSize = 72;
 // When updating this, make sure to update |layoutSubviews|.

[lpromero, 2017/03/29 14:33:01]

Why? Here and below.

[gambard, 2017/03/29 16:25:14]

Because it is the spacing between the labels and the elements on the side.

[lpromero, 2017/03/29 16:35:47]

I don't get why this comment is here.

[gambard, 2017/03/30 06:52:24]

Well... Yes, the comment does not make sense as is.

 const CGFloat kStandardSpacing = 16;
+// When updating this, make sure to update |layoutSubviews|.

[Olivier, 2017/03/29 14:38:24]

why?

[gambard, 2017/03/29 16:25:14]

Because it is the spacing between the labels and the elements on the side.

+const CGFloat kSmallSpacing = 8;
+// Size of the favicon view.
+const CGFloat kFaviconSize = 16;
 // Size of the icon displayed when there is not image.
 const CGFloat kIconSize = 24;
 // Name of the icon displayed when there is not image.
@@ -55,6 +61,7 @@ const CGFloat kAnimationDuration = 0.3;
 @synthesize suggestionIdentifier = _suggestionIdentifier;
 @synthesize delegate = _delegate;
 @synthesize imageFetched = _imageFetched;
+@synthesize attributes = _attributes;
 
 - (instancetype)initWithType:(NSInteger)type
                        title:(NSString*)title
@@ -79,6 +86,8 @@ const CGFloat kAnimationDuration = 0.3;
     // Fetch the image. During the fetch the cell's image should still be set.
     [self.delegate loadImageForArticleItem:self];
   }
+  if (self.attributes)

[lpromero, 2017/03/29 14:33:01]

If there are no attributes, the favicon view is not reconfigured, nor
deconfigured. Does it means it can keep the configuration from a previous item
when the cell is reused?

[gambard, 2017/03/29 16:25:13]

In theory yes. But the call to self.attributes = ... is "very quick", so it
never happens (the same pattern is used in Reading List).
I have plan to fix this :)

+    [cell.faviconView configureWithAttributes:self.attributes];
   cell.titleLabel.text = self.title;
   cell.subtitleLabel.text = self.subtitle;
   [cell setContentImage:self.image];
@@ -114,6 +123,7 @@ const CGFloat kAnimationDuration = 0.3;
 @synthesize noImageIcon = _noImageIcon;
 @synthesize publisherLabel = _publisherLabel;
 @synthesize contentImageView = _contentImageView;
+@synthesize faviconView = _faviconView;
 
 - (instancetype)initWithFrame:(CGRect)frame {
   self = [super initWithFrame:frame];
@@ -123,6 +133,7 @@ const CGFloat kAnimationDuration = 0.3;
     _imageContainer = [[UIView alloc] initWithFrame:CGRectZero];
     _noImageIcon = [[UIImageView alloc] initWithFrame:CGRectZero];
     _publisherLabel = [[UILabel alloc] initWithFrame:CGRectZero];
+    _faviconView = [[FaviconViewNew alloc] init];
 
     _titleLabel.numberOfLines = 2;
     _subtitleLabel.numberOfLines = 2;
@@ -136,11 +147,13 @@ const CGFloat kAnimationDuration = 0.3;
     _titleLabel.translatesAutoresizingMaskIntoConstraints = NO;
     _subtitleLabel.translatesAutoresizingMaskIntoConstraints = NO;
     _publisherLabel.translatesAutoresizingMaskIntoConstraints = NO;
+    _faviconView.translatesAutoresizingMaskIntoConstraints = NO;
 
     [self.contentView addSubview:_imageContainer];
     [self.contentView addSubview:_titleLabel];
     [self.contentView addSubview:_subtitleLabel];
     [self.contentView addSubview:_publisherLabel];
+    [self.contentView addSubview:_faviconView];
 
     [_imageContainer addSubview:_noImageIcon];
 
@@ -154,6 +167,8 @@ const CGFloat kAnimationDuration = 0.3;
     _titleLabel.font = [MDCTypography subheadFont];
     _subtitleLabel.font = [MDCTypography body1Font];
     _publisherLabel.font = [MDCTypography captionFont];
+    _faviconView.font =
+        [[MDCTypography fontLoader] regularFontOfSize:kFaviconSize / 2];
 
     _subtitleLabel.textColor = [[MDCPalette greyPalette] tint700];
     _publisherLabel.textColor = [[MDCPalette greyPalette] tint700];
@@ -226,12 +241,33 @@ const CGFloat kAnimationDuration = 0.3;
     [_imageContainer.heightAnchor
         constraintEqualToAnchor:_imageContainer.widthAnchor],
     [_imageContainer.topAnchor constraintEqualToAnchor:_titleLabel.topAnchor],
+
+    // Publisher.
     [_publisherLabel.topAnchor
         constraintGreaterThanOrEqualToAnchor:_imageContainer.bottomAnchor
                                     constant:kStandardSpacing],
     [_publisherLabel.topAnchor
         constraintGreaterThanOrEqualToAnchor:_subtitleLabel.bottomAnchor
                                     constant:kStandardSpacing],
+    [_publisherLabel.bottomAnchor
+        constraintLessThanOrEqualToAnchor:self.contentView.bottomAnchor
+                                 constant:-kStandardSpacing],
+
+    // Favicon.
+    [_faviconView.topAnchor
+        constraintGreaterThanOrEqualToAnchor:_imageContainer.bottomAnchor
+                                    constant:kStandardSpacing],
+    [_faviconView.topAnchor
+        constraintGreaterThanOrEqualToAnchor:_subtitleLabel.bottomAnchor
+                                    constant:kStandardSpacing],
+    [_faviconView.centerYAnchor
+        constraintEqualToAnchor:_publisherLabel.centerYAnchor],
+    [_faviconView.bottomAnchor
+        constraintLessThanOrEqualToAnchor:self.contentView.bottomAnchor
+                                 constant:-kStandardSpacing],
+    [_faviconView.heightAnchor constraintEqualToConstant:kFaviconSize],

[Olivier, 2017/03/29 14:38:24]

If attributes is nil (supported), this view will still take kFaviconSize x
kFaviconSize. Is this expected?

[gambard, 2017/03/29 16:25:14]

Yes. The callback should be quick enough. I plan to fix this.

+    [_faviconView.widthAnchor
+        constraintEqualToAnchor:_faviconView.heightAnchor],
 
     // No image icon.
     [_noImageIcon.centerXAnchor
@@ -247,16 +283,17 @@ const CGFloat kAnimationDuration = 0.3;
         @"H:|-(space)-[title]-(space)-[image]-(space)-|",
         @"H:|-(space)-[text]-(space)-[image]",
         @"V:|-(space)-[title]-[text]",
-        @"H:|-(space)-[publish]-(space)-|",
-        @"V:[publish]-|",
+        @"H:|-(space)-[favicon]-(small)-[publish]-(space)-|",
       ],
       @{
         @"image" : _imageContainer,
         @"title" : _titleLabel,
         @"text" : _subtitleLabel,
         @"publish" : _publisherLabel,
+        @"favicon" : _faviconView,
       },
-      @{ @"space" : @(kStandardSpacing) });
+      @{ @"space" : @(kStandardSpacing),
+         @"small" : @(kSmallSpacing) });
 }
 
 @end