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.
Attached please find a demonstration patch which moves comment display to its own tab on the node view page. Admin may choose this new view or old view.
The patch needs work still:
- comment control form redirects to the node page after submit. Should be comment page
- same for comment posting form
- probably more
I hope someone gets inspired to finish this patch. I won't revisit this for a while.
Comment | File | Size | Author |
---|---|---|---|
#19 | discuss_tab_0.patch | 10.57 KB | moshe weitzman |
#17 | discuss_tab.patch | 6.29 KB | moshe weitzman |
#11 | discuss_2.patch | 6.71 KB | chx |
#6 | discuss_1.patch | 3.58 KB | chx |
#5 | discuss_0.patch | 3.87 KB | chx |
Comments
Comment #1
moshe weitzman CreditAttribution: moshe weitzman commentedwith attachment this time
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedoh yeah - also on the TODO list is to comment URLs to always point to comment.module and never to node/x#comment-cid or node/x#comment-new and so on. then comment.module can work out whether to show you the discuss tab or the traditional view.
Comment #3
Boris Mann CreditAttribution: Boris Mann commentedI like the concept. Would want this to be part of workflow settings, so that comment-tab could be enabled/disabled per content type.
Comment #4
Jaza CreditAttribution: Jaza commentedI like the idea - big +1. This is a must for sites that have long articles and/or many comments - viewing the node is enough bandwidth as it is, without all the comments getting displayed as well. Also improves usability, IMO.
Would also like to see pagination of the comments tab (I'm sure you've thought of this already, Moshe). Something similar to A List Apart's system, e.g. node/x/discuss/2 (page 2 of that node's comments). Or, if the existing pager system demands it, node/x/discuss?page=2.
Comment #5
chx CreditAttribution: chx commentedLet's discuss! Like do we want to have a reply form on the node display even if the comments are on the discuss page?
Comment #6
chx CreditAttribution: chx commentedI have uploaded moshe's patch. Let's try again.
Comment #7
Bèr Kessels CreditAttribution: Bèr Kessels commentedI like this approach a lot. However, as i use drupal also for blogging, would dislike my bog to behave the same. Thus there must be some -easy- way to get around this. µ
The problem lies mainly in the fact that tabs are not administratable in Drupals menu admin. And I think it is a very bad idea to add a config option where to place the comments. We should go for one way, teh default way, and offer a menu-admin of some sort to change this, for bloggers and slashdot alikes.
How to achieve this? I see two options, but I really hope people see more then one.:
- make local-tasks appear in the menu admin (gets a huge +1 from me) It's a separate issue, though.
- add a config option to the comment admin to allow comments either as tab or as thread under the post. IMO that config option should not set a variable, that gets checked every hook_menu call, but it should alter the menu-entry in the database for the tabs Its possible, I did that before.
But, as said, I hope others have better ideas. for I really would love this feature to get in. But I am afraid a huge lot of people will move away from drupal if we allow tabs only.
Comment #8
chx CreditAttribution: chx commentedNo, not tabs only. I know the drupal_gotos are giving you this impression, but will fix in the evening. Also, there is only a variable_get for node/nid pages and then again, variable_get is double cached, it's very fast.
Comment #9
Bèr Kessels CreditAttribution: Bèr Kessels commentedI know variable is fast, but I was referring to:
&& variable_get('comment_task', FALSE)
that is evaluated every node view.As well as the setting in the comment options. I am no fan of such options; However, it seems in this case that setting is sortof required, unfortunately. So unless others come up with better ideas, just ignore my previous comment/post.
Another idea: put this in hook_nodeapi() $op == "settings". that way this setting can be used per type. I can imagine that this makes no sense for forums and blog entries, yet is very usefull for the books on that same site.
Comment #10
Bèr Kessels CreditAttribution: Bèr Kessels commentedsorry, with "Another idea: put this in..." I meant to say "Another idea: put the settings for tabs vs threads ..."
Comment #11
chx CreditAttribution: chx commentedComment URL is introduced. Instead of
node/$nid#comment-cid
we havecomment/$nid#comment-cid
Other minor fixes.
Comment #12
Boris Mann CreditAttribution: Boris Mann commentedComment URLs are great, except I'd love to have a *full* comment URL -- e.g.
comment/$nid/$cid
. The probably with anchor tags is that they are not visible at the server level -- i.e. not tracked in referrers, etc. etc.Don't want to hijack this issue, so feel free to ignore for now, but wanted to bring up the issue from a search/web traffic/etc. perspective.
Comment #13
nedjo+1 in principle. (I've looked over but not applied the code.)
(Optional) tabbed display is a useful enhancement and justifies the extra configuration option. I also like the new comment URL.
Comment #14
nsk CreditAttribution: nsk commentedAs a user I have no problem with this as long as I can disable it and use the old view. Please note that there is nothing special or innovative in Wikipedia's use of the comment page (talk page), it's just a feature of MediaWiki, the software they use. Many other sites use the same software. IMO it's bad but others may need, so it's probably good to add something similar in Drupal as long as it's not the default.
Comment #15
moshe weitzman CreditAttribution: moshe weitzman commented@nsk (and others) - please do not post simple "like/dislike" comments when an issue is in 'rpatch: code needs review' status. the review guidelines are at http://drupal.org/patch/review. thank you.
Comment #16
Bèr Kessels CreditAttribution: Bèr Kessels commentedDiscussion about a patch should go further then just comments on the code. NSK (and I) just did what the patch review guidelines say under "Challenge the proposed changes.".
Comment #17
moshe weitzman CreditAttribution: moshe weitzman commentedNote: here is a patch which build upon chx's patch, with the changes i describe below. I'd like another person to review this patch before we promote it.
1. I like that access to the new tab is switchable by node type, but you use the permission system to do this. Also, are these permissions defined somewhere? Specifically,
To me, it makes more sense to put this switch on the content-types admin pages and leave comment permissions as a global, not node type specific. This adds a couple node_load() calls but we are saved by the node cache.
2. Here is a description for the checkbox "Show comments on own tab.": "If unchecked, comments will be shown directly beneath the corresponding post, instead of in a tab called discuss"
3: in comment_task(), you have some commented out lines that look useful. You can avoid calling comment_render() when there are no comments by inspecting $node->comment_count . Also use that element instead comment_num_all() in hook_menu().
4. consider adding a 'create new comment' link at the top of the new 'discuss' page (similar to forum/x page).
5. I removed line 792 - looks like a misplaced cut and paste.
6. new discuss page was printing its own output. @chx - you are losing your memory?
Thanks for enhancing this patch.
Comment #18
chx CreditAttribution: chx commentedFirst, let me give you background of this patch: I was contracted for some comment module (4.6) work. Per node type and print theme page stuff comes from that. I was trying to give you a good port forward and eradicating all the per content type stuff. Sorry for the remnants.
Comment #19
moshe weitzman CreditAttribution: moshe weitzman commentedfix last buglet with comment control panel
Comment #20
breyten CreditAttribution: breyten commentedI would like to see this feature, but as it is the patch doesn't apply cleanly -- it's written before the forms patch.
Comment #21
moshe weitzman CreditAttribution: moshe weitzman commentedAnyone want to take this on? HEAD is open again.
Comment #22
Permanently Undecided CreditAttribution: Permanently Undecided commentedThis would be really useful. I wish I understood forms API, I'd give a hand to convert it. :( I hope somebody does!
Comment #23
chx CreditAttribution: chx commentedComment #24
Christefano-oldaccount CreditAttribution: Christefano-oldaccount commentedThis discussion has started up again at http://groups.drupal.org/node/2931
Comment #25
catchBumping another version. This gets even more interesting now that comment settings are per node type.
Comment #26
moshe weitzman CreditAttribution: moshe weitzman commentedfurthermore, custom_url_rewrite_outbound can rewrite comment urls so they point to the tab, and not to an anchor on the node page. i think that was the last obstacle ... well, comment module still blows but this issue doesn't have to tackle.
Comment #27
bsherwood CreditAttribution: bsherwood commented+1 subscribing
Any word on this getting in for 7.x?
Comment #28
lilou CreditAttribution: lilou commented@ christefanø : not the same issue.
Comment #29
moshe weitzman CreditAttribution: moshe weitzman commentedComment #30
Frando CreditAttribution: Frando commentedThere is a contrib module for this, see http://drupal.org/project/talk
I'm not sure if this is really needed in core, contrib might be just fine.
Comment #31
webchickI'm also not sure this belongs in core. Seems like a very specific use case only for wikis, which 90% of Drupal sites are not.
Comment #32
moshe weitzman CreditAttribution: moshe weitzman commentedyeah, i'll buy that