Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#7927 closed enhancement (fixed)

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

Reported by: markjaquith's profile 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 15 years ago.
7927.2.patch (8.8 KB) - added by Viper007Bond 15 years ago.
7927.3.patch (12.4 KB) - added by Viper007Bond 15 years ago.
Close to commit worthy, if not totally commit worthy
7927.4.patch (13.1 KB) - added by Viper007Bond 15 years ago.
Fix bugs and sorting (see below)
7927.5.patch (14.3 KB) - added by Viper007Bond 15 years ago.
7927.diff (1.1 KB) - added by DD32 15 years ago.
7927.2.diff (1.1 KB) - added by DD32 15 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 15 years ago.
7927.7.patch (2.2 KB) - added by Viper007Bond 15 years ago.
Missing parenthesis in patch 5

Download all attachments as: .zip

Change History (38)

#1 @ryan
15 years ago

Patch it up.

#2 @jacobsantos
15 years ago

  • Type changed from task to enhancement

#3 @Viper007Bond
15 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.

@Viper007Bond
15 years ago

#4 @hailin
15 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.

#5 @Viper007Bond
15 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.

#6 follow-up: @ryan
15 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().

#7 @ryan
15 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.

#8 in reply to: ↑ 6 @Viper007Bond
15 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.

#9 @Viper007Bond
15 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.

#10 @Viper007Bond
15 years ago

Also forgot phpdoc. Whoops.

#11 @ryan
15 years ago

  • Component changed from General to Comments
  • Owner anonymous deleted

@Viper007Bond
15 years ago

Close to commit worthy, if not totally commit worthy

#12 @Viper007Bond
15 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.

#13 follow-up: @Viper007Bond
15 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.

#14 @Viper007Bond
15 years ago

  • Keywords has-patch added

@Viper007Bond
15 years ago

Fix bugs and sorting (see below)

#15 @Viper007Bond
15 years ago

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

#16 @Viper007Bond
15 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.

#17 @Viper007Bond
15 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.

#18 @ryan
15 years ago

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

#19 in reply to: ↑ 13 ; follow-up: @kretzschmar
15 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.

#20 in reply to: ↑ 19 @Viper007Bond
15 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.

#21 @Viper007Bond
15 years ago

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

@DD32
15 years ago

#22 @DD32
15 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.

@DD32
15 years ago

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

#23 @DD32
15 years ago

attachment 7927.2.diff added.

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

#24 @markjaquith
15 years ago

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

#25 @DD32
15 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.

#26 @Viper007Bond
15 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.

@Viper007Bond
15 years ago

Missing parenthesis in patch 5

#27 @Viper007Bond
15 years ago

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

#28 @markjaquith
15 years ago

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

#29 @ryan
15 years ago

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