Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
#936304: [META] Style issue comments has recommendations for styling system messages.
The additional classes needed to identify system messages should be added in a drupalorg module preprocess function.
These type of messages usually correspond to the "colored" states in issue listing tables. It's a natural progression of quickly identifying automated messages in issues and should be themed differently from user provided content. Classes will need to be added to the .comment
div to allow theming of different types:
- General
.comment.system-message
- This is a general style for system messages that do not have any particular correlation. Coloring matches the files fieldset background/buttons on issue edit. - Testing Failed
.comment.system-message.testing-failed
- This matches the "Needs work" status in the issue listing tables. - Testing Queued
.comment.system-message.testing-queued
- This matches the "Needs review" status in the issue listing tables. - Committed
.comment.system-message.committed
- This matches the "Fixed" status in the issue listing tables. Generally speaking, there is usually only one commit per issue, so an issue is usually marked "Fixed" once a commit is made. - Closed
.comment.system-message.closed
- This matches the "Closed (fixed)" status in the issue listing tables.
Comment | File | Size | Author |
---|
Comments
Comment #1
drummComment #2
drummScreenshot of the hover state from Mark Carver:
Comment #3
drummThis is ready for initial review at https://drumm-drupal.redesign.devdrupal.org/node/2223537.
Notes:
Comment #4
Bojhan CreditAttribution: Bojhan commentedIt's a bit tricky to review without the real CSS colors. A few things I noticed:
- The hover on messages causes the text to wrap to the next line.
- The message #, how relevant is that? When do you refer to a test that didn't pass?
- The actual text is a little hard to scan, both because of the gray on gray and the actual terminology used.
- We probably shouldn't be using a special font size for these messages.
Comment #5
drummNew URL to review: https://drumm2-drupal.redesign.devdrupal.org/node/2257197
All the greys are the real colors, so that is reviewable.
Comment #6
markhalliwellComment #7
markhalliwellComment #8
markhalliwellComment #9
markhalliwell@drumm, while working on #2275317: Cleanup commit message theme hook, I noticed that there are supposed to be
<pre>
tags around the commit message, however they are not currently showing up anywhere. I don't see any commits that have possibly stripped them out, however I do seefield-name-comment-body
is setup for multiplefield-items
(commits). Is the field somehow stripping these out?Actually, it just dawned on me: System Message is posting using the
Filtered HTML
filter (which doesn't have<pre>
). I'll create an issue to resolve this as well.Comment #10
markhalliwellComment #11
tim.plunkettClosed #2275911: Give System User its own user picture as a dupe :)
Comment #12
DamienMcKennadrumm closed #2276395: The System Message comments should use Druplicon as the avatar as a dupe too.
Comment #13
markhalliwell@drumm, could you add the fix for #2275363: Add <pre> to list of allowed tags in Filtered HTML to https://drumm2-drupal.redesign.devdrupal.org/node/2257197#comment-8774563 as well? Thanks!
Comment #14
drummI managed to lose the code for this in the DrupalCon commit shuffle. It is back now, ready for review, and
<pre>
is allowed.A couple good examples are:
https://drumm2-drupal.redesign.devdrupal.org/node/2250429
https://drumm2-drupal.redesign.devdrupal.org/node/2228241 has multiple commits in the same comment.
Comment #15
markhalliwellLooks good. I have a few trivial comments that could be addressed as a follow-up (only if necessary):
Comment #16
tim.plunkettActually, I delete system messages *all the time* in order to stay below the 300 comment threshold.
But, I could reshow that in a userstyle if its hidden with CSS.
Comment #17
drummIf we remove the links, which I was going to do, but missed, the pages will still exist by direct URL. If we don't remove them here, or if we do and it isn't great, there's #2272951: Style comment links.
And #2272953: Style node changes in issue comments for the changes.
Comment #18
markhalliwellWell, yet again, I'm not privy to such links and don't think about those kinds of things generally. Regardless it's very trivial and shouldn't block the majority of the styling that needs to make it on d.o. The issues @drumm mention should be sufficient to address them.
Comment #19
drummOver in drupalorg, committed the PHP that adds classes, etc: http://cgit.drupalcode.org/drupalorg/commit/?id=6a776ba
Comment #21
drummThis is now on staging, will be deployed this afternoon.
Comment #22
drummNow deployed.
Comment #23
markhalliwellNoticed a couple things:
Un-aligned:
Aligned:
CSS fix:
Comment #26
drummI implemented the CSS fix slightly differently, to avoid the negative margin.
Comment #27
drummNow deployed.