[Snowflake] Unify toolbar text styles

[Snowflake] Unify toolbar text styles

+ Change omnibox/ntp search hint text color to 54% black
+ Change suggestion view list top and bottom padding to 8dp

BUG=723042
Review-Url: https://codereview.chromium.org/2930713002
Cr-Commit-Position: refs/heads/master@{#478833}
Committed: https://chromium.googlesource.com/chromium/src/+/57f1f0bb48b70875fa118be2266104b7249db921

[huayinz, 2017-06-07 16:50:00]

huayinz@chromium.org changed reviewers:
+ twellington@chromium.org

[huayinz, 2017-06-07 16:50:01]

twellington@ - Hi could you take a look? Thanks!

[Theresa, 2017-06-08 16:12:23]

twellington@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Theresa, 2017-06-08 16:12:24]

https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
File chrome/android/java/res/values/dimens.xml (right):

https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/dimens.xml:278: <dimen
name="omnibox_suggestion_list_padding_bottom">8dp</dimen>
These also exist in values-sw600dp/dimens.xml; I having trouble tracing the
origins of why we use 0dp here but 8dp in sw600dp for tablets due to upstreaming
a couple of years ago.

+tedchoc@ - do you recall (or know someone who may recall) why the suggestions
padding is different for phones and tablets?

[Ted C, 2017-06-08 16:37:39]

https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
File chrome/android/java/res/values/dimens.xml (right):

https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/dimens.xml:278: <dimen
name="omnibox_suggestion_list_padding_bottom">8dp</dimen>
On 2017/06/08 16:12:24, Theresa (OOO) wrote:
> These also exist in values-sw600dp/dimens.xml; I having trouble tracing the
> origins of why we use 0dp here but 8dp in sw600dp for tablets due to
upstreaming
> a couple of years ago.
> 
> +tedchoc@ - do you recall (or know someone who may recall) why the suggestions
> padding is different for phones and tablets?

I suspect that dates back to the original material design change in chrome.  I
would guess that the tablet is a dropdown and the phone is a full sheet so maybe
this padding was just unnecessary?  We might end up getting a scrollbar
unnecessarily if it barely just fit where the padding overflows past the
keyboard.

If we have screenshots with 1 suggestion and a full set of suggestions, and ux
signs off on it, then they've updated their specs.

Legacy code review.
https://chrome-internal-review.googlesource.com/c/172055/

[huayinz, 2017-06-12 21:37:47]

On 2017/06/08 16:37:39, Ted C wrote:
>
https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
> File chrome/android/java/res/values/dimens.xml (right):
> 
>
https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
> chrome/android/java/res/values/dimens.xml:278: <dimen
> name="omnibox_suggestion_list_padding_bottom">8dp</dimen>
> On 2017/06/08 16:12:24, Theresa (OOO) wrote:
> > These also exist in values-sw600dp/dimens.xml; I having trouble tracing the
> > origins of why we use 0dp here but 8dp in sw600dp for tablets due to
> upstreaming
> > a couple of years ago.
> > 
> > +tedchoc@ - do you recall (or know someone who may recall) why the
suggestions
> > padding is different for phones and tablets?
> 
> I suspect that dates back to the original material design change in chrome.  I
> would guess that the tablet is a dropdown and the phone is a full sheet so
maybe
> this padding was just unnecessary?  We might end up getting a scrollbar
> unnecessarily if it barely just fit where the padding overflows past the
> keyboard.
> 
> If we have screenshots with 1 suggestion and a full set of suggestions, and ux
> signs off on it, then they've updated their specs.
> 
> Legacy code review.
> https://chrome-internal-review.googlesource.com/c/172055/


See https://bugs.chromium.org/p/chromium/issues/detail?id=723042#c19

[Theresa, 2017-06-12 21:41:55]

On 2017/06/12 21:37:47, huayinz wrote:
> On 2017/06/08 16:37:39, Ted C wrote:
> >
>
https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
> > File chrome/android/java/res/values/dimens.xml (right):
> > 
> >
>
https://codereview.chromium.org/2930713002/diff/1/chrome/android/java/res/val...
> > chrome/android/java/res/values/dimens.xml:278: <dimen
> > name="omnibox_suggestion_list_padding_bottom">8dp</dimen>
> > On 2017/06/08 16:12:24, Theresa (OOO) wrote:
> > > These also exist in values-sw600dp/dimens.xml; I having trouble tracing
the
> > > origins of why we use 0dp here but 8dp in sw600dp for tablets due to
> > upstreaming
> > > a couple of years ago.
> > > 
> > > +tedchoc@ - do you recall (or know someone who may recall) why the
> suggestions
> > > padding is different for phones and tablets?
> > 
> > I suspect that dates back to the original material design change in chrome. 
I
> > would guess that the tablet is a dropdown and the phone is a full sheet so
> maybe
> > this padding was just unnecessary?  We might end up getting a scrollbar
> > unnecessarily if it barely just fit where the padding overflows past the
> > keyboard.
> > 
> > If we have screenshots with 1 suggestion and a full set of suggestions, and
ux
> > signs off on it, then they've updated their specs.
> > 
> > Legacy code review.
> > https://chrome-internal-review.googlesource.com/c/172055/
> 
> 
> See https://bugs.chromium.org/p/chromium/issues/detail?id=723042#c19

Thank you for checking with UX. We should remove these two dimens from
values-sw600dp/dimens.xml now that they're the same in both files.

[huayinz, 2017-06-12 22:51:38]

removed redundant dimens in values-sw600dp

[Theresa, 2017-06-12 22:53:21]

lgtm

[huayinz, 2017-06-12 22:54:52]

The CQ bit was checked by huayinz@chromium.org

[commit-bot: I haz the power, 2017-06-12 22:55:25]

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-06-13 00:14:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497308092809770,
"parent_rev": "4d2b95b2bf6bf09c38f452ba921454a234423524", "commit_rev":
"57f1f0bb48b70875fa118be2266104b7249db921"}

[commit-bot: I haz the power, 2017-06-13 00:14:48]

Description was changed from

==========
[Snowflake] Unify toolbar text styles

+ Change omnibox/ntp search hint text color to 54% black
+ Change suggestion view list top and bottom padding to 8dp

BUG=723042
==========

to

==========
[Snowflake] Unify toolbar text styles

+ Change omnibox/ntp search hint text color to 54% black
+ Change suggestion view list top and bottom padding to 8dp

BUG=723042

Review-Url: https://codereview.chromium.org/2930713002
Cr-Commit-Position: refs/heads/master@{#478833}
Committed:
https://chromium.googlesource.com/chromium/src/+/57f1f0bb48b70875fa118be22661...
==========

[commit-bot: I haz the power, 2017-06-13 00:14:51]

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