Clean up offline_pin_(black, round) icon |
|||||
Issue descriptionBecause of confusion over which icon to apply between: NTP, Omnibox and download notifications, original offline_bolt got replaced with offline pin in round circle, which was then replaced with just an underscored tick. Underscored tick is appropriate for notifications and now named offline_pin Offline pin in circle is appropriate for NTP and Omnibox, however this resource is now duplicated in M56: NTP: offline_pin_black Omnibox: offline_pin_round Duplication was intentional because of necessary M55 merge. My suggestion: 1. Remove _round 2. Rename _black to _round 3. Update NTP to use _round. 4. Ensure NTP works as expected 5. Ensure Omnibox works as expected (CCT, Omnibox (normal and themed tab)) Check it into M56 Related issue: * 663702 * 664546
,
Nov 14 2016
,
Nov 14 2016
The reason the _black icon is smaller is that the resource behind it has smaller dimensions, which makes it appear smaller in the Omnibox. Hence we are not going to be moving _black -> _round, but simply removing it, and replacing usages of _black with _round.
,
Nov 14 2016
Interesting. Does that mean that NTP has different sizing requirements for their icon?
,
Nov 14 2016
No, it means the icon was optimized for NTP and not for general use.
,
Nov 15 2016
Update: Patch has landed on trunk. Waiting for the buildbot to update the bugs.
,
Nov 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68cb4e748bb930a35ef2ae756dcb8d92db74263e commit 68cb4e748bb930a35ef2ae756dcb8d92db74263e Author: fgorski <fgorski@chromium.org> Date: Tue Nov 15 19:04:10 2016 [Offline pages, NTP] Unifying offline_pin_round(_black) icon Removes offline_pin_black icon and replaces its usage with offline_pin_round. This patch is meant to be merged in M55 (without the _black icon removal, as it is not present there). BUG= 664670 , 663702 Review-Url: https://codereview.chromium.org/2502763002 Cr-Commit-Position: refs/heads/master@{#432228} [delete] https://crrev.com/a816bfc268006458526fb07e84ac1e03cd1ac03f/chrome/android/java/res/drawable-hdpi/offline_pin_black.png [delete] https://crrev.com/a816bfc268006458526fb07e84ac1e03cd1ac03f/chrome/android/java/res/drawable-mdpi/offline_pin_black.png [delete] https://crrev.com/a816bfc268006458526fb07e84ac1e03cd1ac03f/chrome/android/java/res/drawable-xhdpi/offline_pin_black.png [delete] https://crrev.com/a816bfc268006458526fb07e84ac1e03cd1ac03f/chrome/android/java/res/drawable-xxhdpi/offline_pin_black.png [delete] https://crrev.com/a816bfc268006458526fb07e84ac1e03cd1ac03f/chrome/android/java/res/drawable-xxxhdpi/offline_pin_black.png [modify] https://crrev.com/68cb4e748bb930a35ef2ae756dcb8d92db74263e/chrome/android/java/res/layout/most_visited_item.xml [modify] https://crrev.com/68cb4e748bb930a35ef2ae756dcb8d92db74263e/chrome/android/java/res/layout/new_tab_page_snippets_card.xml
,
Nov 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bfbee93f33fc4522da61cfa8182696803ad14f95 commit bfbee93f33fc4522da61cfa8182696803ad14f95 Author: Filip Gorski <fgorski@chromium.org> Date: Tue Nov 15 23:23:50 2016 [Offline pages, NTP] Unifying offline_pin_round(_black) icon Removes offline_pin_black icon and replaces its usage with offline_pin_round. This patch is meant to be merged in M55 (without the _black icon removal, as it is not present there). BUG= 664670 , 663702 Review-Url: https://codereview.chromium.org/2502763002 Cr-Commit-Position: refs/heads/master@{#432228} (cherry picked from commit 68cb4e748bb930a35ef2ae756dcb8d92db74263e) Review URL: https://codereview.chromium.org/2502133002 . Cr-Commit-Position: refs/branch-heads/2883@{#581} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/bfbee93f33fc4522da61cfa8182696803ad14f95/chrome/android/java/res/layout/most_visited_item.xml
,
Nov 15 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by fi...@chromium.org
, Nov 14 2016Status: Assigned (was: Untriaged)