Wp-calypso: Comments: Threaded Post View

Created on 27 Oct 2017  路  18Comments  路  Source: Automattic/wp-calypso

Let's implement a threaded view, max 2 levels deep for the 'All' and 'Pending' and 'Approved' tabs. 'Spam' and 'Trash' views may remain flat.

Note that we have some in progress work available from @rodrigoi and @Copons under the comments/management/threaded-view feature flag.

Implementation Tasks:

  • [ ] Refine the post specific comment tree endpoint that was added in (https://github.com/Automattic/jetpack/pull/8239)
    -- Add support for fetching approved/pending in one request. We may need to modify the data structure for this case.
    -- In our worse case scenarios, can we fetch everything in a single request? Or do we need to page our results? #19898
  • [ ] Given a list of ids, add a bulk comment get endpoint, or consider if we should return some subset of expanded comments to optimize for this case.
  • [ ] Apply a threaded view for All, Pending and Approved in the post list view. #19733

Design: Did we need different styling between flat/threaded? Right now flat currently has more padding/margin between each comment than the mocks below.

@drw158: Single post view should have no margins as shown in the mockups. Otherwise, the comments should have top/bottom margins.

Threaded, with pending | Threaded, no pending:

screen shot 2017-11-15 at 2 12 01 pm

Single post view, no replies:

32632381-119f8714-c569-11e7-84cc-773a8b929d61-1


Original Prompt:

All of our comment displays are "flat" (as are all of them in wp-admin). It _is_ nice, however, to see a threaded overview on the front-end of sites (most themes will display comments in a threaded view up to 10 levels deep). We've received some feedback about missing threaded views; do we think it's of value to add to the upcoming post view, maybe something like this?

screen shot 2017-10-27 at 10 30 54 am

Interesting related reading: https://blog.codinghorror.com/web-discussions-flat-by-design/

cc/ @megsfulton

Comments Design [Type] Enhancement

Most helpful comment

Quick mockups for threading:

  • limit is 2 (or 3?), just like the Reader.
  • add the > username indicators to see who replied to who
  • remove post titles and 'in reply to' UI

screen shot 2017-11-09 at 4 10 59 pm

If there is no threading, there isn't much visual difference for this view. I was hoping for otherwise.

no threads approved

_Note: mockups are outdated. We will only do 2 levels of threading. The mockups show 3 levels._

All 18 comments

Oh, we _do_ have threaded display of comments in the Reader single post view.

screen shot 2017-10-27 at 10 41 47 am

Let's make this an investigation task:

Technical:

  1. Can we render a threaded view with pending/spam/trashed comments using our existing endpoints? If we need endpoint work we'll likely miss the Oct cutoff, which is fine, but earliest this could go out would then be for the Nov Jetpack release and we'll communicate that any implementation would be for m4.

Design:

  1. Imo I think if we go with a threaded view, we don't need a toggle to go back to flat. But let's keep that open to discussion.
  2. Does it make sense to filter by type in this view? I think this might boil down to a threaded All, Approved lists, and a flat list for Spam/Trash. Otherwise the last two views need to be mixed as well.
  3. Does bulk edit make sense in this view?
  4. How will we load in more comments? Paging may get much more complex in a threaded view.
  5. Do we allow folks to collapse threads?

Alternatively how difficult would it be to make a core change, that adds comment management to the site post you're looking at? Eg if I'm the author and I see the comments on my post, I'm also able to see pending comments and moderate there directly.

@Automattic/reader how crazy would it be if we allowed comment management options for pending comments in the Reader?

Also related our incomplete never-released post list comment view:
screen shot 2017-10-27 at 4 57 03 pm

@Automattic/reader how crazy would it be if we allowed comment management options for pending comments in the Reader?

Just for a site admin right? Sounds good to me

I'm guessing just for a site admin right? Sounds good to me

@samouri Role wise it can go down as far as an Author to their own posts.

Imo I think if we go with a threaded view, we don't need a toggle to go back to flat.

I agree. I don't see any benefit from being able to toggle between threaded/flat.

Still catching up on history, but this seems like an ok thing to do for the _single post view_ only (as shown in the original image up top).

Seems like we should do something simple at first: just indent the threaded comments, and max the nesting at 2 levels, just like the Reader. We would need to tweak the "replied to" text a bit though. Seems like we should solve the always expanded issue first. #19306

Heres the PR where we implemented the "replied to" text and 2 levels of nesting in case you wanted to try and make it consistent with Reader: https://github.com/Automattic/wp-calypso/pull/16770

example:
28796393-5ccc6da4-760b-11e7-8aca-df4d433aabbd

Quick mockups for threading:

  • limit is 2 (or 3?), just like the Reader.
  • add the > username indicators to see who replied to who
  • remove post titles and 'in reply to' UI

screen shot 2017-11-09 at 4 10 59 pm

If there is no threading, there isn't much visual difference for this view. I was hoping for otherwise.

no threads approved

_Note: mockups are outdated. We will only do 2 levels of threading. The mockups show 3 levels._

@drw158 I think it's showing 3-levels instead of just two?

@jancavan I believe in the Reader it's showing 3 currently, but apparently it's a bug and it should be 2. Whatever we do, will be consistent with the Reader.

Hmm, on Conversations we're nesting them 2-levels deep, but 3-levels in fullpost.

Conversations:
screenshot 2017-11-09 14 23 26

Fullpost:
screenshot 2017-11-09 14 23 35

@samouri @bluefuton Is this a regression?

I believe in the Reader it's showing 3 currently, but apparently it's a bug and it should be 2.

I just saw reply after I'd posted mine. Yeah, I think it's a regression.

Whatever we do, will be consistent with the Reader.

Sounds good 馃憤

I believe in the Reader it's showing 3 currently, but apparently it's a bug and it should be 2.

Fixed in: https://github.com/Automattic/wp-calypso/pull/19656

@Copons this is what I had in mind for the threaded view when we talked about this on Slack (p1510934862000141-slack-lannister) to avoid the weird recursion the _Reader_ components use.

https://github.com/Automattic/wp-calypso/compare/try/comments/comment-threaded-view

screen shot 2017-11-20 at 11 08 00

It's very rough, but the idea is to have a specialized version of CommentList that's connected to the store to get the replies of the commentId and how deep it is in the tree.
Most of the logic can even be moved to a getCommentDepth selector...

I've run into some performance issues with this approach, on a test site that has almost two thousand comments. The _Full_ and _Post_ views just choke when traversing the comment tree for the children, mostly due to the number of re-renders of components on those pages.
A couple things we can do, besides taking a closer look at the render tree and improving render performance:

  • Get the actual tree from the backend, so we don't need to traverse it on the client. Maybe adding a children: [] to the response.
  • use a _View Replies_ link, to lazy load the response tree of a comment.
Was this page helpful?
0 / 5 - 0 ratings