Special comment theming
maulwuff - March 15, 2008 - 16:49
| Project: | Advanced Forum |
| Version: | 6.x-1.x-dev |
| Component: | Styles |
| Category: | task |
| Priority: | normal |
| Assigned: | Michelle |
| Status: | won't fix |
Description
I noticed the following:
Browsing the forum as user #1 does not differentiate between published and unpublished comments. I unpublished a comment, and nothing happened - was confusing.
So I came up with this: Color the left part of a comment in another color to make unpublished comments noticable.
In the module file modify:
<?php
function advanced_forum_preprocess_comment(&$vars) {
if (_is_forum('comment',$vars)) {
// Use our combined node/comment template file
// D5 won't find templates in subdirectories so we need to give it that
$forum_theme = advanced_forum_get_forum_theme_directory();
$vars['template_files'][] = "$forum_theme/advf-forum-post";
#mod start
$vars['unpub'] = $vars['comment']->status;
#mod end
.......
?>in advanced_forum.css add
.forum-post-wrapper-unpub {
background: #808080 url(images/forum-separaterRed.jpg) repeat-y;
border-top: thin solid darkgray;
border-bottom: thin solid darkgray;
}Next, save the attached image to your themes/xyz/advforum/images directory, or use your own file. I called it forum-separaterRed.jpg
Last, edit advf-forum-post.tpl.php and add at about line 62:
leading:
<div class="clear"></div>insert:
<?php if ($unpub) : ?>
<div class="forum-post-wrapper-unpub">
<?php else : ?>
<div class="forum-post-wrapper">
<?php endif; ?>following:
<div class="forum-comment-left">| Attachment | Size |
|---|---|
| forum-separaterRed.jpg | 362 bytes |
#1
Hmm.. I'm on the fence on this one. Theming unpublished nodes/comments differently seems like it should be part of the over all site theme . But I suppose it wouldn't really hurt to add it. Any other opinions on this?
Michelle
#2
This would be a fantastic addition IMO. Like the OP, I often get confused which posts are published/unpublished while administering forums.
#3
Ok, what the heck, I'll put it in. People can always take it out of their themes if it clashes with what their main theme has set.
While I'm at it, I'll add in a class for own comments, though I don't think I'll add any default CSS for that...
I also had someone ask once for a little something extra on new comments. Might as well do that while I'm at it.
Anything else?
Michelle
#4
This might be another task/module entirely or too large of a project, but since you're listening... :)
What about a class for replies from the Author/OP?
Sony's official playstation blog implemented this not too long ago with fantastic results (see attached). They recently released their work as a wordpress plugin.
Again, I really have no idea how hard this would be to implement but I figured I may as well suggest it.
Thanks as always Michelle!
#5
Sure, I can do that, too.
I was going to work on this today but the advanced profile queue is getting really bad. I'm going to take a short break from forums and give that a bit of love. Should be able to get to this issue within the next day or two because it's an easy addition. I know just how to do it... Just need to take a bit of time and add it in.
Michelle
#6
applies still to aplha10
#7
Thanks but there's no need to update version on old issues. Whenever I have time to work on this I look at all the open issues, regardless of versions, to see what I can tackle.
Michelle
#8
ok, just had to redo this procedure because I updated to alpha10! :P
#9
Consolodating into #268684: Retheming - one way forward.
Michelle
#10
Unconsolodating since it got missed.
Michelle
#11
some help for you, based on D5 alpha12.
sorry for not providing a patch, I've got too much changed in the module.
it is a 3-step change:
1. in function advanced_forum_preprocess_comment
add this:
$variables['unpub'] = $variables['comment']->status;2. in advf-forum-post.tpl.php
at about line 63 add this:
leading code:
<?php if (!$top_post): ?><span class="post-num"><?php print $comment_link . ' ' . $page_link; ?></span>
<?php endif; ?>
</div>
insert:
<?php if ($unpub) : ?><div class="forum-post-wrapper-unpublished">
<?php else : ?>
<div class="forum-post-wrapper">
<?php endif; ?>
3. in advanced-forum.css
at around line 57 add this: (the line number may be completely wrong)
leading:
/*** FORUM THREADS ***********************************************************/
.forum-post-wrapper {
background: #F7F5EE;
}
insert:
.forum-post-wrapper-unpublished {background: #FFC8BB;
}
adjust the color to your need.
hope this saves you some time.
#12
Thanks for the effort but that isn't how I'm planning on doing it. I'll get to this as soon as we get the layout problems fixed. No sense in me adding to the CSS when it's in flux.
Michelle
#13
I've given this issue a lot of thought and I keep coming back to my original feeling that this CSS is the theme's responsibility, not AF's. However, AF needs some way of knowing what the theme wants. I took a look at some of the major themes as well as #306358: Add $classes to templates with new theme_process_hook functions and it looks like the goal is to use the variable $classes to hold these special classes. Some themes are still using $node_classes and $comment_classes as well. So I ended up merging these into $classes to cover both possibilities and adding it to advf-forum-post.tpl.php. At the moment, it's only on the naked style and only in D6 as I'm adding it in along with other work being done on the forum topics. I'll leave this active until it's on all styles in both branches but wanted to give an update with "the plan".
Michelle
#14
This is now in all the styles in D6 and will end up in D5 when I copy the styles over after some other stuff gets fixed.
Michelle
#15
Automatically closed -- issue fixed for two weeks with no activity.
#16
Did this get taken out again? I'm using latest (I think) 6.x-1.x-dev and noticed my unpublished comments look the same as published ones... using firebug to look at classes and stuff it doesn't look like it's got anything for 'unpublished'... so I'm not sure if I'm missing something, but I can't seem to find a class to style.
#17
Nope, it's still there. Maybe the theme you're using isn't doing anything special with unpublished comments.
Michelle
#18
I guess that's possible but I don't see a class showing up for unpublished comments as distinct from published ones. If I did, I'd just write some CSS. But they do not appear to be distinguished at all so I'm guessing something is maybe interfering? The class for unpub gets applied in preprocess somewhere?
I'm just trying to figure out why the source doesn't look any different... no hint of unpub status at all.
I don't know if these means anything but, oddly, if I put a
<?php if ($comment->status == comment-unpublished) print ' comment-unpublished'; ?>in various places in advf-forum-post.tpl.php, all the corresponding published comment divs get "comment-unpublished" added to their classes (and they turn colors accordingly), but the unpublished ones don't. But if I change the operator to doesn't equal...!= comment-unpublished, the unpublished ones get the "comment-unpublished" class.So, in short, I can make unpublished posts look unpublished now, but how it's working doesn't make any sense to me unless something is incorrectly reversing the status of the comments & posts somewhere.
#19
"The class for unpub gets applied in preprocess somewhere?"
Ah, that's where the misunderstanding lies. No, that would get applied in your theme. All AF does is print the two most common variables that themes use to add this sort of class. It doesn't create the variables itself. If you don't have a theme that creates variables for unpublished comments then you won't see anything in AF, either.
Michelle
#20
OK. Got it. Will just build it in there somewhere. Tks.
Edit: actually, I'm completely confused. Isn't the only variable involved for this $status which is set in the core? My theme has a comment.tpl.php that assigns a class based on the status variable...but it only seems to "work" when I have AF turned off. Anyway, I've got a topic open in the theming forum so somebody there will probably straighten me out.
#21
See http://drupal.org/node/234593#comment-1090661
Michelle
#22
Way over my head I'm afraid. I think I'm just going to do what I did in #18 and hope it keeps working, though I don't really get why it works at all.
#23
Sorry, I didn't look that closely at your code before. I know why it's working. You have:
<?phpif ($comment->status == comment-unpublished) print ' comment-unpublished';
?>
Since "comment-unpublished" isn't defined as anything, it's essentially 0. So you're saying if status is 0, print ' comment-unpublished'. For some reason Drupal has things backwards and puts the status the reverse of what you'd expect. I believe this got fixed in D7.
Michelle
#24
Here's the issue about status 0 being published: http://drupal.org/node/237636
Michelle
#25
Ah.... emerging slowly from fog. Tks.
#26
Automatically closed -- issue fixed for 2 weeks with no activity.
#27
I feel kinda awkward reopening a topic that has been given a lot of thought already, but I can't escape the feeling something's still wrong here. So I'd still like to rewind comment #16... "it still doesn't work".
Objective observation:
With advanced_forum (naked style) installed on the bare standard installation (that is, D6 + Garland), unpublished forum-nodes & comments do NOT have the color that Garland would give them if advanced_forum wasn't installed.
Does that classify as a 'bug'?
The reason I believe to be simple enough:
In #13 up here you referred to #306358: Add $classes to templates with new theme_process_hook functions. It's wonderful that for D7, people have swept all kinds of loose properties up into $classes (and maybe $comment_classes / $node_classes). However... that's for D7. In D6 (or at least in 'bare standard D6'), $classes/$comment_classes/$node_classes are undefined. So printing these variables in advf-forum-post.tpl.php doesn't do a lot.
So I do think something like was mentioned in #18, is indeed needed 'for backward compatibility' until your code hits D7. The attached little patch does the same, the code is just a little different (it acknowledges that we want to put stuff in $classes).
#28
This has nothing to do with D7 but rather the general direction themes are going in D6. Of the themes I sampled, most use either $classes or $node_classes. In all those themes, this will already work. Garland is the odd duck in that it hard codes the classes in the template. By putting your patch in, themes that already use $(node_)classes will get the "node-unpublised" twice. I could put in extra logic to see if it's in there but, frankly, I don't want to jump through hoops for Garland anymore than I have to.
If this is something you need, it's easy enough to put it in your node preprocess and there is no need to hack the module, so I'm going to pass on this patch.
In investigating this, though, I found a bigger problem in that the blue_lagoon style covers up the theming on unpublished. I will make a new issue for that.
Michelle
#29
But but but but but... hrm okay ;-)
[ Note: this is not meant as a way to convince you of anything, but just as a documentation note for people with my skill level who google 'forum unpublished comments' and end up here ]
I should have reread that chapter about theming in my Drupal book first.
Because when reading this topic before, there was something I didn't get. I was only seeing the whole 'template file' thing. I thought "but... but... the theme does not include your own advf-forum-post.tpl.php, does it? It's EITHER your advf-forum-post.tpl.php OR the theme's whatever.tpl.php that gets executed; they're not stacked."
And that isn't really true. What actually IS called from the theme, before your advf-forum-post.tpl.php, are 'preprocess functions'. In this part, the function THEMENAME_preprocess_comment() is the one that should be filling $classes. And what you're debating in #13, is whether filling $classes should be done inside advanced_forum_preprocess_comment() or inside THEMENAME_preprocess_comment(). (At least for D6.)
(Yeah I know, I guess that should be obvious from the inital post here. But it didn't 'click', with all the other .tpl.php snippets from other commenters.)
So now I get it. And I guess you're right. That should be done in THEMENAME_preprocess_comment().
(I'm not sure whether Garland is that "oddball". I have tried three existing themes that I considered using on my own little blog: abarre, painted & colorpaper. None had a THEME_preprocess_comment() that filled $classes. But I'm totally willing to believe that:
- almost everyone who's serious about theming uses Zen (or another base theme that has a preprocess function) anyway;
- those people are the 'audience' of advanced_forum anyway.)
OK then, I'll leave this be.
Signing off with a last note: my patch was stupid. It has a comment in the HTML part (not the php part) so it prints the comment in the HTML. :(
#30
Unfortunately, a lot of themes copy Garland's bad habits. The big base themes tend to do things more properly, putting more in the preprocess functions. As you've realized, that's where theming that isn't specific to forums should go. Any theme that uses $node_classes or $classes will have that picked up. I can't pick up on Garland's automatically because it is hard coded in the .tpl. Not sure what to do about the new $attributes movement, yet. I may end up going that route instead if it becomes more popular in contrib themes.
It's tricky trying to theme the forums without trampeling on the site's theme or getting into its territory. There's going to be some issues that just don't have a good generic solution given the number of themes out there that havent' standardized on things like this. The good news is that anyone can add stuff like this in just by sticking a preprocess in their theme.
Michelle
#31
Since there's a lot of back and forth in this discussion ... I want to present what I just did for a) feedback, and b) to summarize what I think the solution is.
In comment.module it sets $variables['status'] to comment-unpublished for an unpublished comment. Further if a comment has the class 'comment-unpublished' it automatically gets a color in its background. One approach is in advf-forum-post.tpl.php $classes .= $status takes care of that. But you're suggesting that isn't the best approach and prefer that the theme take care of this.
My site is using the newsflash theme and it doesn't do this. However newswire has newswire_preprocess_comment which does the right things, so I copied newswire_preprocess_comment into newsflash/template.php, renamed it to newsflash_preprocess_comment, and now my comments have the right background color when unpublished.
No changes in the advanced_forum module are required.