[Offline pages] Reduce the icon set for offline bolt, update colors

[Offline pages] Reduce the icon set for offline bolt, update colors

This patch:
* removes the assets for light offline bolt
* applies tinting to draw the bolt using the right color
* updates the colors based on UX recommendation

BUG=591105
R=tedchoc@chromium.org

Committed: https://crrev.com/a85ab8e4d88a1e32a0db232cfd946533dd49ef16
Cr-Commit-Position: refs/heads/master@{#379595}

[fgorski, 2016-03-01 21:45:38]

Ted,

I am reducing icon set in this patch to one.
I am temporarily replacing the icons with solid black, but I hope to receive the
icons in color that fits the OIB without a need to tint.

I have a few comments/questions inline.

Thanks.
Filip

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/website_settings.xml (right):

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/website_settings.xml:35:
android:tint="@color/locationbar_status_color"
This is needed only when the default icon does not come with the right color for
the OIB.

I want the UX to provide reasonable default icon (such that tint would not be
necessary here) and set the tint in code on the location bar layout. WDYT?

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

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/colors.xml:121: <color
name="locationbar_status_color">#8a000000</color>
All 4 colors will be updated once I hear back from UX. Right now it seems that
dark colors are transparent, while light are not, and that bothers me.

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/colors.xml:123: <color
name="locationbar_status_separator_color">#1f000000</color>
I think UX wanted the separator to have a different color and stay transparent
either way.

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
(right):

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java:1295:
bolt.setColorFilter(ApiCompatibilityUtils.getColor(getResources(),
mUseDarkColors
Turns out you were right and this approach solves the problem even when we are
animating alpha from 0 to 1. Tint rocks.

[fgorski, 2016-03-02 19:02:17]

hold on I got new resources and color specs.

[fgorski, 2016-03-02 20:53:51]

Ted, Please take a look.

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/website_settings.xml (right):

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/website_settings.xml:35:
android:tint="@color/locationbar_status_color"
On 2016/03/01 21:45:38, fgorski wrote:
> This is needed only when the default icon does not come with the right color
for
> the OIB.
> 
> I want the UX to provide reasonable default icon (such that tint would not be
> necessary here) and set the tint in code on the location bar layout. WDYT?

Done. Obsoleted by the new resource.

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

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/colors.xml:121: <color
name="locationbar_status_color">#8a000000</color>
On 2016/03/01 21:45:38, fgorski wrote:
> All 4 colors will be updated once I hear back from UX. Right now it seems that
> dark colors are transparent, while light are not, and that bothers me.

Done.

https://codereview.chromium.org/1753573005/diff/1/chrome/android/java/res/val...
chrome/android/java/res/values/colors.xml:123: <color
name="locationbar_status_separator_color">#1f000000</color>
On 2016/03/01 21:45:38, fgorski wrote:
> I think UX wanted the separator to have a different color and stay transparent
> either way.

Done.

[Ted C, 2016-03-03 15:35:27]

Change looks good though, but let's try to share the same tinting if possible.

https://codereview.chromium.org/1753573005/diff/20001/chrome/android/java/res...
File chrome/android/java/res/values/colors.xml (right):

https://codereview.chromium.org/1753573005/diff/20001/chrome/android/java/res...
chrome/android/java/res/values/colors.xml:121: <color
name="locationbar_status_color">#5a5a5a</color>
can we use the same tint colors as the other toolbar assets?  are these colors
actually different?  If it is only subtle, then It think we should push to use
the same.

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

Look how it is used in the ToolbarLayout/ToolbarPhone.

[fgorski, 2016-03-03 17:26:09]

Glad to report that color-wise we are already there :)

https://codereview.chromium.org/1753573005/diff/20001/chrome/android/java/res...
File chrome/android/java/res/values/colors.xml (right):

https://codereview.chromium.org/1753573005/diff/20001/chrome/android/java/res...
chrome/android/java/res/values/colors.xml:121: <color
name="locationbar_status_color">#5a5a5a</color>
On 2016/03/03 15:35:26, Ted C wrote:
> can we use the same tint colors as the other toolbar assets?  are these colors
> actually different?  If it is only subtle, then It think we should push to use
> the same.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...
> 
> Look how it is used in the ToolbarLayout/ToolbarPhone.

That is exactly why I asked the icon to be 5a5a5a it is the right color. (I
updated the line to say light_color_normal)

For the other one, it is in fact #ffffff, but if you prefer I can go with
@android:color/white, which is used by the code in location bar layout.

I can set tint, but this is not exactly what we want, as it sets the color list
for various states. We only have one state, and I am setting it just like we do
that for page icon (on tablet mostly).

[Ted C, 2016-03-07 17:58:24]

lgtm

[fgorski, 2016-03-07 17:59:41]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2016-03-07 18:00:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1753573005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1753573005/40001

[commit-bot: I haz the power, 2016-03-07 18:51:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-07 18:53:28]

Description was changed from

==========
[Offline pages] Reduce the icon set for offline bolt, update colors

This patch:
* removes the assets for light offline bolt
* applies tinting to draw the bolt using the right color
* updates the colors based on UX recommendation

BUG=591105
R=tedchoc@chromium.org
==========

to

==========
[Offline pages] Reduce the icon set for offline bolt, update colors

This patch:
* removes the assets for light offline bolt
* applies tinting to draw the bolt using the right color
* updates the colors based on UX recommendation

BUG=591105
R=tedchoc@chromium.org

Committed: https://crrev.com/a85ab8e4d88a1e32a0db232cfd946533dd49ef16
Cr-Commit-Position: refs/heads/master@{#379595}
==========

[commit-bot: I haz the power, 2016-03-07 18:53:29]

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