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

Issue 11651002: Updated multiple item notification view to match latest mockups (Closed)

Created:
8 years ago by dharcourt
Modified:
7 years, 11 months ago
Reviewers:
Jun Mukai
CC:
chromium-reviews, tfarina, sadrul, ben+watch_chromium.org, miket_OOO, Daniel Erat
Visibility:
Public.

Description

Updated multiple item notification view to match latest mockups This also prepares this view to serve as a generic notification view by simplifying the layout grid and by making the padding mechanism more flexible. The resulting more generic view will be able to be used to display image and basic notifications in addition to multiple item ones. The visual changes made to match the mockups include removal of the timestamp, separator line, message, and secondary icon. TBR=derat@chromium.org for OWNERS review of balloon_view_ash.cc changes. BUG=161101 BUG=164292 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175079

Patch Set 1 : #

Total comments: 21

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : Rebase #

Patch Set 5 : Forgotten EXPORT fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -269 lines) Patch
M chrome/browser/ui/views/ash/balloon_view_ash.h View 1 2 3 1 chunk +1 line, -5 lines 0 comments Download
M chrome/browser/ui/views/ash/balloon_view_ash.cc View 1 2 3 2 chunks +7 lines, -24 lines 0 comments Download
M ui/message_center/message_center.gyp View 1 1 chunk +2 lines, -0 lines 0 comments Download
A ui/message_center/message_center_constants.h View 1 2 3 4 1 chunk +16 lines, -0 lines 0 comments Download
A + ui/message_center/message_center_constants.cc View 1 1 chunk +5 lines, -5 lines 0 comments Download
M ui/message_center/message_view_multiple.h View 1 1 chunk +0 lines, -5 lines 0 comments Download
M ui/message_center/message_view_multiple.cc View 1 2 3 2 chunks +107 lines, -230 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
dharcourt
This is the first of four change lists that will introduce the image notification display ...
8 years ago (2012-12-18 23:58:03 UTC) #1
Jun Mukai
Several nitpicking comments. They're not so relevant to your changes themselves but would be better ...
8 years ago (2012-12-19 00:33:53 UTC) #2
dharcourt
Thanks for the review. Lots of good fixes suggested. I implemented most of them and ...
8 years ago (2012-12-19 01:47:44 UTC) #3
Jun Mukai
lgtm with nitpicking comments https://codereview.chromium.org/11651002/diff/3001/ui/message_center/message_view_multiple.cc File ui/message_center/message_view_multiple.cc (right): https://codereview.chromium.org/11651002/diff/3001/ui/message_center/message_view_multiple.cc#newcode80 ui/message_center/message_view_multiple.cc:80: virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE; ...
8 years ago (2012-12-19 22:28:41 UTC) #4
dharcourt
Hi Jun, Following up on your review feedback, I was able to significantly reduce the ...
8 years ago (2012-12-20 03:27:35 UTC) #5
Jun Mukai
lgtm++
8 years ago (2012-12-20 18:09:10 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/21001
7 years, 11 months ago (2013-01-03 17:17:44 UTC) #7
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 11 months ago (2013-01-03 18:06:52 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/22009
7 years, 11 months ago (2013-01-03 18:20:35 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/22009
7 years, 11 months ago (2013-01-03 19:17:52 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/22009
7 years, 11 months ago (2013-01-03 19:27:30 UTC) #11
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 11 months ago (2013-01-03 19:46:52 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/32010
7 years, 11 months ago (2013-01-03 21:56:33 UTC) #13
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 11 months ago (2013-01-03 22:00:05 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dharcourt@chromium.org/11651002/32010
7 years, 11 months ago (2013-01-03 22:41:20 UTC) #15
commit-bot: I haz the power
7 years, 11 months ago (2013-01-04 02:07:51 UTC) #16
Message was sent while issue was closed.
Change committed as 175079

Powered by Google App Engine
This is Rietveld 408576698