For consistency with other page elements (breadcrumb, navigation, etc.) I'd like too see the messages wrapped
in their own div, like in Bartik for example:

<?php if ($messages): ?>
    <div id="messages"><div class="section clearfix">
      <?php print $messages; ?>
    </div></div> <!-- /.section, /#messages -->
  <?php endif; ?>

At the moment if you don't override the page.tpl.php provided by system module the messages are the only elements not wrapped by a div with an id.
This would help with CSS only themes.

CommentFileSizeAuthor
#1 882796_page.tpl_.php_.patch446 bytesderjochenmeyer

Comments

derjochenmeyer’s picture

StatusFileSize
new446 bytes

It is already possible to theme the messages with the div that gets added in theme_status_messages.

function theme_status_messages($variables) {
  $display = $variables['display'];
  $output = '';

  $status_heading = array(
    'status' => t('Status message'), 
    'error' => t('Error message'), 
    'warning' => t('Warning message'),
  );
  foreach (drupal_get_messages($display) as $type => $messages) {
    $output .= "<div class=\"messages $type\">\n";
    if (!empty($status_heading[$type])) {
      $output .= '<h2 class="element-invisible">' . $status_heading[$type] . "</h2>\n";
    }
    if (count($messages) > 1) {
      $output .= " <ul>\n";
      foreach ($messages as $message) {
        $output .= '  <li>' . $message . "</li>\n";
      }
      $output .= " </ul>\n";
    }
    else {
      $output .= $messages[0];
    }
    $output .= "</div>\n";
  }
  return $output;
}

Attached patch however implements the suggestion made above.

lilou’s picture

Status: Active » Needs review
Jeff Burnz’s picture

Version: 7.x-dev » 8.x-dev
Category: bug » feature

Not a bug and way to late. Core template review was over a year ago (so long ago I have forgotten when).

tr’s picture

#1: 882796_page.tpl_.php_.patch queued for re-testing.

sun’s picture

Title: wrap messages in their own div » Wrap messages in an own #messages DIV in default page.tpl.php
Status: Needs review » Reviewed & tested by the community
Issue tags: +markup, +Front end

Lovely.

catch’s picture

Issue tags: +Needs change record

This will need documentation in the theme upgrade docs, tagging as such.

dries’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x. Moving to 7.x for Angie to consider.

webchick’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Needs work

While this seems like a harmless enough fix, it would introduce a markup change in D7 themes, which could affect CSS-only themes. Since #1 has a workaround for D7, I'm inclined to keep this D8-only.

Marking back to 8.x-dev and "needs work" pending a http://drupal.org/node/add/changenotice node.

catch’s picture

Title: Wrap messages in an own #messages DIV in default page.tpl.php » Markup change notification for Wrap messages in an own #messages DIV in default page.tpl.php
Category: feature » task
Priority: Normal » Critical
Status: Needs work » Active
webchick’s picture

Issue tags: +Novice

And since adding that summary is relatively easy, tagging as Novice.

derjochenmeyer’s picture

Status: Needs review » Active

My first Change record... Does this need more elaboration?

http://drupal.org/node/1294560

derjochenmeyer’s picture

Status: Active » Needs review
catch’s picture

Status: Active » Fixed

Looks good to me. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

xjm’s picture

Title: Markup change notification for Wrap messages in an own #messages DIV in default page.tpl.php » Wrap messages in an own #messages DIV in default page.tpl.php
Category: task » feature
Priority: Critical » Normal
Issue tags: -Novice, -Needs change record