Reader: move post comments to Redux #4817

bluefuton · 2016-04-18T12:10:28Z

This PR moves post comments in the Reader to use Redux state instead of Flux stores.

It builds on the work done by @samuelclemens in #3528.

bluefuton · 2016-04-18T13:42:54Z

To do:

when posting a new comment, the relative post date is a bit eager. If the comment requires moderation, the time resets from "1s ago" to "0s ago". We should probably say "just now" if it's been less than a minute. Video: https://cloudup.com/cl7dnN0kSJz
increase the number of replies needed to trigger the 'show x comments' link. In Reader: hide replies initially for threads with lots of comments #967, we originally suggested 15. Perhaps 5?
Required props missing:
@blowery noticed: "clicking on the Comments icon in a post card doesn't scroll you down to the comments any more"

bluefuton · 2016-04-18T13:58:50Z

Hi @jancavan - would you mind checking this PR please and letting me know if you have any further feedback beyond the todo list above? As far as I can see, everything discussed on the original PR (#3528) has been addressed.

blowery · 2016-04-18T14:36:00Z

I think we should just show replies when there's less than 5. Having to click a thing to get in when there's only a couple replies is really a bummer.

post that bummed me out: http://calypso.localhost:3000/read/blogs/24928942/posts/5639#comments

blowery · 2016-04-18T14:36:49Z

Clicking on the Comments icon in a post card doesn't scroll you down to the comments any more

lancewillett · 2016-04-18T22:34:55Z

@mtias Meta-question: can State label be a milestone instead? Why can't we use Framework instead?

jancavan · 2016-04-18T22:52:42Z

I think we should just show replies when there's less than 5

+1

Can "Cancel reply" be moved to the right instead of in between Reply and Like?

The root comments are ordered chronologically (oldest -> newest), but the nested replies are in an odd order:
http://calypso.localhost:3000/read/feeds/23059853/posts/922262993#comments

bluefuton · 2016-04-19T14:35:21Z

More todo from @jancavan:

Fix ordering of nested replies
Move 'cancel reply' to the right

bluefuton · 2016-04-19T17:31:08Z

Clicking on the Comments icon in a post card doesn't scroll you down to the comments any more

This functionality was originally added in #2058, so worth checking there to see what the regression might be.

Edit: fixed in c0b8e3e.

bluefuton · 2016-04-20T09:03:50Z

Nested replies are now shown in chronological order, which matches the behaviour of the existing Reader comments.

bluefuton · 2016-04-20T09:12:44Z

Comment reply input doesn’t return to top level after successful comment posting

bluefuton · 2016-04-20T10:21:24Z

The downside of ordering the nested replies chronologically is that, when you post a new comment on a thread, it appears at the bottom. This might mean it's out of view - see https://cloudup.com/ccyrBzfU4y8

We fudge this on the existing Flux comments by always putting the new comment at the top of the replies. This often isn't its final position - it moves downwards after a reload if there's more than one reply.

Three possible solutions here:

sort nested replies in reverse chronological order (same as top-level comments) - easiest solution
fudge comment order so that user's reply always appears at top
scroll to and highlight newly posted comment

Thoughts @jancavan @blowery?

jancavan · 2016-04-21T02:49:21Z

sort nested replies in reverse chronological order (same as top-level comments) - easiest solution

What we're doing now - where we slide your new comment in topmost (and it just shows up at the bottom after a refresh) is actually good. Only reason I brought it up thought was I noticed that the nested replies were neither in chronological/reverse chronological order. It somehow was arbitrary, but I'm unable to reproduce the issue :/

bluefuton · 2016-04-21T09:44:31Z

Only reason I brought it up thought was I noticed that the nested replies were neither in chronological/reverse chronological order.

Could it have been that the replies weren't all at the same depth, but they were flattened out by the indentation limit (so they all appeared at the same indent)? That caught me out a couple of times yesterday :)

bluefuton · 2016-04-21T12:02:55Z

What we're doing now - where we slide your new comment in topmost (and it just shows up at the bottom after a refresh) is actually good

Great - happy to stick with that. I've made the change in a7bc448.

bluefuton · 2016-04-21T14:28:24Z

Ready for 👀 @blowery @jancavan

jancavan · 2016-04-21T15:38:26Z

@bluefuton the nested replies beginning 3rd level become really confusing with the blue bar. Let me play around with the styling.

blowery · 2016-04-21T15:41:50Z

@jancavan maybe we collapse the commenting levels based on viewport width? Desktop browsers with lots of room see the full nesting and folks on narrower viewports see less and less?

jancavan · 2016-04-21T18:39:52Z

Desktop browsers with lots of room see the full nesting

I think that's a good idea to allow more nesting on desktop, but we should still set a limit. Infinite nesting doesn't really work anymore after several levels deep.

I've made changes here (d21884d) which helps, but could be made better if we changed the show/hide icon: https://cloudup.com/cA4DzdYEWCV

This isn't great, however, since after 3rd-level indentation, the hierarchy is still off: http://calypso.dev:3000/read/feeds/23059853/posts/939824593#comments

Another solution (but is out of scope): Just allow comments to be expandable, but not collapsible. This also allows for proper hierarchy.

Sidenote: I believe this is old code, but I poked at this branch and the gravatar is positioned absolute which shouldn't really be the case as it makes the css hard to maintain. Changing the positioning values would have to be done in many different places.

blowery · 2016-04-21T19:49:24Z

rebased this and force pushed it

- Removed CommentStore and CommentLikeStore - Should validate refs before accessing them - Validate post prop - Refactored form component to handle auto growth a bit better - Fix the click on reply issue when another reply box is active - Fixed style - Simplified the replay click fix - Remove all remaining comment store and comment like store files, and turn off associated tests - humanDate: show "just now" for times less than a minute ago - If a comment has 5 or fewer replies, show all replies (no hide/show controls) - Remove isRequired on props that may be null before provided by a Redux selector - Full post: check if we need to scroll to comments when receiving new shouldShowComments prop - Move 'cancel reply' link to right hand side - Order nested replies in chronological order - Return comment form to bottom after reply posted - When adding a new comment from the current user, skip the date sort so that it appears at the top of the current branch - Remove handleBlur in comment form. Component was unmounting before delay() completed, resulting in setState warnings

blowery · 2016-05-04T14:55:47Z

Force pushed again to pick up latest.

Looks really good. Really really good.

blowery · 2016-05-05T18:45:18Z

This moved to #5236

bluefuton self-assigned this Apr 18, 2016

bluefuton added [Status] In Progress [Feature] Reader The reader site on Calypso. labels Apr 18, 2016

bluefuton mentioned this pull request Apr 18, 2016

Reader: Convert post comments to use redux state #3528

Closed

5 tasks

bluefuton added this to the Offline i1: State tree consolidation milestone Apr 18, 2016

mtias added the State label Apr 18, 2016

bluefuton force-pushed the update/reader/move-comments-to-redux branch from ac673f7 to eb9374d Compare April 18, 2016 13:21

bluefuton added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Apr 18, 2016

bluefuton removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Apr 19, 2016

bluefuton force-pushed the update/reader/move-comments-to-redux branch from c789fb6 to e3e1d5f Compare April 19, 2016 13:51

bluefuton force-pushed the update/reader/move-comments-to-redux branch from a843c13 to a7bc448 Compare April 21, 2016 12:02

bluefuton force-pushed the update/reader/move-comments-to-redux branch 2 times, most recently from 2cfa682 to 6a4828a Compare April 21, 2016 14:26

bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels Apr 21, 2016

bluefuton removed the [Status] In Progress label Apr 21, 2016

blowery mentioned this pull request Apr 21, 2016

Posts: Inline comments viewer doesn't show proper nesting #4928

Closed

blowery force-pushed the update/reader/move-comments-to-redux branch from d21884d to 7bade48 Compare April 21, 2016 19:49

mtias mentioned this pull request Apr 28, 2016

Framework: Redux single data tree and query components #5046

Closed

bluefuton force-pushed the update/reader/move-comments-to-redux branch from 7bade48 to 096ff14 Compare May 3, 2016 09:25

samuelclemens and others added 2 commits May 4, 2016 10:43

Remove blue bar indentation from second-leve replies

b84af5a

blowery force-pushed the update/reader/move-comments-to-redux branch from 096ff14 to b84af5a Compare May 4, 2016 14:55

blowery merged commit 0fcd958 into master May 5, 2016

blowery deleted the update/reader/move-comments-to-redux branch May 5, 2016 12:39

scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 5, 2016

blowery removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label May 5, 2016

blowery mentioned this pull request May 5, 2016

Revert "Reader: move post comments to Redux" #5235

Merged

jancavan mentioned this pull request May 6, 2016

Reader: move post comments to Redux, try 2 #5236

Merged

This was referenced May 16, 2016

Reader Start: create UI components #5264

Merged

Reader: fix misaligned Follow buttons on list items #5502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reader: move post comments to Redux #4817

Reader: move post comments to Redux #4817

bluefuton commented Apr 18, 2016

bluefuton commented Apr 18, 2016 •

edited

Loading

bluefuton commented Apr 18, 2016 •

edited

Loading

blowery commented Apr 18, 2016 •

edited

Loading

blowery commented Apr 18, 2016

lancewillett commented Apr 18, 2016

jancavan commented Apr 18, 2016 •

edited

Loading

bluefuton commented Apr 19, 2016 •

edited

Loading

bluefuton commented Apr 19, 2016 •

edited

Loading

bluefuton commented Apr 20, 2016

bluefuton commented Apr 20, 2016 •

edited

Loading

bluefuton commented Apr 20, 2016

jancavan commented Apr 21, 2016

bluefuton commented Apr 21, 2016

bluefuton commented Apr 21, 2016

bluefuton commented Apr 21, 2016

jancavan commented Apr 21, 2016

blowery commented Apr 21, 2016

jancavan commented Apr 21, 2016

blowery commented Apr 21, 2016

blowery commented May 4, 2016 •

edited

Loading

blowery commented May 5, 2016

Reader: move post comments to Redux #4817

Reader: move post comments to Redux #4817

Conversation

bluefuton commented Apr 18, 2016

bluefuton commented Apr 18, 2016 • edited Loading

bluefuton commented Apr 18, 2016 • edited Loading

blowery commented Apr 18, 2016 • edited Loading

blowery commented Apr 18, 2016

lancewillett commented Apr 18, 2016

jancavan commented Apr 18, 2016 • edited Loading

bluefuton commented Apr 19, 2016 • edited Loading

bluefuton commented Apr 19, 2016 • edited Loading

bluefuton commented Apr 20, 2016

bluefuton commented Apr 20, 2016 • edited Loading

bluefuton commented Apr 20, 2016

jancavan commented Apr 21, 2016

bluefuton commented Apr 21, 2016

bluefuton commented Apr 21, 2016

bluefuton commented Apr 21, 2016

jancavan commented Apr 21, 2016

blowery commented Apr 21, 2016

jancavan commented Apr 21, 2016

blowery commented Apr 21, 2016

blowery commented May 4, 2016 • edited Loading

blowery commented May 5, 2016

bluefuton commented Apr 18, 2016 •

edited

Loading

bluefuton commented Apr 18, 2016 •

edited

Loading

blowery commented Apr 18, 2016 •

edited

Loading

jancavan commented Apr 18, 2016 •

edited

Loading

bluefuton commented Apr 19, 2016 •

edited

Loading

bluefuton commented Apr 19, 2016 •

edited

Loading

bluefuton commented Apr 20, 2016 •

edited

Loading

blowery commented May 4, 2016 •

edited

Loading