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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

derjochenmeyer’s picture

FileSize
446 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