Splitting off notification.css from Bug 465924.

> .messageImage { > width: 16px; > height: 16px; > margin-top: 0px; > margin-bottom: 0px; > - -moz-margin-start: 6px; > + -moz-margin-start: 5px; I think 5px looks better. > diff --git a/suite/themes/modern/jar.mn b/suite/themes/modern/jar.mn > + skin/modern/global/icons/question-16.png (global/icons/question-16.png) I'm including question-16.png because it is already referenced elsewhere in suite. There are some things from Kudens notification.css that I didn't include because I didn't understand why they were there e.g. border-colour and -moz-border-radius on the .messageCloseButton. I didn't use Kudens background colours for the notifications. > notification { > color: #000000; > background-color: #FFFFE0; > } > > notification[type="info"] { > background-color: #C7D0D9; > } > > notification[type="critical"] { > background-color: #FFCCCC; > } Winstripe has this rule but I think it makes things look worse so I didn't include it: > .messageCloseButton > .toolbarbutton-icon { > -moz-margin-end: 5px; > }

Comment on attachment 399723 [details] [diff] [review] Patch v1.0 >GIT binary patch Haven't looked at the images yet... >+ color: #000000; > background-color: #E8DB99; >- color: #000000; background-colo(u)r first throughout please. >+notification:not([type="info"]) > .notification-inner { .notification-inner inherits the type, so you can check it directly. Also, the type can only be info, warning and critical, and you're already overriding it for critical, so this is only effective for warning. It might make more sense to default to info background colour and override for warning too. >+ border-color: #E8DB99 !important; Why !important? >+notification[type="info"] .messageImage { Sadly .messageImage does not inherit type :-( .notification-inner[type="info"] > hbox > .messageImage >+ margin-top: 0; >+ margin-bottom: 0; Nit: 0px;

Comment on attachment 399723 [details] [diff] [review] Patch v1.0 > .messageImage { > width: 16px; > height: 16px; > margin-top: 0px; > margin-bottom: 0px; I noticed that the icons look very low with these margins. I'd have to try some other values e.g. 5px bottom to see how they look. >+.messageText { >+ margin-top: 0; >+ margin-bottom: 0; >+ -moz-margin-start: 5px; >+ -moz-margin-end: 1px; Ironically these margins *do* have to be important!

The alpha transparency was implemented really badly; the information icon would only work by chance because its background is the Modern default, but the other two icon's shadows won't work properly :-(

(In reply to comment #4) > The alpha transparency was implemented really badly; the information icon would > only work by chance because its background is the Modern default, but the other > two icon's shadows won't work properly :-( These look strange if there is no shadow...

My fault, I hadn't realised that the shadow was quite so blue.

> >+ color: #000000; > > background-color: #E8DB99; > >- color: #000000; > background-colo(u)r first throughout please. Fixed. > >+notification:not([type="info"]) > .notification-inner { > .notification-inner inherits the type, so you can check it directly. Also, the > type can only be info, warning and critical, and you're already overriding it > for critical, so this is only effective for warning. It might make more sense > to default to info background colour and override for warning too. Fixed. > >+ border-color: #E8DB99 !important; > Why !important? Apparently there is also a rule for the outset class coming from global.css which sets the border colour and which gets applied after the rules in notification.css. > >+notification[type="info"] .messageImage { > Sadly .messageImage does not inherit type :-( > .notification-inner[type="info"] > hbox > .messageImage Fixed. > >+ margin-top: 0; > >+ margin-bottom: 0; > Nit: 0px; Fixed. > > .messageImage { > > width: 16px; > > height: 16px; > > margin-top: 0px; > > margin-bottom: 0px; > I noticed that the icons look very low with these margins. I'd have to try some > other values e.g. 5px bottom to see how they look. I found that 5px was too much. 3px seems right to me but this is awfully subjective. > >+.messageText { > >+ margin-top: 0; > >+ margin-bottom: 0; > >+ -moz-margin-start: 5px; > >+ -moz-margin-end: 1px; > Ironically these margins *do* have to be important! Fixed.

Comment on attachment 400022 [details] [diff] [review] Patch v1.1 > notification { > background-color: #E8DB99 > color: #000000; > } > > notification[type="info"] { > background-color: #C7D0D9; > } [The idea was that we swap the background colours around so that the "default" type is "info" and then type="warning" overrides the background and border.]

Comment on attachment 400022 [details] [diff] [review] Patch v1.1 Setting a+ so this can go in without further ado.

Carrying forward r=Neil > (From update of attachment 400022 [details] [diff] [review]) >> notification { >> background-color: #E8DB99 >> color: #000000; >> } >> >> notification[type="info"] { >> background-color: #C7D0D9; >> } > [The idea was that we swap the background colours around so that the "default" > type is "info" and then type="warning" overrides the background and border.] Oops. Fixed.

Comment on attachment 400361 [details] [diff] [review] Patch v1.2 [Checkin: Comment 12] http://hg.mozilla.org/comm-central/rev/e80058ce2515