Suggestions UI - Add Data Source and Item

ios/chrome/browser/ui/suggestions/suggestions_data_source.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_data_source.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_item.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+typedef NS_ENUM(NSInteger, ItemType) {
+  ItemTypeText = kItemTypeEnumZero,

[marq (ping after 24h), 2017/01/11 13:43:09]

Chromium style is to use all caps ("MACRO_STYLE") for enum values -- see
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md....

[lpromero, 2017/01/11 13:46:05]

This is outside the scope of this CL, as this is the style used pervasively for
ItemType.

[gambard, 2017/01/11 15:10:38]

Acknowledged.

+  ItemTypeArticle,
+  ItemTypeExpand,
+};
+
+}  // namespace
+
+@implementation SuggestionsDataSource {
+  NSMutableArray<NSMutableArray<CollectionViewItem*>*>* _items;

[lpromero, 2017/01/11 09:47:04]

No action needed: Hmm, you work around the fact that CollectionViewController's
collectionViewModel is readonly? I wonder if we should make it settable (and
remove the loadModel method).

[gambard, 2017/01/11 15:10:38]

This was to allow the addition of items before setting the
CollectionViewController. I removed it as I force a CollectionViewController to
be set in the init.

+}
+
+@synthesize collectionViewController = _collectionViewController;
+
+- (instancetype)init {
+  self = [super init];
+  if (self) {
+    _items = [NSMutableArray arrayWithCapacity:1];

[lpromero, 2017/01/11 09:47:04]

I usually don't try to give clues like that, especially in your case where this
array can get easily more objects. I just use [NSMutableArray array].

[gambard, 2017/01/11 15:10:38]

Done.

+    for (NSInteger i = 0; i < 3; i++) {
+      NSMutableArray<CollectionViewItem*>* arrayToAdd =
+          [NSMutableArray arrayWithCapacity:3];
+      [arrayToAdd
+          addObject:[[SuggestionsItem alloc] initWithType:ItemTypeText
+                                                    title:@"The title"
+                                                 subtitle:@"The subtitle"]];

[lpromero, 2017/01/11 09:47:04]

Maybe remove this bogus data?

[gambard, 2017/01/11 15:10:37]

Why? This is still a demo. It shows what kind of items can be displayed.

+      [_items addObject:arrayToAdd];
+    }
+  }
+  return self;
+}
+
+#pragma mark - Properties
+
+- (void)setCollectionViewController:
+    (CollectionViewController*)collectionViewController {
+  _collectionViewController = collectionViewController;
+  if (!collectionViewController) {
+    return;
+  }
+  [collectionViewController loadModel];
+  CollectionViewModel* model = collectionViewController.collectionViewModel;
+  NSInteger sectionIdentifier = kSectionIdentifierEnumZero;
+  for (NSMutableArray<CollectionViewItem*>* arrayWithItems in _items) {
+    [model addSectionWithIdentifier:sectionIdentifier];
+    for (CollectionViewItem* item in arrayWithItems) {
+      [model addItem:item toSectionWithIdentifier:sectionIdentifier];
+    }
+    sectionIdentifier++;
+  }
+}
+
+#pragma mark - Public methods
+
+- (void)addTextItem:(NSString*)title
+           subtitle:(NSString*)subtitle
+          toSection:(NSUInteger)section {
+  SuggestionsItem* item = [[SuggestionsItem alloc] initWithType:ItemTypeText
+                                                          title:title
+                                                       subtitle:subtitle];
+  NSInteger sectionIdentifier = kSectionIdentifierEnumZero + section;
+  NSInteger sectionIndex = section;
+  if (section >= [_items count]) {
+    sectionIndex = [_items count];
+    sectionIdentifier = kSectionIdentifierEnumZero + section;
+    [_items addObject:[NSMutableArray arrayWithObject:item]];
+    if (_collectionViewController) {
+      [_collectionViewController.collectionView performBatchUpdates:^{
+        [_collectionViewController.collectionViewModel
+            addSectionWithIdentifier:sectionIdentifier];
+        [_collectionViewController.collectionView
+            insertSections:[NSIndexSet indexSetWithIndex:sectionIndex]];
+      }
+                                                         completion:^(BOOL){

[lpromero, 2017/01/11 09:47:04]

Can't you just pass nil?

[gambard, 2017/01/11 15:10:38]

Done.

+                                                         }];
+    }
+  }
+  [[_items objectAtIndex:section] addObject:item];

[lpromero, 2017/01/11 09:47:04]

Isn't this already done line 80? Plus, if section was more than _items.count,
there is no object at that index right?

[gambard, 2017/01/11 15:10:37]

Acknowledged.

[lpromero, 2017/01/11 18:54:43]

I don't understand how this doesn't crash if you pass a section that is 10 more
than the initial count for example.

[gambard, 2017/01/12 08:46:16]

If you look at line 59 of the patch 5, there is if ([model numberOfSections] <=
inputSection) which adds a new section and changes the section index to the one
of the new section.

[lpromero, 2017/01/12 11:43:35]

Fair enough.

+  if (_collectionViewController) {
+    [_collectionViewController.collectionViewModel addItem:item
+                                   toSectionWithIdentifier:sectionIdentifier];
+    [_collectionViewController.collectionView performBatchUpdates:^{
+      [_collectionViewController.collectionView insertItemsAtIndexPaths:@[
+        [NSIndexPath indexPathForRow:[[_items objectAtIndex:section] count] - 1
+                           inSection:section]
+      ]];
+    }
+                                                       completion:^(BOOL){

[lpromero, 2017/01/11 09:47:04]

Idem, can you pass nil?

[gambard, 2017/01/11 15:10:37]

Done.

+                                                       }];
+  }
+}
+
+@end