Open Reading List on tap on More

Open Reading List on tap on More

Forward the More action from the Reading List section to the BVC to let it
open the Reading List view.

BUG=702241
Review-Url: https://codereview.chromium.org/2798883002
Cr-Commit-Position: refs/heads/master@{#462153}
Committed: https://chromium.googlesource.com/chromium/src/+/fa80dd944e1937f0ab144bee0ac385d537bbd74a

[gambard, 2017-04-05 12:49:59]

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

[gambard, 2017-04-05 12:50:00]

PTAL.

[stkhapugin, 2017-04-05 13:06:03]

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm:119:
commandWithTag:IDC_SHOW_READING_LIST]];
This is ugly. Isn't it supposed to be done through coordinators somehow?

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.h
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.h:38:
@property(nonatomic, weak) id<ContentSuggestionsCommands> commandHandler;
nit: can you start adding nullability markers in this header while it's small?

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:255: if
(categoryInfo) {
nit: if (!categoryInfo) { return; } // to avoid identation

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:281:
serviceCallback);
break;

[gambard, 2017-04-05 15:30:17]

Thanks, PTAL

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm:119:
commandWithTag:IDC_SHOW_READING_LIST]];
On 2017/04/05 13:06:02, stkhapugin wrote:
> This is ugly. Isn't it supposed to be done through coordinators somehow?

Yes in bling*... For now this is the way (from marq).

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.h
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.h:38:
@property(nonatomic, weak) id<ContentSuggestionsCommands> commandHandler;
On 2017/04/05 13:06:02, stkhapugin wrote:
> nit: can you start adding nullability markers in this header while it's small?

Done.

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
(right):

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:255: if
(categoryInfo) {
On 2017/04/05 13:06:02, stkhapugin wrote:
> nit: if (!categoryInfo) { return; } // to avoid identation

Done.

https://codereview.chromium.org/2798883002/diff/1/ios/chrome/browser/content_...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:281:
serviceCallback);
On 2017/04/05 13:06:02, stkhapugin wrote:
> break;

Done.

[stkhapugin, 2017-04-05 16:47:35]

The CQ bit was checked by stkhapugin@chromium.org

[stkhapugin, 2017-04-05 16:47:36]

lgtm

Thanks for annotations! lgtm

[commit-bot: I haz the power, 2017-04-05 16:48:03]

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-04-05 18:35:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491410855826770,
"parent_rev": "0406b013e105d58c0c34fac7b853591ee584466a", "commit_rev":
"fa80dd944e1937f0ab144bee0ac385d537bbd74a"}

[commit-bot: I haz the power, 2017-04-05 18:36:24]

Description was changed from

==========
Open Reading List on tap on More

Forward the More action from the Reading List section to the BVC to let it
open the Reading List view.

BUG=702241
==========

to

==========
Open Reading List on tap on More

Forward the More action from the Reading List section to the BVC to let it
open the Reading List view.

BUG=702241

Review-Url: https://codereview.chromium.org/2798883002
Cr-Commit-Position: refs/heads/master@{#462153}
Committed:
https://chromium.googlesource.com/chromium/src/+/fa80dd944e1937f0ab144bee0ac3...
==========

[commit-bot: I haz the power, 2017-04-05 18:36:25]

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