ContentSuggestions collection adds items with animation

ContentSuggestions collection adds items with animation

This CL changes the ContentSuggestions collection to add items with an animation
instead of reloading the entire collection.

BUG=699124, 700296
Review-Url: https://codereview.chromium.org/2741343005
Cr-Commit-Position: refs/heads/master@{#457083}
Committed: https://chromium.googlesource.com/chromium/src/+/3a87cbc4fe0ad88df511929517797a10b9879361

[gambard, 2017-03-14 15:53:06]

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

[gambard, 2017-03-14 15:53:07]

PTAL.

[lpromero, 2017-03-14 16:15:11]

Description was changed from

==========
ContentSuggestions collection add items with animation

This CL changes the ContentSuggestions collection to add items with an animation
instead of reloading the entiere collection.

BUG=699124, 700296
==========

to

==========
ContentSuggestions collection adds items with animation

This CL changes the ContentSuggestions collection to add items with an animation
instead of reloading the entire collection.

BUG=699124, 700296
==========

[lpromero, 2017-03-14 16:23:02]

lgtm

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
(right):

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:38:
- (NSArray<NSIndexPath*>*)addSuggestionsToModel:
Either remove "toModel" here or add it to the second method. Would it make sense
to move the second method above this one?

If someone calls -addSuggestionsToModel:, does it create the sections if they
don't exist? Or does one ensures first to call -addSectionsForSuggestions:?

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h
(right):

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h:43:
// Add the |suggestions| to the collection and its model.
Adds

[gambard, 2017-03-15 10:03:42]

Thanks!

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
(right):

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:38:
- (NSArray<NSIndexPath*>*)addSuggestionsToModel:
On 2017/03/14 16:23:01, lpromero wrote:
> Either remove "toModel" here or add it to the second method. Would it make
sense
> to move the second method above this one?
> 
> If someone calls -addSuggestionsToModel:, does it create the sections if they
> don't exist? Or does one ensures first to call -addSectionsForSuggestions:?

You should call -addSectionForSuggestions: first. I have added a comment.
Added ToModel to the method name.

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h
(right):

https://codereview.chromium.org/2741343005/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h:43:
// Add the |suggestions| to the collection and its model.
On 2017/03/14 16:23:01, lpromero wrote:
> Adds

Done.

[lpromero, 2017-03-15 10:28:34]

Nits. Still lgtm

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
(right):

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:37:
// Adds the sections for the |suggestions| to the model and returns their index.
their indices

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:38:
// The caller must ensure the corresponding sections have been added to the
Should this comment be added to the other method?

[gambard, 2017-03-15 14:18:28]

Thanks

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
(right):

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:37:
// Adds the sections for the |suggestions| to the model and returns their index.
On 2017/03/15 10:28:34, lpromero wrote:
> their indices

Done.

https://codereview.chromium.org/2741343005/diff/60001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h:38:
// The caller must ensure the corresponding sections have been added to the
On 2017/03/15 10:28:34, lpromero wrote:
> Should this comment be added to the other method?

Ooops... Done!

[gambard, 2017-03-15 15:01:49]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-03-15 15:01:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2741343005/#ps100001
(title: "Rebase on correct head")

[commit-bot: I haz the power, 2017-03-15 15:02:22]

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-15 15:14:11]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1489590109868450,
"parent_rev": "8fc9138646abfd18c2b3b15c045f1d0cb9a1628e", "commit_rev":
"3a87cbc4fe0ad88df511929517797a10b9879361"}

[commit-bot: I haz the power, 2017-03-15 15:14:55]

Description was changed from

==========
ContentSuggestions collection adds items with animation

This CL changes the ContentSuggestions collection to add items with an animation
instead of reloading the entire collection.

BUG=699124, 700296
==========

to

==========
ContentSuggestions collection adds items with animation

This CL changes the ContentSuggestions collection to add items with an animation
instead of reloading the entire collection.

BUG=699124, 700296

Review-Url: https://codereview.chromium.org/2741343005
Cr-Commit-Position: refs/heads/master@{#457083}
Committed:
https://chromium.googlesource.com/chromium/src/+/3a87cbc4fe0ad88df51192951779...
==========

[commit-bot: I haz the power, 2017-03-15 15:14:56]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/3a87cbc4fe0ad88df51192951779...