[NTP Client] Always add space above the Signin Promo

[NTP Client] Always add space above the Signin Promo

We used to only add space between the signin promo and the preceding
item when it was not a suggestion, but since most of the time it is
not a suggestion and the message can be ambiguous when attached to
the suggestions, we now want to just always add that space.

BUG=676311
Review-Url: https://codereview.chromium.org/2613593003
Cr-Commit-Position: refs/heads/master@{#443872}
Committed: https://chromium.googlesource.com/chromium/src/+/eaa81c92de12b78ebd441547afad5664f05a8b76

[dgn, 2017-01-04 14:11:52]

dgn@chromium.org changed reviewers:
+ mvanouwerkerk@chromium.org, vitaliii@chromium.org

[dgn, 2017-01-04 14:11:54]

PTAL. Submission is also blocked on UX approval.

Preview: https://goo.gl/photos/gaRUsHpbyZVSjcQL8

[dgn, 2017-01-04 14:12:02]

The CQ bit was checked by dgn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 14:12:21]

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

[Michael van Ouwerkerk, 2017-01-04 14:37:22]

https://codereview.chromium.org/2613593003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
(right):

https://codereview.chromium.org/2613593003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java:190:
if (hasCardAbove) return R.drawable.ntp_signin_promo_card_bottom;
This would no longer be needed, would it?

[commit-bot: I haz the power, 2017-01-04 15:08:56]

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

[commit-bot: I haz the power, 2017-01-04 15:08:57]

Dry run: This issue passed the CQ dry run.

[vitaliii, 2017-01-04 15:14:28]

Ack.

-me from reviewers.

I have no idea what you are doing there :)

[vitaliii, 2017-01-04 15:15:01]

Description was changed from

==========
[NTP Client] Always add space above the Signin Promo

We used to only add space between the signin promo and the preceding
item when it was not a suggestion, but since most of the time it is
not a suggestion and the message can be ambiguous when attached to
the suggestions, we now want to just always add that space.

BUG=676311
==========

to

==========
[NTP Client] Always add space above the Signin Promo

We used to only add space between the signin promo and the preceding
item when it was not a suggestion, but since most of the time it is
not a suggestion and the message can be ambiguous when attached to
the suggestions, we now want to just always add that space.

BUG=676311
==========

[vitaliii, 2017-01-04 15:15:01]

vitaliii@chromium.org changed reviewers:
- vitaliii@chromium.org

[dgn, 2017-01-04 18:31:55]

https://codereview.chromium.org/2613593003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
(right):

https://codereview.chromium.org/2613593003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java:190:
if (hasCardAbove) return R.drawable.ntp_signin_promo_card_bottom;
On 2017/01/04 14:37:21, Michael van Ouwerkerk wrote:
> This would no longer be needed, would it?

Indeed, thanks. It also means that we need to make the suggestions think that
there is no card coming after it to use the right 9-patch (TBD)

[dgn, 2017-01-13 20:54:02]

The CQ bit was checked by dgn@chromium.org to run a CQ dry run

[dgn, 2017-01-13 20:54:21]

PTAL, the changes have been UX approved

[commit-bot: I haz the power, 2017-01-13 20:54:41]

Dry run: 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-01-13 21:42:43]

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

[commit-bot: I haz the power, 2017-01-13 21:42:44]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2017-01-16 11:22:28]

lgtm

[dgn, 2017-01-16 11:38:00]

The CQ bit was checked by dgn@chromium.org

[commit-bot: I haz the power, 2017-01-16 11:38: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-01-16 12:09:11]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484566680832930,
"parent_rev": "27ecb94bbf93d0324fd74ca1f689e806cae2df51", "commit_rev":
"eaa81c92de12b78ebd441547afad5664f05a8b76"}

[commit-bot: I haz the power, 2017-01-16 12:09:44]

Description was changed from

==========
[NTP Client] Always add space above the Signin Promo

We used to only add space between the signin promo and the preceding
item when it was not a suggestion, but since most of the time it is
not a suggestion and the message can be ambiguous when attached to
the suggestions, we now want to just always add that space.

BUG=676311
==========

to

==========
[NTP Client] Always add space above the Signin Promo

We used to only add space between the signin promo and the preceding
item when it was not a suggestion, but since most of the time it is
not a suggestion and the message can be ambiguous when attached to
the suggestions, we now want to just always add that space.

BUG=676311

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

[commit-bot: I haz the power, 2017-01-16 12:09:45]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/eaa81c92de12b78ebd441547afad...

[Wez, 2017-02-07 19:54:21]

wez@chromium.org changed reviewers:
+ bauerb@chromium.org

[Wez, 2017-02-07 19:54:22]

Hallo bauerb@chromium.org!
Due to a depot_tools patch which mistakenly removed the OWNERS check for
non-source files (see crbug.com/684270), the following files landed in this CL
and need a retrospective review from you:
	components/ntp_snippets_strings.grdp
Thanks,
Wez

[Bernhard Bauer, 2017-03-27 09:31:56]

lgtm