Add a footer item for the Content Suggestions

Add a footer item for the Content Suggestions

Add a footer item containg a button and an action.

BUG=700296
Review-Url: https://codereview.chromium.org/2749543002
Cr-Commit-Position: refs/heads/master@{#456747}
Committed: https://chromium.googlesource.com/chromium/src/+/c7f0d7fb68b9f6f0da1a043e4cc00b3473244514

[gambard, 2017-03-13 14:11:10]

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

[gambard, 2017-03-13 14:11:10]

PTAL.

[lpromero, 2017-03-13 14:45:14]

lgtm. Can you please add it to the material_cell_catalog?

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

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:16:
// Initialize a footer with a |title| and an |action|.
Mention that the action will be related to the button.
Note, action makes me think about target/action, i.e. a SEL (selector), and not
a block. Maybe rename in buttonBlock, or just "block" if it's clear enough?

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:18:
title:(NSString*)title
Do you prefer this style over the regular init and settable properties? The
second seems more flexible at call site.

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:19:
action:(void (^)())action NS_DESIGNATED_INITIALIZER;
Optional nit: use ProceduralBlock.

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

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm:69:
_actionButton = [UIButton buttonWithType:UIButtonTypeSystem];
No other styling? Will it be an MDCButton?

[gambard, 2017-03-14 10:02:43]

Thanks, I have added it to the cell catalog.

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

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:16:
// Initialize a footer with a |title| and an |action|.
On 2017/03/13 14:45:14, lpromero wrote:
> Mention that the action will be related to the button.
> Note, action makes me think about target/action, i.e. a SEL (selector), and
not
> a block. Maybe rename in buttonBlock, or just "block" if it's clear enough?

Done.

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:18:
title:(NSString*)title
On 2017/03/13 14:45:14, lpromero wrote:
> Do you prefer this style over the regular init and settable properties? The
> second seems more flexible at call site.

I usually do init only when the number of argument is < 5.

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:19:
action:(void (^)())action NS_DESIGNATED_INITIALIZER;
On 2017/03/13 14:45:13, lpromero wrote:
> Optional nit: use ProceduralBlock.

Done.

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

https://codereview.chromium.org/2749543002/diff/1/ios/chrome/browser/ui/conte...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm:69:
_actionButton = [UIButton buttonWithType:UIButtonTypeSystem];
On 2017/03/13 14:45:14, lpromero wrote:
> No other styling? Will it be an MDCButton?

For now I have no design for it so I go for the easy solution. It will probably
become a MDCButton.

[lpromero, 2017-03-14 10:16:19]

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

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:30:
@property(nonatomic, readonly, strong) UIButton* actionButton;
Maybe just button?

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm:24:
- (void)executeAction;
executeBlock.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:17:
// Tests that configureCell: sets the title of the button and the action.
block

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:22:
void (^action)() = ^void() {
block

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:43:
// Tests that when the cell is reused the button action is removed.
button's block or button's action.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:46:
void (^action)() = ^void() {
block

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/settings/material_cell_catalog_view_controller.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/material_cell_catalog_view_controller.mm:667:
@"is mandatory to read it! It describe how to write "
describes

[gambard, 2017-03-14 11:02:24]

Thanks, PTAL.

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

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.h:30:
@property(nonatomic, readonly, strong) UIButton* actionButton;
On 2017/03/14 10:16:18, lpromero wrote:
> Maybe just button?

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item.mm:24:
- (void)executeAction;
On 2017/03/14 10:16:19, lpromero wrote:
> executeBlock.

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:17:
// Tests that configureCell: sets the title of the button and the action.
On 2017/03/14 10:16:19, lpromero wrote:
> block

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:22:
void (^action)() = ^void() {
On 2017/03/14 10:16:19, lpromero wrote:
> block

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:43:
// Tests that when the cell is reused the button action is removed.
On 2017/03/14 10:16:19, lpromero wrote:
> button's block or button's action.

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_footer_item_unittest.mm:46:
void (^action)() = ^void() {
On 2017/03/14 10:16:19, lpromero wrote:
> block

Done.

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/settings/material_cell_catalog_view_controller.mm
(right):

https://codereview.chromium.org/2749543002/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/material_cell_catalog_view_controller.mm:667:
@"is mandatory to read it! It describe how to write "
On 2017/03/14 10:16:19, lpromero wrote:
> describes

Done.

[lpromero, 2017-03-14 11:50:42]

lgtm

[gambard, 2017-03-14 12:24:41]

The CQ bit was checked by gambard@chromium.org

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

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-14 12:31:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-14 12:31:09]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[gambard, 2017-03-14 16:07:47]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-03-14 16:07:48]

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/2749543002/#ps60001
(title: "Fix gn")

[commit-bot: I haz the power, 2017-03-14 16:08:30]

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-14 17:18:34]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489507667851120,
"parent_rev": "81e3d61f4f273627a7fb9d6d7f2bbbc382c95d7e", "commit_rev":
"c7f0d7fb68b9f6f0da1a043e4cc00b3473244514"}

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

Description was changed from

==========
Add a footer item for the Content Suggestions

Add a footer item containg a button and an action.

BUG=700296
==========

to

==========
Add a footer item for the Content Suggestions

Add a footer item containg a button and an action.

BUG=700296

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

[commit-bot: I haz the power, 2017-03-14 17:19:16]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c7f0d7fb68b9f6f0da1a043e4cc0...