Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(369)

Issue 2784133002: Cros system menu - Use WiFi strike-through image for "No networks" row (Closed)

Created:
3 years, 8 months ago by Evan Stade
Modified:
3 years, 8 months ago
Reviewers:
tdanderson, stevenjb
CC:
chromium-reviews, davemoore+watch_chromium.org, kalyank, stevenjb+watch_chromium.org, oshima+watch_chromium.org, sadrul
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Cros system menu - Use WiFi strike-through image for "No networks" row and tray icon. This CL addresses the linked bug and rewrite NetworkIconSource to use vector icon structs instead of ImageSkia for badges. This is necessary because the badge needs to be painted to the same canvas instead of overlaid on top so that we can use PATH_MODE_CLEAR. Test with CL flag --shill-stub="wifi=none,eth=0". The item in the system menu should have a strike through. The item in the tray should as well. Also test with other flags like --shill-stub="wifi=12,eth=0,vpn=1" to verify other changes. BUG=706453 TBR=stevenjb@chromium.org Review-Url: https://codereview.chromium.org/2784133002 Cr-Commit-Position: refs/heads/master@{#461434} Committed: https://chromium.googlesource.com/chromium/src/+/a6ce76b8e2242b6f8d4e89133579fa13944c7757

Patch Set 1 #

Patch Set 2 : fix tray too #

Total comments: 2

Patch Set 3 : docs #

Unified diffs Side-by-side diffs Delta from patch set Stats (+132 lines, -118 lines) Patch
M ash/common/system/chromeos/network/network_icon.h View 1 2 chunks +3 lines, -5 lines 0 comments Download
M ash/common/system/chromeos/network/network_icon.cc View 1 17 chunks +97 lines, -98 lines 0 comments Download
M ash/common/system/chromeos/network/tray_network.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M ash/resources/vector_icons/network_badge_off.icon View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/status/network_menu.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/paint_vector_icon.h View 1 2 2 chunks +12 lines, -3 lines 0 comments Download
M ui/gfx/paint_vector_icon.cc View 1 5 chunks +16 lines, -9 lines 0 comments Download

Messages

Total messages: 23 (15 generated)
Evan Stade
3 years, 8 months ago (2017-03-30 18:15:26 UTC) #4
tdanderson
Nice, LGTM https://codereview.chromium.org/2784133002/diff/20001/ui/gfx/paint_vector_icon.h File ui/gfx/paint_vector_icon.h (right): https://codereview.chromium.org/2784133002/diff/20001/ui/gfx/paint_vector_icon.h#newcode58 ui/gfx/paint_vector_icon.h:58: // FIXME Anything left to fix? Also, ...
3 years, 8 months ago (2017-03-30 20:48:51 UTC) #9
Evan Stade
https://codereview.chromium.org/2784133002/diff/20001/ui/gfx/paint_vector_icon.h File ui/gfx/paint_vector_icon.h (right): https://codereview.chromium.org/2784133002/diff/20001/ui/gfx/paint_vector_icon.h#newcode58 ui/gfx/paint_vector_icon.h:58: // FIXME On 2017/03/30 20:48:51, tdanderson wrote: > Anything ...
3 years, 8 months ago (2017-03-30 23:56:47 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2784133002/40001
3 years, 8 months ago (2017-04-03 14:26:23 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/400935)
3 years, 8 months ago (2017-04-03 14:34:08 UTC) #15
Evan Stade
TBR stevenjb for function name update in network_menu.cc
3 years, 8 months ago (2017-04-03 14:37:18 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2784133002/40001
3 years, 8 months ago (2017-04-03 14:37:38 UTC) #20
commit-bot: I haz the power
3 years, 8 months ago (2017-04-03 15:28:50 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a6ce76b8e2242b6f8d4e89133579...

Powered by Google App Engine
This is Rietveld 408576698