Comments: add single comment view #19635

rodrigoi · 2017-11-09T15:16:49Z

Very much in progress, but hidden under a feature flag 😄

Depends on:

D8153-code
Comments Redesign: Always expanded view #19560: Comments Redesign: Always expanded view

This PR adds to new view for single comments, with the capability of performing moderation actions via query string parameters.

What to expect

This PR works only with the M3 redesign feature flag and depends heavily on components that still have work in progress, so warnings and errors are expected. Also, there's a regression on the comment list that shows all panels open, until #19560 gets resolved.
Also, there's duplication of code until dependent PRs get merged.

What to test:

The main reason this view exists is to perform moderation actions from emails. That's the area that requires testing at the moment. Most design elements depends on other M3 redesign PRs and will adjust once they are merged.

How to test:

Start Calypso with ENABLE_FEATURES=comments/management/m3-design npm start
Navigate to a comment by clicking on the timestamp link.
With WP.com Sandbox:
- Sandbox a WordPress.com site.
- Verify that comments moderation is required on Settings > Discussion > Comment must be manually approved for the sandboxed site
- Apply D8153-code on your sandbox
- Create a new comment with non editor user
- You should receive an email notification with and Approve, Spam and Trash
- Clicking on any of those links should perform the corresponding action
Without a WP.com Sandbox
- Navigate to a comment
- manually add to the url the following parameters as query string:
  - action: either approve, spam, trash or delete
  - for example/comment/rodrigoidummyfreesite.wordpress.com/165?action=approve

matticbot · 2017-11-09T15:16:56Z

Test live: https://calypso.live/?branch=update/comments/add-single-comment-view

a8ci18n · 2017-11-09T15:24:50Z

client/my-sites/comment/comment-permalink.jsx

+
+const CommentPermailnk = ( { permaLink, translate } ) => (
+	<Card className="comment__comment-permalink">
+		<SectionHeader label={ translate( 'Comment Permalink' ) } />


Hi! I've found a possible matching string that has already been translated 43 times:
translate( 'Permalink to this comment' ) ES Score: 8
See 1 additional suggestion in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

a8ci18n · 2017-11-09T15:34:30Z

client/my-sites/comment/comment-permalink.jsx

+const CommentPermalink = ( { isLoading, permaLink, translate } ) =>
+	! isLoading && (
+		<Card className="comment__comment-permalink">
+			<SectionHeader label={ translate( 'Comment Permalink' ) } />


Hi! I've found a possible matching string that has already been translated 43 times:
translate( 'Permalink to this comment' ) ES Score: 8
See 1 additional suggestion in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

gwwar · 2017-11-09T19:27:31Z

I know technically everything is a post, but in ?action=approve&site_id=99999999999&post_id=4 is the post_id a meant to be a post or a comment?

gwwar · 2017-11-09T19:39:34Z

client/my-sites/comments/controller.js

+	 *
+	 * we may not have user capabilities loaded onto state.
+	 * We trust that the API will handle user capabilities for us.
+	 */


Still much more secure than the inverse ;)

gwwar · 2017-11-09T19:40:51Z

client/my-sites/comment/main.jsx

+	const { commentId, siteFragment } = ownProps;
+
+	const siteId = getSiteId( state, siteFragment );
+	// const isJetpack = isJetpackSite( state, siteId );


Do we need Jetpack checks? We should have the related endpoint work ready.

gwwar · 2017-11-09T19:42:57Z

@drw158 did we need anything here in the header to help differentiate between the post specific view and one for a single comment?

gwwar

Thanks @rodrigoi! Let's tidy some of the commented out sections and get this in. This feels like a good chunk of work already.

If we feel like this might not ship with the new changes we can add another feature flag but I don't think it's necessary right now.

Things we can address in follow up PRs:

timing and how much information the route needs. eg can we simplify this to only needing the action param?
fix the clean cache flash of not having correct permissions
no feedback when replying to a comment
a redirect when deleting a comment permanently from this view
hitting the endpoint with the extra params I think does perform the update, but we're not seeing a re-render
add an edit action once that's available in the new component.

davewhitley · 2017-11-09T21:27:23Z

Because this is mainly for email, I think this design will be fine for now.

gwwar · 2017-11-13T19:35:54Z

@rodrigoi can we try landing this and follow up with the other items in the next PRs?

…nt view

…or permailnk... it's permalink!

…ity warning when state is empty

Copons · 2017-11-14T15:58:40Z

Overall this works well, but there are a bunch of follow up PRs that we should take care soonish, other than the ones already pointed out by @gwwar:

The page header reuses CommentListHeader which was built with the Post View in mind.
So, right now, the Comment View has an header that contains the post title, date, and a "View Post" link. It doesn't make sense.
cc @drw158 we oughta change this, likely with a new lean and clean header component in /my-sites/comment.
Somehow the possibility to externally change a comment status made an oldie but goldie issue emerge.
Try to mark a comment as spam via mail link or direct URL (e.g. comments/123/example.org/456?action=spam): we land on a comment; this comment has a white background, and no labels.
In other words: we don't have any clues at all to know that our comment is spam (but the same happens for trash) and, by extension, that our action was completed successfully.
(cc @drw158)
PS to the previous point: do we need to display a notice to confirm the action?
PPS: damn, the "undoable" Comment actions don't work in this view because they depends on the CommentList.updateLastUndo function.
We should check for updateLastUndo === undefined before firing it in CommentActions, since we wouldn't need it in Comment View.
The comment permalink should word-wrap: break-word or it'll likely overflow on small screens.

With the exception of the last point, I think we can easily fix and/or discuss in follow up PRs all the other points. 👍

* Comments: sidebar menu item is selected on both list and single comment view * Comments: updates link on the author date to the single comment view * Comments: adds moderation data component

rodrigoi added [Feature] Comments Comments on posts and the admin screen for managing them. [Status] In Progress labels Nov 9, 2017

rodrigoi self-assigned this Nov 9, 2017

rodrigoi requested a review from a team November 9, 2017 15:16

a8ci18n reviewed Nov 9, 2017

View reviewed changes

gwwar added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 9, 2017

gwwar reviewed Nov 9, 2017

View reviewed changes

gwwar approved these changes Nov 9, 2017

View reviewed changes

Copons mentioned this pull request Nov 10, 2017

Comments management: email notifications still point to wp-admin #17949

Closed

rodrigoi mentioned this pull request Nov 10, 2017

Comments: Moderation Emails Overrides Automattic/jetpack#8144

Merged

rodrigoi force-pushed the update/comments/add-single-comment-view branch from 09250cf to 3fa23c0 Compare November 10, 2017 16:41

Rodrigo Iloro added 10 commits November 14, 2017 10:23

Comments: sidebar menu item is selected on both list and single comme…

aa81b8a

…nt view

Comments: updates link on the author date to the single comment view

9d15bfe

Comments: adds experimental handler for query actions

d3ec567

Comments: adds handling of delete action on the controller

4537da8

Comments: adds main and permalink component for comment view

44198eb

Comments: adds styles for comment view components

8c052e4

Comments: adds external actions handling on the comtroller

a18e01f

Comments: prevent flashing of security warning on comment view

82cc729

Comments: adds comment link to comment header timestamp

6e8ba1b

Comments: hide permalink while comment is loading

79308e4

Rodrigo Iloro added 3 commits November 14, 2017 10:26

Comments: fixes typo on the permalink component. It's not permaink, n…

975481a

…or permailnk... it's permalink!

Comments: removes unnecessary Jetpack checks and fixes flash of secur…

bbb15a4

…ity warning when state is empty

Comments: adds moderation data component

5d0b7d4

rodrigoi force-pushed the update/comments/add-single-comment-view branch from 3fa23c0 to 5d0b7d4 Compare November 14, 2017 13:27

rodrigoi removed the [Status] In Progress label Nov 14, 2017

rodrigoi changed the title ~~Comments: add single comment view [In progress]~~ Comments: add single comment view Nov 14, 2017

Rodrigo Iloro added 2 commits November 14, 2017 12:40

Comments: removes external link from comment author for comment view

ace7111

Comments: removes link to comment view from M1 design

3806a68

apeatling added the Marked for Review Day label Nov 14, 2017

Rodrigo Iloro added 2 commits November 14, 2017 15:12

Comments: adds word wrap to permalink for small screens

6ccf10f

Comment: use external link when comment view feature flag is not enabled

8c4b546

rodrigoi merged commit a1c399a into master Nov 14, 2017

rodrigoi deleted the update/comments/add-single-comment-view branch November 14, 2017 18:43

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 14, 2017

gwwar removed the Marked for Review Day label Nov 14, 2017

This was referenced Nov 14, 2017

Comments: comment specific view UI #19773

Closed

Comments: Moderation Hotlinks #19774

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments: add single comment view #19635

Comments: add single comment view #19635

rodrigoi commented Nov 9, 2017 •

edited

Loading

matticbot commented Nov 9, 2017

a8ci18n Nov 9, 2017

a8ci18n Nov 9, 2017

gwwar commented Nov 9, 2017

gwwar Nov 9, 2017

gwwar Nov 9, 2017

gwwar commented Nov 9, 2017

gwwar left a comment •

edited

Loading

davewhitley commented Nov 9, 2017

gwwar commented Nov 13, 2017

Copons commented Nov 14, 2017

Comments: add single comment view #19635

Comments: add single comment view #19635

Conversation

rodrigoi commented Nov 9, 2017 • edited Loading

matticbot commented Nov 9, 2017

a8ci18n Nov 9, 2017

Choose a reason for hiding this comment

a8ci18n Nov 9, 2017

Choose a reason for hiding this comment

gwwar commented Nov 9, 2017

gwwar Nov 9, 2017

Choose a reason for hiding this comment

gwwar Nov 9, 2017

Choose a reason for hiding this comment

gwwar commented Nov 9, 2017

gwwar left a comment • edited Loading

Choose a reason for hiding this comment

davewhitley commented Nov 9, 2017

gwwar commented Nov 13, 2017

Copons commented Nov 14, 2017

rodrigoi commented Nov 9, 2017 •

edited

Loading

gwwar left a comment •

edited

Loading