Styling status messages in system.css

What's the best way to test this?

Yes, but do you have any page samples that use the .status class?

If you're going to put it into system.css, I'd suggest taking it out of the other themes (Garland, Marvin, etc).

I think this is a good idea. I recently made a simple new theme for my site in Drupal 6 and it bugged me that I had to add .status to my css. Like theborg said, error messages are themed by default so why not have a little theming for status messages to just make them more useable.

perhaps share a screenshot?

Patch still applied against CVS HEAD.

Still applies, good to have things consistent.

Why .status is gray? Maybe make them as .ok (green)?

Hm. I would love to get a review from a few designer-eyes out there. The grey seems to clash a bit with Garland, although I understand choosing a "neutral" colour in case someone has a bright neon pink and orange theme.

The Zen theme goes with slightly bolder colors. Grey is still too drab to really draw attention, IMO.

Also, some really nice icons would help draw the eye. Again, the Zen theme has some nice ones (see the attached image), which yoroy doesn't like! lol #370419: Remove messages.css in favor of core's styling

Now that Stark is in, it would be good to move stuff from all the other themes into system.css and make it look nice in Stark.

- The most suitable color for neutral 'for your information' messages would be a light blue. something like #e8e8ff
- Design comment? Lose the border. The background color alone defines enough of a 'box'
- Icons? Possibly, but then I'd say use the watchdog ones (or improved versions thereof :-)

All together, it would look something like this:

#18 is a lovely, lovely thing filled with loveliness. +10,000 from me.

As a novice themer, I found it absolutely bizarre that I had to style these myself and copy/paste a bunch of crap from Garland to make my theme look relatively nice.

However, more "advanced" themers are going to want an easy way to shut this off so they can put their own styling in there instead. So probably put this styling in system-messages.css or something similar.

Where does the "OK" message come from? I only see status, warning, and error in http://api.drupal.org/api/function/drupal_set_message/7

The .ok class in drupal's stylesheets aren't related to status messages.

system-messages.css +1. That's how Zen handles it in D6. Easy to modifiy/override/remove that way.

+1 to #18.

Beautiful, and sorely needed. Anything that improves the 'polish' of Drupal is a godsend to me. And this does it a hundredfold!

Good point JohnAlbin, so we skip the green one.

I cannot write the patch, but what can I do to help somebody else do it? :-)
If we want to use the icons, we should probably do that in #218674: Update watchdog-*.png status icons

These are the colors:

  background: #e8e8ff url(images/info.png) no-repeat;
  background: #fcc url(images/error.png) no-repeat;
  background: #FCFCA7 url(images/warning.png) no-repeat;

  background-position: 8px;  <-- makes the icon align middle
  background-position: 8px 4px; <-- makes the icon align top

padding for the container div:
padding: 0.25em 0.5em 0.25em 32px;

@yoroy. It seems we have more momentum in this issue. Since any icons are better than NO message icons; and watchdog already has icons. So let's do this issue first and then do #218674: Update watchdog-*.png status icons .

I can roll a patch tonight.

I tried to use the SVG source in http://drupal.org/node/218674#comment-1249156, but neither Illustrator, Fireworks or Photoshop would open it. What's the trick to opening it? Actually, yoroy, maybe you'd better generate the individual icons and post them here. :-)

Here they are. (The SVG source over there should work with Inkscape)

If you put the images in #25 above into the misc directory and name them error.png, info.png, and warning.png and apply this patch, you'll get… the screenshot I posted in the next comment.

I mean this.

Also, I forgot to mention that the RTL style is not ideal. CSS doesn't have a way to position a background image relative to the top right corner. Specifically, you can position it exactly on the right (which this patch does), but not "5px from the right" (which this patch should do). The only crufty work-around I can see for this is to included 5 pixels of space inside the actual png files.

Why the borders? It's ugly, eats another few pixels of vertical space and visually kinda represents shouting to me, I really wish Drupal would stop doing that.

If you added them to prevent them from not standing out against a theme that uses a color similar to one of the backgrounds here, I'd say that's for the theme to fix/override. Why make it ugly for everyone just to prevent some edge cases from happening?

Is there a specific reason you're using px for the positioning?
I used "padding: 0.25em 0.5em 0.25em 32px;" for the div and "background-position: 8px 0.5em;" for the icon.

I wouldn't mind adding a few transparent pixels, maybe we should use that for the LTR positioning as well then.

Seems I added a bit too much gloss to the blue info icon, the one in your screengrab has a lot less contrast in the upper half then mine in #18. I'll check and re-upload using the names you used as well.

I agree on the borders looking messy.

The bulleted lists should be aligned where the non-bulleted lists are.

Also, why aren't the icons vertically aligned?

Left-aligning the list items is not for stark theme to do. Any other theme would hopefully make this prettier looking indeed.
(or should we cuz we are patching system.css itself? dunno)

Peoples seemt to think the icons should align along the top.
(I don't really feel like they have to. Center vertically is certainly easier since that's default behaviour)

re: RTL alignment, on second thought I don't think we should add transparent pixels to the actual icon file. not clean and not improving re-usabiltyness.I would probably guesstimate with a horizontal 96% offset or something, but that's also sub-optimal.

I think that system.css should be putting in sensible defaults, so that all a themer has to do is override what they don't like about the theme. Therefore I think that the left align should go in as part of this. and is overridable in themes. Realistically, most themes will want the alignment the same, so it would mean that all themes would have to duplicate this work. So from the core point of view, each core theme would have to change the alignment, probably using the same code, so we then have the same code in 3 or 4 places to maintain in future.

Also, I vote for centred icons - it seems to make more sense. The only time that I think the top align works better is if you might have multiple of one type (eg error) and were going to put the icon next to each error. Personally I think one for the area is sufficient.

Agreed with wretched sinner. yoroy is right that it's not in stark's place to style these... in fact, it's not in stark's place to style *anything.* :) That's why they should be styled like this by default in system-messages.css. If a theme doesn't like the default styling, they just add stylesheets[all][] = system-messages.css (or whatever the syntax is) and do their own thing.

BTW, the Zen issue where people asked for a top-aligned image was #308251: Add useful styling for status, warning, and error messages. Here's the screenshot of center-aligned image with 40+ messages (using Zen 6.x-1.0 message icons):

So… Is that better or worse?

I see you guys were planning about adding info messages. If we continue with that in #375928: Add type "info" to Drupal Messenger service, we can work on the CSS issues in this issue.

I agree that message boxes should be fully styled in a core CSS file. This is something all themes need and most will probably use the default styling anyway.

subscribe

Coming from the design side, I need to echo the "lose the borders" comment. More visually attractive. Also works better in cases where a jQuery fade-out effect would be used.

The #18 comment is overall the best-looking solution.

+ 1 for #18 - clean, nice, clear, no borders, nice simple icons, I would close this task with that and would be totally happy with the outcome

+1 for #18 as well for the core base theme

Ok good. +1s all around. Now how about some folks to review/work on the patch? :)

#18 looks nice, but whats about top-aligned?
some error messages are long so better view icons alligned at top

I'm working of rerolling this patch.

Please look at the screen shot shown with a 'status' and a 'warning'. I can't get three types of errors yet. So use Firebug to emulate the last one. :-)

Here we go. I've rerolled the patch.

I removed the message borders, leaved the icon at top left and used em instead of px.

By the way I'm using icons in #25, but changed their name to start with "system-" instead. So those icons needs to be added as well.

I think this patch is a great idea. Having good looking default system messages is a good idea since most themes really don't need, or want to adjust the message styling. As long as they look nice to start out with then themers don't need to muck with them as much.

+ background-position: right 0.33em;

should be

+ background-position: 0.33em right;

as per http://www.w3schools.com/css/pr_background-position.asp

Should properties be in alphabetical order? http://drupal.org/node/302199 According to the docs they should, but that doesn't seem to be the practice in system.css as of yet.

Should we consolidate properties like this into a single line?

+  background-color: #e8e8ff;
+  background-image: url(../../misc/system-info.png);
+  background-position: 8px 0.33em; /* LTR */
+  background-repeat: no-repeat;

background: #e8eff url(../../misc/system-info.png) no-repeat 8px 0.33em; /* LTR */

It seems that most places in system.css do it this way, however there are a few exceptions.

Screenshot before applying patch. Tested by calling drupal_set_message() from the devel.module php execution block, hence the extra, blank line in the 'status' message.

http://drupal.org/files/stark-before.jpg

Patch applies cleanly. Still need to download the status, error, and warning icons from #25, rename them, and place them in misc/

Screenshot after patching.

http://drupal.org/files/stark-after.jpg

Some thoughts.

Garland's CSS needs to be updated. Before and after screenshots below. The green text on a blue background should be fixed, and we should probably remove the borders, or adjust the colors so they match with the block colors.

Thanks for the review. I will create an updated patch later.

We should not consolidate the properties in a single line, though. It's easier for a themer to override a single property. And it's easier to highlight which part is /* LTR */ enabled.

We need to land this patch before Garland or other themes update their CSS.

Forgot the status flag.

I would argue that because Garland is part of core it should be updated with this patch as well. However, as long as we are aware that this patch causes issues with Garland and can get those fixed shortly after this patch gets committed I would say this is good to go.

Point taken about easier to theme for the multi-line css properties. Though I think it would be worthwhile to make sure that all of the system.css, and similar files are using one style or the other just for consistency. At some point.

Marking as RTBC since my issues with this patch have been addressed, and I'm pretty sure we can easily fix Garland's CSS once this is committed.

Again just a reminder for anyone else reviewing committing this code, you need the error, info, and warning icons from #25, and they need to be renamed as per #44.

Let's fix both core themes in one go, please.

Point taken! I'll update Garland and reroll the patch.

However, please note I plan to keep the borders in Garland, which I believe are better suited when the background uses gradients.

There is no need to modify Stark since it'll use the new core css.

I changed my mind about updating Garland.

Looks like some changes will affect the color module and I don't want to break user defined colors in Garland and Minelli.

/*******************************************************************
 * Color Module: Don't touch                                       *
 *******************************************************************/

/**
 * Generic elements.
 */
.messages {
  background-color: #fff;
  border: 1px solid #b8d3e5;
}
. . .

Can we ask the Garland maintainer to look at this issue?

Personally in almost all of my themes, I split out multiple messages of the same type into separate things visually.

@#46 / xmacinfo: What's the main impetus for not using CSS shorthand? It dramatically reduces filesizes, is easier to read (imo as a themer), and can still be overridden by simply setting "background-color: #FFF" or whatever. Plus, FireBug and WebKit Inspector break it down anyways.

I'm +1 for using CSS shorthand... but since both ways are valid, I wouldn't make it a sticky point. Just offering my 2¢.

Curious to see if this patch still applies

Setting to 'needs review' - testbot was broken.

We need to ask Webchick permission to submit this patch prior any work on Garland. If she agrees to, I'll make any changes necessary to reroll this patch.

If we still need Garland to be modified as well, we need a Garland expert since I don't want to mess around in the "Don't touch" section of Garland styles.

This may now have an impact on #484860: Initial D7UX admin theme.

Would Seven use the same system messages visual as core?

@xmacinfo: it is best to look at some screenshots of Seven with this proposal. Since Seven mostly uses gray variants, I doubt it wold not look right with this scheme either. Mark did not design any message colors or boxes.

This a reroll taking into account Seven and Garland (see #50 ;-)). As for Stark it will look the same as the latest screen capture.

-> Garland now uses the system-messages.css colors and I removed the Garland message borders.

-> Seven now uses the system-messages.css colors and I removed the Seven message borders.

As for Seven, please note that Gábor told me that the messages colors where not defined by the UX theme yet. So using the default system colors should not be a bad thing for Seven. And borders for messages are a thing of the past. Let's get rid of these borders.

I think we should have Garland, Stark and Seven use the default System colors. If we really need Seven specific colors, this might as well be discussed in another issue.

I dont think these changes will reflect nicely on the "Seven" theme, please leave that out of this patch.

I am rerolling removing Seven from this patch. Bojhan suggest that we can open a new issue later for Seven if we need to change the display of status messages.

:-)

This reroll is the same patch as #62, but removing Seven from the patch as suggested by Bojhan.

Please take the images in #25 to test out the complete features.

Great work, looks very nice.....

+++ modules/system/system-messages.css	17 Aug 2009 18:58:12 -0000
@@ -0,0 +1,30 @@
+  background-color: #e8e8ff;
+  background-image: url(../../misc/system-info.png);
+  background-position: 8px 0.33em; /* LTR */

Should we move system-info.png, system-warning.png and system-error.png into modules/system/images/*.png so that we don't have to have the weird ../../ relative directories? It's nice to have things modular. Maybe webchick would know more.

+++ modules/system/system-messages.css	17 Aug 2009 18:58:12 -0000
@@ -0,0 +1,30 @@
+div.messages ul {
+  margin-bottom: 0;
+  margin-top: 0;
+}

Should these be 0px? Does "0" conform to CSS standards?

+++ themes/garland/style.css	17 Aug 2009 18:58:12 -0000
@@ -1045,7 +1045,6 @@ tr.taxonomy-term-divider-bottom {
  */
 .messages {
   background-color: #fff;
-  border: 1px solid #b8d3e5;
 }
 
 .preview {

Do we still need .messages in garland's style.css?

11 days to code freeze. Better review yourself.

About moving images "into modules/system/images/*.png".

Well, all other Drupal core images are in /misc. There must be a good reason for this.

About 0px versus 0

Yes, when using 0 we must not display the type of unit. :-)

About "Do we still need .messages in garland's style.css?"

Unfortunately, Garland uses it and here I just remove the border.

But here is a question, do we want Seven to load the status message icons as well?

:-)

Marked #290343: Add icons to status messages. duplicate of this.

Great find yoroy, and thanks for these icons.

Yes, for those who cannot distinguish colors easily, we need icons to give the same type of feedback.

This patch will need a reroll since core changed quickly the last few days.

Remaining issues:

• Add the icons to seven? The reroll in #66 removed the icons for Seven.
• Use the same messages colors for Seven? #63 I was ask to remove the colors for Seven, which uses another set of messages colors. So do you prefer Seven messages colors or this issue messages colors?

As soon as I have some comments for these issues I will reroll. :-)

Subscribe.

#218674: Update watchdog-*.png status icons is a duplicate of this issue, since in #25 yoroy introduces new Watchdog icons.

- I say let Seven use its own colors for messages and focus on a nicer core default for this.
- We could leave out the icons for now and re-open #218674: Update watchdog-*.png status icons for updating the *core* icons in /misc ?

(I'm collecting my icon work in http://drupal.org/project/protocons, has status icons too :-)

I see 32 380 (current) but http://drupal.org/files/images/protocons-preview.png just 28 665 so 12% traffic improvement
+1 resonable

Glad to see a patch before beta!!!

Also there's a issue #718662: DBLog listings truncate messages in the middle of HTML tags we could tune some dblog render

Can we get a re-roll here? Not quite sure what is outstanding, and I'd like to review this. Thanks.

Rerolling but now like this:
- without icons
- not 'div.messages' but '.messages' class definitions. didn't test that thoroughly.
- previous patches used the blueish tint for status messages, changing to the green tint. Most of these messages are of the 'succes' variety anyway, "blabla has been saved" etc. The blue was looking too weak anyway.

Le screenshot:

Thanks @yoroy!

The left padding looks odd without an icon. Maybe we should remove it until we actually have icons?

I see we are not addressing Seven here, but what about Garland? Is anything needed of there?

Yup, lets drop the padding.

Must confess I didn't check Garland at all, but webchick in #50 requested we *do* incorporate any necessary Garland fixes here. Could somebody post Garland screengrabs of before and after latest patch?

Changes to Seven would be great to get backported to the contrib Seven D6 project once this is worked out :)

Garland screengrabs, ahoy!

Garland is not properly styled as seen in comment in #83. We should have the same visual as in #79 minus the left padding. :-)

Without padding and icon as a bg-image there's no need for a seperate RTL stylesheet anymore, right?
This patch only removes the padding, not looked at Garland yet.

now with 100% more patch

Please do Garland as well. :-)

Why not use the icons in #25 too? Well... until new icons are available.

Just tested this :)

+++ modules/system/system.css	15 Mar 2010 01:22:30 -0000
@@ -64,39 +64,6 @@
-.error {
-  color: #e55;
-}
...
-.warning {
-  color: #e09010;
-}
...
-.ok {
-  color: #008000;
-}

We can't remove these. Doing so breaks the password strength feedback messages, the drag and drop messages on the block listing page (the asterisk showing modified blocks is wrapped in a span.warning), and possibly more.

+++ modules/system/system-messages.css	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,24 @@
+.messages,
+.status,
+.warning,
+.error {
+  color: #000;
+  margin: 0 1em 5px 1em;
+  padding: 0.25em 0.5em;
+  background-color: #dfd;
+}

We need to use div.messages here, to avoid screwing the others up. Since .messages is a common class for all of these, anything that applies to all three should be in div.messages { ... }. No need to do div.messages, div.status, etc.

I don't like adding color: #000 here, but I believe it is necessary.

Regarding margins, let's only style what's absolutely necessary. Left and Right margins shouldn't be added here IMO. This should be a decision made by the theme, if needed. If we do need margins, let's make them all ems, and not mix px and ems.

+.messages ul {
+  margin-top: 0;
+  margin-bottom: 0;
+}

Should we really style .messages ul?

142 critical left. Go review some!

I only have a vague idea of what I'm doing here…

@yoroy Thanks, I think this is almost ready ;) I rerolled it to add some padding and the margin-bottom back, and fix a couple of minor coding style issues.

Seven looks fine, so we just need to figure out what we are doing with Garland. It's not good the way it is, so we need to do something. Garland is applying the border colors on the messages through the color module, which is what's going on in the screenshot (attached). What should we do about that? No borders, old border colors? Do we like these new colors better for Garland? Let's figure that out so we can wrap this up.

@jacine: Nice work. But there may be reason to keep border in Garland.

Personally I would remove the borders as well in Garland. But since Garland uses a lot of gradients, message background colors without border may look awkward when displayed along Garland multiple gradients around the content area.

Is there a way to modify Garland as to not use the Color module to generate the border color?

To me, the best fix would be to keep Garland message styling as is; let it keep the borders, and keep its own colors.

We are deleting everything in drupal.css and moving it to markup, leave it there please...

I agree with @yoroy that it looks better the way it was. This patch gives Garland back the old colors and borders.

No one is willing to test? :(

For consistency with the Stark and Garland screenshots, here is a screenshot of it with Seven.

Summary of what this patch does:

- move css rules for styling messages to a new system-messages.css
- apply a better looking default look to these messages.
- fixes for Garland to keep Garland's own existing look.

So, a new css file with better defaults that none of the current core themes use after all (besides Stark of course). Which is fine. Thanks all!

+++ modules/system/system-messages.css	18 Mar 2010 05:06:17 -0000
@@ -0,0 +1,36 @@
+tr.error {
...
+table tr.warning {
...
+tr.ok {

table for tr.warning must be superfluous.

+++ modules/system/system-messages.css	18 Mar 2010 05:06:17 -0000
@@ -0,0 +1,36 @@
+div.ok,
+tr.ok {
+  background: #dfd;

+++ themes/garland/style.css	18 Mar 2010 05:06:19 -0000
@@ -1107,8 +1107,14 @@ tr.taxonomy-term-divider-bottom {
+div.error {
+  background: #fcc;

@@ -1116,6 +1122,12 @@ div.error, tr.error {
+div.warning {
+  background: #ffd;

Needs to use background-color.

+++ themes/garland/style.css	18 Mar 2010 05:06:19 -0000
@@ -254,7 +254,7 @@ span.submitted, .description, .vertical-
-.messages {
+div.messages {
   margin: .75em 0 .75em;
   padding: .1em .5em .15em;
 }

@@ -1096,7 +1096,7 @@ tr.taxonomy-term-divider-bottom {
-.messages {
+div.messages {
   background-color: #fff;
   border: 1px solid #b8d3e5;
 }

Why does the same selector exist twice?

132 critical left. Go review some!

Rerolled patch from #94

- Removed table from tr.warning selector

- Used background-color for all background attributes

- Consolidated selectors for system messages in Garland/style.css (combined div.messages and moved .messages ul and .messages li)

Needs review for testbot

Sorry for not responding sooner...

The reason that there are 2 entries for .messages in Garland is because one is in the color module section. Users are instructed not to edit the color section of the file so that part needs to stay the way it was in the previous patch.

Rerolled patch from #99

- Moved div.messages margin and padding rule back to line 257

- Moved .messages ul and .messages li back to their original locations

@james.elliott Thanks :D

The new CSS file needs to be added to the patch.

Why yes, it would help to add the -N switch.

I remembered for this one :D

And status :)

Great! Looks good to me. Thank you. :D

Committed to CVS HEAD. Thanks.

I'm writing this from an iPhone. So not verified, but I think that the new system-message.css file have not been committed. Can someone verify this?

Confirmed:

“modules/system/system-messages.css” is missing from “http://drupal.org/cvs?commit=349090”.

So we need core maintainer attention.

In case anybody is being completely baffled by their Eclipse debugging being completely broken by this commit:
You can prevent eclipse from repeatedly continuing on in a debugging loop with

touch modules/system/system-messages.css

until the missing file is committed.

I have no idea why a missing CSS file would completely ruin a debugger.

But another triumph for "git bisect".

Committed missing CSS file.

That's wild that that breaks Eclipse so badly. Huh.

Looks good, thanks for all.