Revives the new message center design.

Revives the new message center design.

It's hidden under a flag since it's still work in progress.

BUG=161052
TEST=run chrome with --enable-new-message-center-bubble flag and see the design of the bubble.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175829

[dharcourt, 2013-01-07 23:08:16]

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
File ui/message_center/message_center_bubble.cc (right):

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_bubble.cc:271: }
Couldn't the view and its shadow be two views one after another in the box
layout view instead of children of a container view? It seems like that would
simpler but there may be disadvantages I'm not thinking of.

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_bubble.cc:337: init_params.max_width +=
kMarginBetweenItems * 2;
Message_view.h's kWebNotificationWidth needs to be changed from 320 to 300 to
prevent this from making the message center 340 pixels wide, 20 pixels more than
the design calls for. Note that changing kWebNotificationWidth will change the
current appearance, making current notifications 300 pixels wide instead of 320.
If that's an issue, kWebNotificationWidth shouldn't be changed (except perhaps
for being renamed to kWebNotificationCenterWidth), min_width and max_width
shouldn't be adjusted here, and message_simple_view.cc should be adjusted to
draw itself based on its bounds rather than on kWebNotificationWidth.

[Jun Mukai, 2013-01-08 01:33:22]

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
File ui/message_center/message_center_bubble.cc (right):

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_bubble.cc:271: }
On 2013/01/07 23:08:16, dharcourt wrote:
> Couldn't the view and its shadow be two views one after another in the box
> layout view instead of children of a container view? It seems like that would
> simpler but there may be disadvantages I'm not thinking of.

The margin between the |view| and shadow has to be 0 but the margin between
shadow and the next view has to be kMarginBetweenItems - kItemShadowHeight.  So
just adding both to scroll_content_ doesn't work well.

And rather than doing some hack to scroll_content_, I think additional
|container| would be simpler.

Thoughts?

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_bubble.cc:337: init_params.max_width +=
kMarginBetweenItems * 2;
On 2013/01/07 23:08:16, dharcourt wrote:
> Message_view.h's kWebNotificationWidth needs to be changed from 320 to 300 to
> prevent this from making the message center 340 pixels wide, 20 pixels more
than
> the design calls for. Note that changing kWebNotificationWidth will change the
> current appearance, making current notifications 300 pixels wide instead of
320.
> If that's an issue, kWebNotificationWidth shouldn't be changed (except perhaps
> for being renamed to kWebNotificationCenterWidth), min_width and max_width
> shouldn't be adjusted here, and message_simple_view.cc should be adjusted to
> draw itself based on its bounds rather than on kWebNotificationWidth.

very good catch...

I talked with somas, and he said we want to keep the same width of MessageView
regardless of its bubble, which is 300 pixel. So changing kWebNotificationWidth
to 300 would be fine.  thanks!

[dharcourt, 2013-01-08 01:43:37]

lgtm.

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
File ui/message_center/message_center_bubble.cc (right):

https://codereview.chromium.org/11773004/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_bubble.cc:271: }
On 2013/01/08 01:33:22, Jun Mukai wrote:
> On 2013/01/07 23:08:16, dharcourt wrote:
> > Couldn't the view and its shadow be two views one after another in the box
> > layout view instead of children of a container view? It seems like that
would
> > simpler but there may be disadvantages I'm not thinking of.
> 
> The margin between the |view| and shadow has to be 0 but the margin between
> shadow and the next view has to be kMarginBetweenItems - kItemShadowHeight. 
So
> just adding both to scroll_content_ doesn't work well.
> 
> And rather than doing some hack to scroll_content_, I think additional
> |container| would be simpler.
> 
> Thoughts?

You're right :-). Sorry I missed that.

[commit-bot: I haz the power, 2013-01-08 02:14:49]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Jun Mukai, 2013-01-08 02:19:33]

oops, miket, please review this too...

[miket_OOO, 2013-01-08 22:19:15]

lgtm

[commit-bot: I haz the power, 2013-01-09 01:59:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/11773004/5001

[commit-bot: I haz the power, 2013-01-09 03:23:11]

Retried try job too often on win_rel for step(s) unit_tests

[commit-bot: I haz the power, 2013-01-09 03:54:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/11773004/5001

[commit-bot: I haz the power, 2013-01-09 06:36:25]

Retried try job too often on win_aura for step(s) content_browsertests

[commit-bot: I haz the power, 2013-01-09 17:38:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/11773004/5001

[commit-bot: I haz the power, 2013-01-09 18:13:26]

Change committed as 175829