WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 18 months ago

#7635 closed enhancement (fixed)

Threaded Comments

Reported by: ryan Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version:
Component: General Keywords: blessed
Focuses: Cc:

Description

Support threaded comments.

Attachments (14)

7635.diff (8.7 KB) - added by ryan 6 years ago.
7635.2.diff (10.2 KB) - added by ryan 6 years ago.
Add style and callback args to wp_list_comments(), add cancel_comment_reply_link().
7635.3.diff (10.3 KB) - added by ryan 6 years ago.
Refresh for current trunk
7635.4.diff (2.4 KB) - added by DD32 6 years ago.
7635.5.diff (2.4 KB) - added by DD32 6 years ago.
comment-styling-kubrick-rev1.patch (2.6 KB) - added by noel 6 years ago.
Comment styling for kubrick, take 1
commentstyling-rev2.patch (1.6 KB) - added by noel 6 years ago.
Comment styling
show-reply-via-jquery.patch (1.9 KB) - added by Viper007Bond 6 years ago.
Hide threaded reply link, show via jQuery
comment-reply-jquery.patch (2.3 KB) - added by Viper007Bond 6 years ago.
Move all of comment-reply.js to jQuery + trim filesize in half
reply-to-comments-nojs.patch (4.2 KB) - added by Viper007Bond 6 years ago.
Foolin' around with no-JS fallback for replying to comments, may not be ready for commit
separate_comments.diff (1.9 KB) - added by ryan 6 years ago.
Add separate_comments flag to comments_template()
avatar-size.diff (940 bytes) - added by Otto42 6 years ago.
Patch to add avatar_size parameter
7635.6.patch (2.7 KB) - added by azaozz 6 years ago.
threaded-comments-options-simplification.patch (2.7 KB) - added by Viper007Bond 6 years ago.
Move depth to dropdown per Matt's request + tweak translation stuff (translation stuff needs review)

Download all attachments as: .zip

Change History (69)

comment:2 ryan6 years ago

Patch introduces wp_list_comments(), which uses a new comment walker class. A custom class can be passed to wp_list_comments() if someone doesn't like the layout in the default class. Ideally, wp_list_comments() would be very customizable so that replacing the walker class isn't necessary.

Default theme patched to use wp_list_comments() and friends.

Two new JS functions ripped from Threaded Comments plugin and cut down to the minimum. The thread expand/collapse from the plugin is not supported and probably won't be a core feature.

This is very, very rough and ugly. First draft.

ryan6 years ago

comment:3 Viper007Bond6 years ago

  • Type changed from defect to enhancement

ryan6 years ago

Add style and callback args to wp_list_comments(), add cancel_comment_reply_link().

ryan6 years ago

Refresh for current trunk

comment:4 ryan6 years ago

Thoughts on the patch? Does it seem like a good approach to go forward with? If so, I can commit and we can go about getting the bundled themes using the new API and looking decent.

comment:5 ryan6 years ago

(In [8869]) wp_list_comments() and threaded comment support. First cut. see #7635

comment:6 ryan6 years ago

Todo: Reply links should be hidden by default and enabled with JS.

comment:7 ryan6 years ago

Default theme needs styling: Align gravatars on right or move to left. Fix alt for child comments. Fix overlap when replying.

comment:8 scohoust6 years ago

It's pretty good, although I think some settings in the admin area (easy enable/disable and perhaps depth of comments spring to mind) might be something to consider for some users.

comment:9 ryan6 years ago

Any options are dependent on if the theme supports threaded comments. If we put options in the admin they won't do anything without theme support. That's not so nice. Maybe we can make it easy for themes to add these settings to a theme options page.

comment:10 DD326 years ago

Notes:

  • supports ol|ul|div style
  • if callback registered for start_el, end_el is assumed to have been taken care of by start_el callback
  • "class="classname"" html fixed (was quoted)
  • Creates Valid XHTML for nested lists, HOWEVER:
    • Nested lists must be within their own <li>, The result is that nested lists get a bulletpoint.. You can hide the bullet, but with a <ul> mearly hiding it results in a number being skiped.
    • So: Valid XHTML & dodgy numbers with UL, or Invalid XHTML but good list points?

DD326 years ago

comment:11 DD326 years ago

So: Valid XHTML & dodgy numbers with UL, or Invalid XHTML but good list points?

It may be i'm mistaken here.

I was certain that a nested list in XHTML had to be

<li>Parent..</li>
<li><ul><li>Child</li></ul></li>

(ie. in its own holder)

But it appears thats not the case, the UL can go into the parent list, So this means that a bit more trickery needs to be done.. I'll have a furthur play with it.

comment:12 DD326 years ago

Eugh, Turned out it was due to the my callback change that buggered it up.

Patch is the same as before, But adds end-callback for overriding end_el(), which is needed if the callback requires something other than </div> or </li>

Also creates valid xhtml now.

DD326 years ago

comment:13 ryan6 years ago

(In [8876]) Threaded comments improvements from DD32. see #7635

comment:14 ryan6 years ago

(In [8877]) Add respondID and respondRoot args to cancelCommentReply(). see #7635

comment:15 follow-up: ryan6 years ago

(In [8878]) Add depth handling to comment_reply_link(). see #7635

comment:16 in reply to: ↑ 15 DD326 years ago

Replying to ryan:

(In [8878]) Add depth handling to comment_reply_link(). see #7635

 	851	 
 	852	    if ( 0 == $args['depth'] || $args['max_depth'] < $args['depth'] ) 
851	853	 
852	854	    extract($args, EXTR_SKIP); 

Looks like you've missed a return there?

comment:17 ryan6 years ago

(In [8897]) Add type argument to wp_list_comments(). Make comments arg optional. Introduce separate_comments(). see #7635

comment:18 ryan6 years ago

(In [8898]) Add missing return. Props DD32. see #7635

comment:19 ryan6 years ago

Added a type argument to wp_list_comments(). Type can be comment, pingback, trackback, or pings. pings includes both tracbacks and pingbacks. Comments are separated with separate_comments(). Themes can call this themselves instead of relying on wp_list_comments() to do it if desired.

comment:20 follow-up: Speedboxer6 years ago

How about integrating threaded comments into the reply feature in the comments admin? So that, if the selected theme has threaded comments implemented, replys to comments will be threaded under the parent comment.

comment:21 in reply to: ↑ 20 Speedboxer6 years ago

Replying to Speedboxer:

How about integrating threaded comments into the reply feature in the comments admin? So that, if the selected theme has threaded comments implemented, replys to comments will be threaded under the parent comment.

Nevermind, it seems to do this already. :)

comment:22 ryan6 years ago

(In [8941]) Fix depth inversion. see #7635

comment:23 ryan6 years ago

(In [8961]) First cut of comment paging. Add paging and threading settings. see #7769 #7635

noel6 years ago

Comment styling for kubrick, take 1

comment:24 ryan6 years ago

Maybe we should apply alt styling to entire threads instead of individual comments or drop alt styling altogether. Or, put comment_class() on the comment div instead of the li for list mode.

comment:25 ryan6 years ago

(In [8962]) Add thread alt classes. see #7635

comment:26 ryan6 years ago

(In [8963]) s/list/ul/ see #7635

noel6 years ago

Comment styling

comment:27 ryan6 years ago

(In [8971]) Comment styling for default theme. Props noel. see #7635

comment:28 Otto426 years ago

I'm converting my comments.php to use the new stuff. Tricky-ish.

Question: How can I make it default to the last page of comments instead of the first page?

comment:29 Otto426 years ago

Bug: If you're using this, the gravatar size is fixed at 32 px with no way to adjust it other than editing the core code.

What's best? A filter? A global? A parameter to wp_list_comments()?

comment:30 Viper007Bond6 years ago

Parameter or perhaps an option in the admin area. Filters and globals are a pain for the average user.

Viper007Bond6 years ago

Hide threaded reply link, show via jQuery

comment:31 ryan6 years ago

(In [9037]) Hide reply link by default. Enable with JS. Props Viper007Bond. see #7635

comment:32 ryan6 years ago

(In [9039]) If comment type is empty set it to 'comment'. see #7635

comment:33 Otto426 years ago

A good point was brought up.. If you can do wp_list_comments('type=pings'); or what have you, then it would be nice to know if there are any comments of that type before you call it. Like have_comments('type=pings') or something similar.

This is problematic because as near as I can see, the $wp_query->comments_by_type doesn't get setup until you have already called wp_list_comments with a non-all type. Perhaps the query should always separate the comments when they are pulled out of the DB.

comment:34 Viper007Bond6 years ago

It'd be nice if the comment pages had pretty URLs.

Viper007Bond6 years ago

Move all of comment-reply.js to jQuery + trim filesize in half

comment:35 azaozz6 years ago

(In [9082]) Move all of comment-reply.js to jQuery + trim filesize in half, props Viper007Bond, see #7635

Viper007Bond6 years ago

Foolin' around with no-JS fallback for replying to comments, may not be ready for commit

ryan6 years ago

Add separate_comments flag to comments_template()

comment:36 ryan6 years ago

Patch adds flag to comments_template() for requesting comments to be separated by type.

comment:37 ryan6 years ago

(In [9092]) Add arg to comments_template() for requesting separating comments by type. see #7635

comment:38 ryan6 years ago

(In [9093]) before and after for comment_reply_link(). see #7635

Otto426 years ago

Patch to add avatar_size parameter

comment:39 follow-up: Otto426 years ago

Here's a patch to add an avatar_size parameter to wp_list_comments(). It will let you have the avatar any size you like, or not have it at all with it set to zero. Tested on latest trunk and it works fine.

comment:40 in reply to: ↑ 39 ; follow-up: josephscott6 years ago

Replying to Otto42:

Here's a patch to add an avatar_size parameter to wp_list_comments(). It will let you have the avatar any size you like, or not have it at all with it set to zero. Tested on latest trunk and it works fine.

The get_avatar() call already checks the show_avatar option to determine when avatars should be shown.

comment:41 in reply to: ↑ 40 Otto426 years ago

Replying to josephscott:

Replying to Otto42:
The get_avatar() call already checks the show_avatar option to determine when avatars should be shown.

This lets the theme override that, if desired.

azaozz6 years ago

comment:42 azaozz6 years ago

Attempting to make the js a bit more generic so it can be used in most themes without changes.

comment:43 azaozz6 years ago

(In [9112]) No-JS mode for replying to comments and some improvements/cleanup of comment-reply.js, includes patch by Viper007Bond, see #7635

comment:44 azaozz6 years ago

The above patch makes several changes: introduces 2 new template tags used to allow replying without js: comment_parent_field() outputs the hidden input field and comment_form_title() outputs a different title for the form when replying (optional).

Changes: cancel_comment_reply_link() takes only one argument text = ' ', the rest is set from js. The DIV surrounding it shouldn't be hidden (for supporting no-js mode) and is optional.

comment:45 azaozz6 years ago

(In [9113]) No jQuery requirement for comment-reply.js, see #7635

comment:46 azaozz6 years ago

(In [9114]) Use no-js reply on error in comment-reply.js, see #7635

Viper007Bond6 years ago

Move depth to dropdown per Matt's request + tweak translation stuff (translation stuff needs review)

comment:47 ryan6 years ago

(In [9175]) comment_id_fields(). see #7635

comment:48 Otto426 years ago

The Walker_Comment's start_el() function needs to pass all the $args, merged with the three fields it must send anyway, down to the comment_reply_link function, making those fields available to be set in the calls to higher level functions.

For example, when calling wp_list_comments, there's currently no way to set the text of the "Reply" link. If the $args were passed along, one could simply add a reply_text parameter to the call.

comment:49 Otto426 years ago

Also, there's still no way to set the avatar_size. Can we get that patch added in already? Having to hack the code manually every time I svn up is getting annoying.

comment:50 ryan6 years ago

(In [9206]) Threaded comments options cleanup. Props Viper007Bond. see #7635

comment:51 ryan6 years ago

(In [9207]) Add avatar_size arg. Props Otto42. see #7635

comment:52 ryan6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Closing this ticket as fixed. Otto42 can you start a new ticket with patch for your args request?

comment:53 Otto426 years ago

ryan: Done. See #7901.

comment:54 gaveideer18 months ago

For the record #7901 is also closed.


Status changed from new to closed

Resolution set to fixed

Otto42 can you start a new ticket with patch for your args request?

(In [9207])gaveideer
Closing this ticket as fixed.

Version 0, edited 18 months ago by gaveideer (next)

comment:55 gave_ideer18 months ago

*spam removed*

Last edited 18 months ago by Otto42 (previous) (diff)
Note: See TracTickets for help on using tickets.