Add header for the Content Suggestions

Add header for the Content Suggestions

This CL adds headers for the Content Suggestions sections.

BUG=702236
Review-Url: https://codereview.chromium.org/2751653008
Cr-Commit-Position: refs/heads/master@{#457738}
Committed: https://chromium.googlesource.com/chromium/src/+/663bdb216fc90132e85a40649d8c04c62cbf29bb

[gambard, 2017-03-16 15:49:40]

gambard@chromium.org changed reviewers:
+ lpromero@chromium.org

[gambard, 2017-03-16 15:49:41]

PTAL.

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm:174:
[self.collectionViewController.collectionView reloadData];
This is a bug fix, appearing only when you add header.

[lpromero, 2017-03-16 16:43:21]

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm:174:
[self.collectionViewController.collectionView reloadData];
On 2017/03/16 15:49:41, gambard wrote:
> This is a bug fix, appearing only when you add header.

Can you explain more on the issue in a comment in the code?

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm:255:
[self addHeader:suggestion.suggestionIdentifier.sectionInfo];
Optional nit: move one line up?

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.h
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.h:11:
@interface ContentSuggestionsHeaderItem
Can you add a comment? I am not a fan of subclassing CollectionViewTextItem.
Where do you use the suggestionIdentifier you are strapping to this item? Since
it is a header and not a suggestion, how come it even can have a suggestion
identifier?

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.mm
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.mm:13:
@synthesize suggestionIdentifier = _suggestionIdentifier;
Can you add a comment like:

// Do not add more functionality to this class. If needed, please create a new
subclass of CollectionViewItem instead of CollectionViewTextItem.

[gambard, 2017-03-17 09:33:39]

Thanks, PTAL.

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm:174:
[self.collectionViewController.collectionView reloadData];
On 2017/03/16 16:43:21, lpromero wrote:
> On 2017/03/16 15:49:41, gambard wrote:
> > This is a bug fix, appearing only when you add header.
> 
> Can you explain more on the issue in a comment in the code?

Done.

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm:255:
[self addHeader:suggestion.suggestionIdentifier.sectionInfo];
On 2017/03/16 16:43:21, lpromero wrote:
> Optional nit: move one line up?

Done.

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.h
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.h:11:
@interface ContentSuggestionsHeaderItem
On 2017/03/16 16:43:21, lpromero wrote:
> Can you add a comment? I am not a fan of subclassing CollectionViewTextItem.
> Where do you use the suggestionIdentifier you are strapping to this item?
Since
> it is a header and not a suggestion, how come it even can have a suggestion
> identifier?

Done.

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.mm
(right):

https://codereview.chromium.org/2751653008/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_header_item.mm:13:
@synthesize suggestionIdentifier = _suggestionIdentifier;
On 2017/03/16 16:43:21, lpromero wrote:
> Can you add a comment like:
> 
> // Do not add more functionality to this class. If needed, please create a new
> subclass of CollectionViewItem instead of CollectionViewTextItem.

Acknowledged.

[lpromero, 2017-03-17 12:03:58]

The CQ bit was checked by lpromero@chromium.org

[lpromero, 2017-03-17 12:03:59]

lgtm

[commit-bot: I haz the power, 2017-03-17 12:04:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 12:21:16]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489752238532470,
"parent_rev": "e46faca0fb01521d83c4a5614c3692477dd01faa", "commit_rev":
"663bdb216fc90132e85a40649d8c04c62cbf29bb"}

[commit-bot: I haz the power, 2017-03-17 12:21:59]

Description was changed from

==========
Add header for the Content Suggestions

This CL adds headers for the Content Suggestions sections.

BUG=702236
==========

to

==========
Add header for the Content Suggestions

This CL adds headers for the Content Suggestions sections.

BUG=702236

Review-Url: https://codereview.chromium.org/2751653008
Cr-Commit-Position: refs/heads/master@{#457738}
Committed:
https://chromium.googlesource.com/chromium/src/+/663bdb216fc90132e85a40649d8c...
==========

[commit-bot: I haz the power, 2017-03-17 12:22:00]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/663bdb216fc90132e85a40649d8c...