Suggestions UI - favicon item

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

 // Copyright 2016 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_collection_updater.h"
 
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #import "ios/chrome/browser/ui/collection_view/collection_view_controller.h"
 #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"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
 @implementation SuggestionsCollectionUpdater
 
 @synthesize collectionViewController = _collectionViewController;
 
 #pragma mark - Properties
 
 - (void)setCollectionViewController:
     (SuggestionsViewController*)collectionViewController {
   _collectionViewController = collectionViewController;
   [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"
                            subtitle:@"This is the subtitle of an article, can "
                                     @"spawn on multiple lines"
                               image:[UIImage
                                         imageNamed:@"distillation_success"]]
         toSectionWithIdentifier:sectionIdentifier];
+
+    // Expandable Item.
     SuggestionsExpandableItem* expandableItem =
         [[SuggestionsExpandableItem alloc]
             initWithType:ItemTypeExpand
                    title:@"Title of an Expandable Article"
                 subtitle:@"This Article can be expanded to display "
                          @"additional information or interaction "
                          @"options"
                    image:[UIImage imageNamed:@"distillation_fail"]
               detailText:@"Details shown only when the article is "
                          @"expanded. It can be displayed on "
@@ skipping 34 matching lines @@
   [_collectionViewController.collectionView performBatchUpdates:^{
     [_collectionViewController.collectionView
         insertItemsAtIndexPaths:@[ [NSIndexPath
                                     indexPathForRow:numberOfItemsInSection
                                           inSection:sectionIndex] ]];
   }
                                                      completion:nil];
 }
 
 - (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