Issue 10907224: Data promo notification should follow the normal ash notifications theme. This

Issue 10907224: Data promo notification should follow the normal ash notifications theme. This (Closed)

Created:
8 years, 3 months ago by varunjain

Modified:
8 years, 3 months ago

Reviewers:
sadrul, sky

CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Data promo notification should follow the normal ash notifications theme. This CL does two things: 1. Re-work the ash's network tray infrastructure to show "messages" instead of "errors". 2. Use the re-worked network tray to show the data promo notification. BUG=148824 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=156743

Patch Set 1 : patch #

Total comments: 2

Patch Set 2 : patch #

Total comments: 2

Patch Set 3 : patch #

Patch Set 4 : patch #

Patch Set 5 : patch #

Created: 8 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+157 lines, -158 lines)			Patch
M	ash/resources/ash_resources.grd	View	1 2 3	1 chunk	+1 line, -0 lines	0 comments	Download
A +	ash/resources/default_100_percent/common/notification_3g.png	View	1 2 3	Binary file		0 comments	Download
M	ash/system/network/network_observer.h	View	1	1 chunk	+19 lines, -15 lines	0 comments	Download
M	ash/system/network/tray_network.h	View	1	2 chunks	+12 lines, -12 lines	0 comments	Download
M	ash/system/network/tray_network.cc	View	1 2 3	8 chunks	+87 lines, -76 lines	0 comments	Download
D	chrome/app/theme/default_100_percent/notification_3g.png	View		0 chunks	+-1 lines, --1 lines	0 comments	Download
M	chrome/app/theme/theme_resources.grd	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/chromeos/network_message_observer.cc	View	1 2	7 chunks	+14 lines, -9 lines	0 comments	Download
M	chrome/browser/chromeos/status/data_promo_notification.h	View		2 chunks	+6 lines, -11 lines	0 comments	Download
M	chrome/browser/chromeos/status/data_promo_notification.cc	View		6 chunks	+16 lines, -32 lines	0 comments	Download
M	chrome/browser/chromeos/system/ash_system_tray_delegate.cc	View	1 2 3	2 chunks	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

sadrul

A lot of the assets are moving into ash/. You may need to add the ...

8 years, 3 months ago (2012-09-13 22:47:39 UTC) #2

varunjain

All network notifications related assets are in that directory, so I put this one there ...

8 years, 3 months ago (2012-09-13 22:54:23 UTC) #3

sky

LGTM http://codereview.chromium.org/10907224/diff/8001/chrome/browser/chromeos/network_message_observer.cc File chrome/browser/chromeos/network_message_observer.cc (right): http://codereview.chromium.org/10907224/diff/8001/chrome/browser/chromeos/network_message_observer.cc#newcode71 chrome/browser/chromeos/network_message_observer.cc:71: default: Can't you remove this so you get ...

8 years, 3 months ago (2012-09-14 02:06:48 UTC) #4

varunjain

8 years, 3 months ago (2012-09-14 02:53:24 UTC) #5

http://codereview.chromium.org/10907224/diff/8001/chrome/browser/chromeos/net...
File chrome/browser/chromeos/network_message_observer.cc (right):

http://codereview.chromium.org/10907224/diff/8001/chrome/browser/chromeos/net...
chrome/browser/chromeos/network_message_observer.cc:71: default:
On 2012/09/14 02:06:48, sky wrote:
> Can't you remove this so you get a compile time error if someone adds a new
> type?

Done.

Expand Messages | Collapse Messages