WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#7927 closed enhancement (fixed)

Paged comments should show the LATEST page of comments by default, not the EARLIEST

Reported by: markjaquith Owned by:
Milestone: 2.7 Priority: high
Severity: normal Version: 2.7
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

Right now, paged commenting shows the earliest page of comments (paged at "5", it'll show #1-5).

The sorts of sites that need comment paging do so because they have really active commenting communities. By showing the first page of comments, the conversation is retarded, as commenters will get hung up on responding to first page comments.

It should show the latest page of comments. If enough people want the first page of comments to be shown, we could add an option (though I'd want them to make their case for it... in my experience showing the first page by default kills the conversation once the first page fills up... it'll just be people throwing their comments into a black hole).

Attachments (9)

7927.patch (1.9 KB) - added by Viper007Bond 5 years ago.
7927.2.patch (8.8 KB) - added by Viper007Bond 5 years ago.
7927.3.patch (12.4 KB) - added by Viper007Bond 5 years ago.
Close to commit worthy, if not totally commit worthy
7927.4.patch (13.1 KB) - added by Viper007Bond 5 years ago.
Fix bugs and sorting (see below)
7927.5.patch (14.3 KB) - added by Viper007Bond 5 years ago.
7927.diff (1.1 KB) - added by DD32 5 years ago.
7927.2.diff (1.1 KB) - added by DD32 5 years ago.
untested, but is virtually identical to the last, POC for viper's OK'ing
7927.6.patch (2.2 KB) - added by Viper007Bond 5 years ago.
7927.7.patch (2.2 KB) - added by Viper007Bond 5 years ago.
Missing parenthesis in patch 5

Download all attachments as: .zip

Change History (38)

comment:1 ryan5 years ago

Patch it up.

comment:2 jacobsantos5 years ago

  • Type changed from task to enhancement

comment:3 Viper007Bond5 years ago

See also: #7919

If we have an option to show the oldest comments first, then that option needs to make sure users get redirected to the lat page after leaving a comment.

Viper007Bond5 years ago

comment:4 hailin5 years ago

It would be nice if it is a configuration option.
Eg, the following site uses paged_comments + threaded_comments plugins,

http://stuffwhitepeoplelike.com/about/

The order shows as:

Pages: [460] 459 458 457 456 455 454 453 452 451 450 … 1 » Show All

However, if the user wants to show them in ascending order, just change a flag in order option, and that will do it.

comment:5 Viper007Bond5 years ago

My patch isn't commit worthy. I intended on adding those options but wanted to figure out the best way of doing the page thing first.

As for the pages list, if that doesn't make it into the core (we'll see), I'll be writing a plugin at least.

comment:6 follow-up: ryan5 years ago

Pages list is in core: paginate_comments_links(). I think show all support needs some work though.

As for the patch, I think we need to recalc max page in wp_list_comments() if $wp_query->comments is not used and maybe consolidate the max page code into get_num_comment_pages().

comment:7 ryan5 years ago

I think we'll need options for sort order and for page order. I think sorting by descending date is weird for comments, but oh well. Threads should be sorted ascending even if the overall order is descending, I think.

comment:8 in reply to: ↑ 6 Viper007Bond5 years ago

Yeah, I'm working on an updated patch now since my earlier patch's method seems alright.

Replying to ryan:

Pages list is in core: paginate_comments_links(). I think show all support needs some work though.

Jeeze, I really need to read through the new comment code more. Heh.

Viper007Bond5 years ago

comment:9 Viper007Bond5 years ago

I just realized that's still not commit worthy as I forget to add overall comment sorting, but it shows my progress and allows you guys to weigh in incase I'm going in the wrong direction.

This latest patch contains a new option for controlling which comments page is shown by default (the oldest or the newest, defaults to newest) and everything else works based on that. It also figures out the last page again if wp_list_comments() is passed a custom $comments array.

comment:10 Viper007Bond5 years ago

Also forgot phpdoc. Whoops.

comment:11 ryan5 years ago

  • Component changed from General to Comments
  • Owner anonymous deleted

Viper007Bond5 years ago

Close to commit worthy, if not totally commit worthy

comment:12 Viper007Bond5 years ago

Okay, latest patch and this one is "done" assuming I didn't forget anything and there aren't any mistakes (it needs checking).

It also needs UI work probably, but I'll leave that for more talented hands.

comment:13 follow-up: Viper007Bond5 years ago

Fixed a bunch of bugs I found after uploading the previous patch (isn't that always how it goes? *sigh*). I also realized that reversing the overall top level items wasn't what I wanted, I wanted to just reverse the items on that one page so that's been fixed.

Unless I discover yet more bugs, patch 4 should be ready to go.

comment:14 Viper007Bond5 years ago

  • Keywords has-patch added

Viper007Bond5 years ago

Fix bugs and sorting (see below)

comment:15 Viper007Bond5 years ago

Oh, seems if I overwrite it moves down. http://trac.wordpress.org/ticket/7927#comment:13 is the comment about patch 4.

comment:16 Viper007Bond5 years ago

Don't commit. I want to better sanitize my function (make sure we never divide by zero) and I forgot to redirect to the last page if the default page is the oldest comments.

Viper007Bond5 years ago

comment:17 Viper007Bond5 years ago

More bug fixes (mainly when threading is disabled) plus redirecting to the correct page after posting a comment.

Should hopefully be bug free, but based on the above, who knows. The biggest problem is there's so many combinations of different options to test for (threading on/off, paging on/off, order per page) that it makes it easy for something to slip through.

I'm fairly confident the code is good to go though.

comment:18 ryan5 years ago

(In [9296]) Comment paging and sorting from Viper007Bond. see #7927

comment:19 in reply to: ↑ 13 ; follow-up: kretzschmar5 years ago

Replying to Viper007Bond:

Fixed a bunch of bugs I found after uploading the previous patch (isn't that always how it goes? *sigh*). I also realized that reversing the overall top level items wasn't what I wanted, I wanted to just reverse the items on that one page so that's been fixed.

Unless I discover yet more bugs, patch 4 should be ready to go.

I just tested the comment paging in trunc. It works as expected without paging. With paging I would prefer to sort ALL comments first and then do the paging. Just reversing the items on one page is not very intuitive (for me).

If a user checks to sort new comments first he expects to have comment 9,8,7,6,5 on page 1 and 4,3,2,1 on page two.

comment:20 in reply to: ↑ 19 Viper007Bond5 years ago

Replying to kretzschmar:

If a user checks to sort new comments first he expects to have comment 9,8,7,6,5 on page 1 and 4,3,2,1 on page two.

That would mean you could never link to a comment.

Say you want to link to comment number 3. That would be /post/comment-page-2/#comment-3

When more comments arrived that created a third page, comment number 3 would then be on page 3, breaking the link to that comment.

I believe it also made for some craziness with the next/prev comment page links as the "newer" and "older" text would then be reversed and that woulda been a pain to fix.

comment:21 Viper007Bond5 years ago

Plus I doubt the average user would even notice the page number so it doesn't really matter.

DD325 years ago

comment:22 DD325 years ago

attachment 7927.diff added.

  • get_query_var('cpage') was returning 1 for every page for me. Appears to have been that line setting it to 1 regardless.

DD325 years ago

untested, but is virtually identical to the last, POC for viper's OK'ing

comment:23 DD325 years ago

attachment 7927.2.diff added.

Ignore this patch, Its rubbish :) - attachment 7927.diff added. is still OK

comment:24 markjaquith5 years ago

The previous/next comment page links should throw #comments on the end.

comment:25 DD325 years ago

attachment 7927.diff added.

It appears that patch is useless too(My local options werent set correctly), I'll leave it up to Viper007Bond for fixing up.

comment:26 Viper007Bond5 years ago

We need a get_comment_link() or whatever too.

The link-to-comment URLs (the date in the default theme) are just <a href="#comment-123".... This is problematic if we're showing the latest page by default as that'd generate http://site.com/hello-world/#comment-123 but if we get to the next page, that'd be wrong.

This function can probably get away with cheating by doing get_permalink($post_ID) . /comment-page-[current page]/#comment-123` rather than actually calculating out what page the comment would be displayed on.

Off to fix my horrible/broken custom $comments for wp_list_comments() code that DD32 attempted to fix.

Viper007Bond5 years ago

Viper007Bond5 years ago

Missing parenthesis in patch 5

comment:27 Viper007Bond5 years ago

Er, that'd be patch 6 that had the missing parenthesis.

comment:28 markjaquith5 years ago

(In [9317]) Fix comment paging for when $comments is passed in. props Viper007Bond. see #7927

comment:29 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.