Posted by Gurpartap Singh on May 26, 2007 at 3:52pm
15 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | forum.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Gurpartap Singh |
| Status: | needs work |
| Issue tags: | Forum |
Issue Summary
Currently on the forums listing page, in the Last post column, it displays time ago and username, but it would be lots handy for the user to see the linked title.
Something like this(node title is trimmed):
This is node title...
2 hours ago
by Admin
Couldn't prepare a working patch; http://drupal.org/node/144292 needs to gets in.
Comments
#1
This is a usability feature, users usually see last update in a forum, but Drupal's forum provides no link to the latest post, making the user go into the forum and look at them. Most of the time it's just easier for the user to directly goto the latest post in any forum.
Patch is simple, but can be improved?
#2
Here's a screen shot.
#3
This is a tricky situation, as it falls into the same problems as #new links don't work across multiple pages. We probably don't want to propagate that bug into a second interface area.
#4
#5
Well, currently we are not linking to latest comment, but the node with latest comment. Btw, already working on that thing.
#6
Ok this has the requested feature. Please review that part, if it looks good, maybe it can be put into comment.module as a function(which returns an array -- the third argument for
l()) and use wherever necessary.#7
Page thing needs work.
#8
It seems way to complex requiring lots of code, which should probably be going into comment.module at this issue http://drupal.org/node/6162
So untill that is fixed, we can have link to the node itself, at least.
#9
Re-roll. It's a little bit of code to review..
#10
This looks like a great usability improvement!
But IMO this code is kind of weird....
<?php+function _forum_format($topic, $show_title = FALSE) {
if ($topic && !empty($topic->timestamp)) {
- return t('@time ago<br />by !author', array('@time' => format_interval(time() - $topic->timestamp), '!author' => theme('username', $topic)));
+ if (!$show_title) {
+ return t('@time ago<br />by !author', array('@time' => format_interval(time() - $topic->timestamp), '!author' => theme('username', $topic)));
+ }
+ else {
+ return t('!title<br />@time ago<br />by !author', array('!title' => l(truncate_utf8($topic->node_title, 25, TRUE, TRUE), "node/$topic->nid"),'@time' => format_interval(time() - $topic->timestamp), '!author' => theme('username', $topic)));
+ }
?>
What you're doing is passing in display parameters (and hard-coding HTML) in an API function. What I would recommend doing instead is changing this to a theme function, with core only providing the "show_title" way, and if people want to override that, they can do so in their themes.
Also, +1 to linking to #new. I know #new has problems on pagers, but when that bug finally gets sorted out, we can just fix it in both places, imo. Linking to the node without jumping to the new comment on the 90%+ forum posts that *don't* have multiple pages is a usability detriment, because people might not realize there are new replies unless they're observing closely.
#11
Also, why
n.title AS node_title? title is fine. It's more consistent with how this is called elsewhere, and $thread->title is descriptive enough.#12
_forum_format() is a helper function for theme functions in the module. And we want to display titles only on forum's listing. Not on node listing inside a particular forum. So that function is called for both of those listings, and we are setting the argument to send title to true only in one of them(i.e. forum listing page say ?q=forum).
Other than theme function, is there any other suggested/alternate way?
#13
Bump
#14
Re-roll.
#15
This time for new forum template files.
#16
I think this needs another reroll.
#17
Re-roll.
#18
So 6162 is fixed, which surely means a re-roll here.
What's the chances of this making it in as a usability improvement in D6? I guess slim, but won't up to 7.x just yet.
#19
#20
I don't see any reason to bump to 7.x. This breaks no APIs, and it a usability enhancement.
#21
Can anyone provide a simple patch for the latest 5.x version?
#22
Nevermind, patched it myself.
#23
Moving feature requests to the D7 queue.
#24
Marking #9844: Display name of active thread in forum overview as duplicate of this.
#25
@denny (or anyone, really) how exactly were you able to apply this patch to 5.x? 5.x seems to be rejecting everything for me when I try to apply?
#26
Is this approach worth committing? If there's any other, please point direct way to implement :)
#27
This is my modification of the patched template file;- a little less? complex, and easier to theme.
#28
This still doesn't deal with the pagin issue (per node/6162). - also this is now in advanced_forum (although there's performance issues with the current implementation) http://drupal.org/node/268273
#29
#30
What do you think of phpbb style?
#31
re: #28: The performance issues have now been taken care of. Still might be worth getting this into core, though, as I override a big chunk of core to get it working.
Michelle
#32
Now addressing #6162: #new links don't work on paged threads
#33
#34
#35
The last submitted patch failed testing.
#36
Hm, patch applies fine, and passes all tests.
#37
Gurpartap - did you cvs up?
#38
When trying to apply, hunk 2 failed to apply. attached is the reject file.
#39
yo update
#40
tested and it all works, however it is confusing as it does not link to the latest post - just the node with the latest comment. Is this the desired behaviour?
I would have thought it best to jump to the last comment of the node? (maybe the performance cost is too high?)
#41
#42
Instead of comment_num_new() could do SELECT MAX(cid) then link to the comment/$cid#comment-$cid URLs, shouldn't be any different in terms of performance.
#43
This patch should not use truncate_utf8(), especially not with the word-splitting set to TRUE. This will not work for CJK languages. See
http://drupal.org/node/269911#comment-2795478
and #200185: truncate_utf8() is used as a substring function
#44
Related issue on truncate_utf8():
#768040: truncate_utf8() only works for latin languages (and drupal_substr has a bug)
#45
The comments in #43 are wrong now -- truncate_utf8() is being fixed up so it can be used for any character set in #768040: truncate_utf8() only works for latin languages (and drupal_substr has a bug).