Should cleanup the bootstrap_status_messages() function to coincide with Bootstrap documentation.

For instance, the warning type should be a warning class, not info.
Also, I think having the titles would be nice, idk, just personal preference I guess. If anyone agrees, they should be changed from <h2/> to <h4/> like the docs.

CommentFileSizeAuthor
#6 bootstrap-1976972-6.patch1005 bytesmrded
#1 bootstrap-cleanup-status-messages-1976972-1.patch1.56 KBmarkhalliwell

Comments

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new bootstrap-cleanup-status-messages-1976972-1.patch1.56 KB

Changed the warning messages to use the warning class and changed the <h2/> to <h4/> per bootstrap docs. Added optional "info" message support, although probably not used. Do we want to remove the element-invisible class from the headers or make this a theme setting to default to hidden?

andregriffin’s picture

andregriffin commented
Status: Needs review » Reviewed & tested by the community

I think #1 is good as-is. Although it looks nice, I think displaying the headers is superfluous.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Hmm, looks like you already committed ;)

andregriffin’s picture

andregriffin commented

Hm, I'm getting used to the TortoiseGit/SmartGitHg workflow apparently... ;)

Status: Fixed » Closed (fixed)

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

mrded’s picture

mrded commented
Status: Closed (fixed) » Needs review
StatusFileSize
new bootstrap-1976972-6.patch1005 bytes

I just replaced this:

$output .= " <ul>\n";
foreach ($messages as $message) {
  $output .= '  <li>' . $message . "</li>\n";
}
$output .= " </ul>\n";

to l() function.

Check out my patch.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs review » Closed (fixed)

@mrded: re: #6 1) This should have been a separate issue. 2) I really don't want to pass a theming function through another theming function, especially when it adds additional (unnecessary) classes to a simple list. If it's desired or needed for a specific use case, override it in your sub-theme.