[Autofill] Tweak credit card signin promo visually.

[Autofill] Tweak credit card signin promo visually.

Smaller font and light grey background.

BUG=626442
TEST=manual

Committed: https://crrev.com/3fc9f535f390b2d82d7308af7c3611cff248dc3a
Cr-Commit-Position: refs/heads/master@{#406070}

[Mathieu, 2016-07-15 15:44:01]

mathp@chromium.org changed reviewers:
+ estade@chromium.org

[Mathieu, 2016-07-15 15:44:02]

Hi Evan, PTAL, thanks!

[Mathieu, 2016-07-15 15:48:20]

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

[commit-bot: I haz the power, 2016-07-15 15:48:56]

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, 2016-07-15 17:10:28]

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

[commit-bot: I haz the power, 2016-07-15 17:10:29]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-07-15 17:27:08]

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_base_view.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_base_view.cc:23: const SkColor
AutofillPopupBaseView::kItemTextColor =
s/Item/Label (?)

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:77:
canvas->FillRect(entry_rect, kPromoPopupBackground);
can you move this color logic to the controller?

[Mathieu, 2016-07-15 19:29:14]

Patchset #2 (id:20001) has been deleted

[Mathieu, 2016-07-15 19:30:13]

Patchset #2 (id:40001) has been deleted

[Mathieu, 2016-07-15 19:30:20]

Hi, PTAL?

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_base_view.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_base_view.cc:23: const SkColor
AutofillPopupBaseView::kItemTextColor =
On 2016/07/15 17:27:08, Evan Stade wrote:
> s/Item/Label (?)

Done.

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:77:
canvas->FillRect(entry_rect, kPromoPopupBackground);
On 2016/07/15 17:27:08, Evan Stade wrote:
> can you move this color logic to the controller?

Done.

[Evan Stade, 2016-07-15 19:33:38]

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:77:
canvas->FillRect(entry_rect, kPromoPopupBackground);
On 2016/07/15 19:30:20, Mathieu Perreault wrote:
> On 2016/07/15 17:27:08, Evan Stade wrote:
> > can you move this color logic to the controller?
> 
> Done.

it seems to still be here. Also I had intended for the hovered bg color logic to
move as well.

[Mathieu, 2016-07-18 13:48:02]

Thanks, have a look!

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/1/chrome/browser/ui/views/aut...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:77:
canvas->FillRect(entry_rect, kPromoPopupBackground);
On 2016/07/15 19:33:38, Evan Stade wrote:
> On 2016/07/15 19:30:20, Mathieu Perreault wrote:
> > On 2016/07/15 17:27:08, Evan Stade wrote:
> > > can you move this color logic to the controller?
> > 
> > Done.
> 
> it seems to still be here. Also I had intended for the hovered bg color logic
to
> move as well.

Sorry, moved the logic now.

https://codereview.chromium.org/2152243002/diff/80001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/popup_constants.h (right):

https://codereview.chromium.org/2152243002/diff/80001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/popup_constants.h:16: constexpr SkColor kBorderColor
= SkColorSetRGB(0xC7, 0xCA, 0xCE);
Note: According to https://codereview.chromium.org/1885183002/, "const variables
of integral type with initializer already are constexpr", but the discussion
there seems to say that it's best form to put it anyway.

[Mathieu, 2016-07-18 13:58:11]

Patchset #3 (id:80001) has been deleted

[Mathieu, 2016-07-18 15:45:02]

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

[commit-bot: I haz the power, 2016-07-18 15:45:29]

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, 2016-07-18 17:24:20]

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

[commit-bot: I haz the power, 2016-07-18 17:24:21]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-07-18 17:52:57]

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:433: } else if
(suggestions_[index].frontend_id ==
nit: no else after return

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:438: return
SK_ColorWHITE;
this is redundant with kPopupBackground. You should probably return
SK_ColorTRANSPARENT

[Mathieu, 2016-07-18 18:01:41]

Thanks, PTAL

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:433: } else if
(suggestions_[index].frontend_id ==
On 2016/07/18 17:52:56, Evan Stade wrote:
> nit: no else after return

Done.

https://codereview.chromium.org/2152243002/diff/100001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:438: return
SK_ColorWHITE;
On 2016/07/18 17:52:56, Evan Stade wrote:
> this is redundant with kPopupBackground. You should probably return
> SK_ColorTRANSPARENT

Done.

[Evan Stade, 2016-07-18 18:13:08]

lgtm

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/auto...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:431: if (index ==
selected_line_) {
nit: no curlies

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:75: // at |index|.
nit: comment is unnecessary

[Mathieu, 2016-07-18 18:45:13]

Submitting...

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/auto...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/auto...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:431: if (index ==
selected_line_) {
On 2016/07/18 18:13:08, Evan Stade wrote:
> nit: no curlies

Done.

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/autofill/autofill_popup_view_views.cc (right):

https://codereview.chromium.org/2152243002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/autofill/autofill_popup_view_views.cc:75: // at |index|.
On 2016/07/18 18:13:08, Evan Stade wrote:
> nit: comment is unnecessary

Done.

[Mathieu, 2016-07-18 18:45:37]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-07-18 18:45:37]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2152243002/#ps140001
(title: "no curlies")

[commit-bot: I haz the power, 2016-07-18 18:46:27]

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, 2016-07-18 19:47:36]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2016-07-18 19:47:48]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-18 19:48:59]

Description was changed from

==========
[Autofill] Tweak credit card signin promo visually.

Smaller font and light grey background.

BUG=626442
TEST=manual
==========

to

==========
[Autofill] Tweak credit card signin promo visually.

Smaller font and light grey background.

BUG=626442
TEST=manual

Committed: https://crrev.com/3fc9f535f390b2d82d7308af7c3611cff248dc3a
Cr-Commit-Position: refs/heads/master@{#406070}
==========

[commit-bot: I haz the power, 2016-07-18 19:49:00]

Patchset 5 (id:??) landed as
https://crrev.com/3fc9f535f390b2d82d7308af7c3611cff248dc3a
Cr-Commit-Position: refs/heads/master@{#406070}