📰 Remove whitespace between card title and attribution

[NTP Client] Remove whitespace between card title and attribution

Allows the attribution line to move up beside the thumbnail and below
the title when there is not description

BUG=690420, 631447
Review-Url: https://codereview.chromium.org/2690123002
Cr-Commit-Position: refs/heads/master@{#450933}
Committed: https://chromium.googlesource.com/chromium/src/+/3371166ee6b13e7d34d3a8a216bf041abc3ce9b9

[dgn, 2017-02-13 16:15:49]

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

[dgn, 2017-02-13 16:15:50]

PTAL. Posted preview on the bug.

[dgn, 2017-02-13 16:15:58]

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

[commit-bot: I haz the power, 2017-02-13 16:16:23]

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-02-13 16:48:42]

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

[commit-bot: I haz the power, 2017-02-13 16:48:43]

Dry run: This issue passed the CQ dry run.

[vitaliii, 2017-02-14 07:20:32]

Description was changed from

==========
[NTP Client] Remove whitespace between card title and attribution

Allows the attribution line to move up beside the thumbnail and below
the title when there is not description

BUG=690420,631447
==========

to

==========
[NTP Client] Remove whitespace between card title and attribution

Allows the attribution line to move up beside the thumbnail and below
the title when there is not description

BUG=690420,631447
==========

[dgn, 2017-02-14 18:10:31]

FYI: CL not for review at the moment, as it always pushes the download icon at
the end of the line. I'm currently experimenting with other layouts (grid, etc)

[dgn, 2017-02-15 16:53:32]

PTAL

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
(left):

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:80:
private final boolean mUseFaviconService;
moved the final members together

[dgn, 2017-02-15 16:56:24]

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

[dgn, 2017-02-15 17:02:49]

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

[dgn, 2017-02-15 17:02:50]

+Michael for review since we discussed this.

[dgn, 2017-02-15 17:02:57]

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

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

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

[Bernhard Bauer, 2017-02-15 17:47:51]

lgtm

[commit-bot: I haz the power, 2017-02-15 17:58:16]

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

[commit-bot: I haz the power, 2017-02-15 17:58:17]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2017-02-15 17:59:33]

Nice! lgtm with nits

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/new_tab_page_snippets_card.xml (right):

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/new_tab_page_snippets_card.xml:94:
android:layout_marginStart="@dimen/snippets_thumbnail_padding"
nit: should the dimension be renamed to "snippets_thumbnail_margin"?

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
(right):

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:77:
/** Total horizontal space occupied by the thumbnail, sum of its size and
padding. */
"size and margin"?

[dgn, 2017-02-15 18:15:56]

Thanks!

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/new_tab_page_snippets_card.xml (right):

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/new_tab_page_snippets_card.xml:94:
android:layout_marginStart="@dimen/snippets_thumbnail_padding"
On 2017/02/15 17:59:33, Michael van Ouwerkerk wrote:
> nit: should the dimension be renamed to "snippets_thumbnail_margin"?

Done.

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
(right):

https://codereview.chromium.org/2690123002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:77:
/** Total horizontal space occupied by the thumbnail, sum of its size and
padding. */
On 2017/02/15 17:59:33, Michael van Ouwerkerk wrote:
> "size and margin"?

Done.

[dgn, 2017-02-15 18:16:00]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-02-15 18:16:00]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2690123002/#ps60001
(title: "address comments")

[commit-bot: I haz the power, 2017-02-15 18:16:42]

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-02-15 19:51:33]

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

[commit-bot: I haz the power, 2017-02-15 19:51:34]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[dgn, 2017-02-16 10:23:31]

The CQ bit was checked by dgn@chromium.org

[commit-bot: I haz the power, 2017-02-16 10:24:12]

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

[dgn, 2017-02-16 10:42:43]

The CQ bit was unchecked by dgn@chromium.org

[dgn, 2017-02-16 10:52:11]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-02-16 10:52:11]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2690123002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-16 10:52:43]

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-02-16 11:34:07]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487242331358480,
"parent_rev": "4ea35f1d6c27d519aeb7ced90692059aeb131569", "commit_rev":
"3371166ee6b13e7d34d3a8a216bf041abc3ce9b9"}

[commit-bot: I haz the power, 2017-02-16 11:34:48]

Description was changed from

==========
[NTP Client] Remove whitespace between card title and attribution

Allows the attribution line to move up beside the thumbnail and below
the title when there is not description

BUG=690420,631447
==========

to

==========
[NTP Client] Remove whitespace between card title and attribution

Allows the attribution line to move up beside the thumbnail and below
the title when there is not description

BUG=690420,631447

Review-Url: https://codereview.chromium.org/2690123002
Cr-Commit-Position: refs/heads/master@{#450933}
Committed:
https://chromium.googlesource.com/chromium/src/+/3371166ee6b13e7d34d3a8a216bf...
==========

[commit-bot: I haz the power, 2017-02-16 11:34:49]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/3371166ee6b13e7d34d3a8a216bf...