Reader: More sensible comment fetching

[blowery]

Right now, we're fetching comments the same way that the native apps do. We ask for the first 20 top level comments and all of their kids. This works for the large majority of our posts, as most posts do not have more than 20 top level comments, but it does not work (at all) when we have more. It also doesn't let us poll for new comments easily.

I'd like to ditch hierarchical fetching and instead fetch the first 50 top level comments (along with their reply counts, if possible) and then fill in the replies in response to a user action.

We'll still need to gather the total number of comments and display it on the comment button we use around the app.

Mocks a comin.

This would also fix #967 and lay the groundwork for #792 and maybe #974

[blowery]

The API is question is https://developer.wordpress.com/docs/api/1.1/get/sites/%24site/posts/%24post_ID/replies/

We're using it with the hierarchical field set true, which is the problem. We need to pull from the API in reverse chrono order, building up the comment tree as we process each item. Since replies have to happen after the initial comment ( crosses fingers ), we'll process all replies to a comment before the comment itself. So we shouldn't miss anything, but we'll have to walk through blocks of comments to get enough top level comments to show anything. In practice I don't think it'll be too bad, especially if we pull 50 – or min( 50, count from the post object) – at a go.

Then to fetch new comments, we just ask for comments after the most recent comment we have.

[shaunandrews]

Here's the mockup:

We can load a sane number of comments up front (I did 3 here for example), and let the user click/tap to load the earlier ones—perhaps paged 10 at a time—with a simple "x of x" page count.

Replies are hidden by default, but show a count. It'd be cool to show commenters names/gravs too, but maybe best saved for another day.

[samuelclemens]

The comments count "x of x" is really helpful IMO, it gives the sense of where you are at the comments.

But... There's some problem.

In your mock, you draw "55" as the total number of comments ( i concluded that because of the action bar comments count on top ), but the API does not return this in a straightforward way.

Unless we consider pingbacks/trackbacks as comments - the API certainly does.

Currently reader comments filter them out, since CommentStore action requests only type: comments but the comment count displayed everywhere does include them. (It actually comes from post.discussion.comment_count)

So lets say we moved past that, and we do display pingbacks/trackbacks in comments list (which imo we should).

In your mockup it says "3 of 55", while actually it displays 3 top-level comments and 3 (or more, since a reply can have more replies) replies, so it should be "6 (or a larger number) of 55" since, 55 is the total comment count, including replies, pingbacks, and trackbacks.

You might say we should display only the top-level comments count as the total number, but unless we fetch all comments and build the whole tree there is no way I can see to tell the number of root comments.

To sum up:

1. Let's display pingback/trackbacks in the comments list.
2. Lets display the count as "commentsOnScreen of totalCommentsCount", where:
   commentsOnScreen - sum of top-level comments and their reply trees
   totalCommentsCount - "found" field returned by the API, sum of comments,pingbacks and trackbacks.

What do you think?

[blowery]

I'm down with displaying ping/trackbacks if that's the only way we get a reasonable comment count.

[shaunandrews]

Showing ping/trackbacks is cool with me. Maybe they should be styled differently, as they'll likely attribute a site, rather than an author. Daily Post uses screenshots (mshots), which could be interesting at well: p23sd-11u5-p2

[samuelclemens]

What about the count bieng sum of top-level comments and their reply trees? Are you ok with that?

To clarify it a bit, what i mean:

Notice it's 7 of 98, not 3.

Because the way it's really is:

It's required imo in order to make sense with all the counts, especially the 98 of 98:

[blowery]

Ah, I see. I think just ditch the "$num of" part and just show the total.

Maybe break out the View Earlier link into a separate thing that makes it more obvious that there are more comments to show?

[blowery]

I wonder if we should threshold the hiding of replies too. Maybe just show 'em if there are less than 3 (per level)

[samuelclemens]

I kind of think that "$num of X" gives some more sense where you are at the comments... Why you don't like it?

[blowery]

It's too complicated to determine and explain. I think the last example, where we have two top level comments and 96 replies to the second one is really odd.

[blowery]

I think the only thing we really need to communicate is "there are more comments above this top level comment"

[samuelclemens]

Another demo of what i meant:

[alisterscott]

I noticed today that the comment count on a post in the stream includes pingbacks, but when viewing the post the pingbacks aren't shown, so for example this post shows 3 comments in the stream, but only 2 on the view: https://wordpress.com/read/feeds/24763464/posts/976082332

[stale]

This issue has been marked as stale because it hasn't been updated in a while. It will be closed in a week. If you would like it to remain open, can you please comment below and see what you can do to get things moving with this issue? Thanks! 🙏