Suggestions UI - favicon item

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

Index: ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm
diff --git a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm b/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm
index 272ef9833d841acbfa19625573d10ac592d7dd41..1400e88cd5ba28345e2667edc4a3832b7763e0fc 100644
--- a/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm
+++ b/ios/chrome/browser/ui/suggestions/suggestions_collection_updater.mm
@@ -10,6 +10,7 @@
 #import "ios/chrome/browser/ui/collection_view/collection_view_model.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_article_item.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_expandable_item.h"
+#import "ios/chrome/browser/ui/suggestions/suggestions_favicon_item.h"

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

Should it be SuggestionsFaviconsItem?

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

In all class names favicon is used. So I followed the trend :)

 #import "ios/chrome/browser/ui/suggestions/suggestions_item.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_stack_item.h"
 #import "ios/chrome/browser/ui/suggestions/suggestions_view_controller.h"
@@ -30,18 +31,35 @@
   [collectionViewController loadModel];
   CollectionViewModel* model = collectionViewController.collectionViewModel;
   NSInteger sectionIdentifier = kSectionIdentifierEnumZero;
+
+  // Stack Item.
   [model addSectionWithIdentifier:sectionIdentifier];
   [model addItem:[[SuggestionsStackItem alloc] initWithType:ItemTypeStack
                                                       title:@"The title"
                                                    subtitle:@"The subtitle"]
       toSectionWithIdentifier:sectionIdentifier++];

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

I am not a fan of using incrementing sectionIdentifier for static lists of
sections like here. Let say you conditionally add or not the favicon section.
You have no way to know the section identifier of the section that holds the
standard, article and expandable items.

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

I know, but once again, this is for demo :)
The content of this function should be completely removed once we use a mediator
to pipe the data in. I am waiting to see the interface with the data providers
to construct a meaningful mediator.
Once the mediator is built, the data will come from it and the ItemType and the
SectionIdentifier will (hopefully) be more meaningful.

 
+  // Favicon Item.
+  [model addSectionWithIdentifier:sectionIdentifier];
+  SuggestionsFaviconItem* faviconItem =
+      [[SuggestionsFaviconItem alloc] initWithType:ItemTypeFavicon];
+  for (NSInteger i = 0; i < 6; i++) {
+    [faviconItem addFavicon:[UIImage imageNamed:@"bookmark_gray_star"]
+                  withTitle:@"Super website! Incredible!"];
+  }
+  faviconItem.delegate = _collectionViewController;
+  [model addItem:faviconItem toSectionWithIdentifier:sectionIdentifier++];
+
   for (NSInteger i = 0; i < 3; i++) {
     [model addSectionWithIdentifier:sectionIdentifier];
+
+    // Standard Item.
     [model addItem:[[SuggestionsItem alloc] initWithType:ItemTypeText
                                                    title:@"The title"
                                                 subtitle:@"The subtitle"]
         toSectionWithIdentifier:sectionIdentifier];
+
+    // Article Item.
     [model addItem:[[SuggestionsArticleItem alloc]
                        initWithType:ItemTypeArticle
                               title:@"Title of an Article"
@@ -50,6 +68,8 @@
                               image:[UIImage
                                         imageNamed:@"distillation_success"]]
         toSectionWithIdentifier:sectionIdentifier];
+
+    // Expandable Item.
     SuggestionsExpandableItem* expandableItem =
         [[SuggestionsExpandableItem alloc]
             initWithType:ItemTypeExpand
@@ -104,7 +124,7 @@
 }
 
 - (BOOL)shouldUseCustomStyleForSection:(NSInteger)section {
-  return section == 0;
+  return section == 0 || section == 1;

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

Here you should translate to sectionIdentifier and then check for the particular
sectionIdentifier value. That's where having a static enum for sections is
useful. In the example of the conditionally added favicon section, this would
break here, because in the case of the favicon section not added, you shouldn't
return YES for section 1.

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

Same as the other comment.
This will be rewritten once the mediator comes in.

 }
 
 @end