Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#936 - comment confirmation message #1020

Closed
wants to merge 4 commits into from
Closed

Conversation

DavidCramer
Copy link
Contributor

@DavidCramer DavidCramer commented Mar 15, 2018

This adds a setting in the discussion page to allow for the changing of the success message when posting a comment.

Fixes #936

@DavidCramer DavidCramer changed the title [WIP] #936 - comment confirmation message #936 - comment confirmation message Mar 15, 2018
@DavidCramer
Copy link
Contributor Author

initially we were going to use an alternate success message if the user has had an accepted comment before, however this is more complex to do since you need to check etc.
Also, when you post a comment that is to be moderated first, the comment still shows but has a note stating the comment is awaiting moderation.
So the function of this is more to indicate to the user that the comment will show shortly.

screen shot 2018-03-15 at 12 06 39

@DavidCramer DavidCramer changed the base branch from develop to 0.7 March 15, 2018 10:09
@DavidCramer
Copy link
Contributor Author

@westonruter this is ready for review now. I'm unsure of the approach. I think it's ok, but a second opinion will be good.

@westonruter
Copy link
Member

westonruter commented Mar 17, 2018

@DavidCramer I think there may be a better way. We actually can determine if the comment was approved via in the \AMP_Theme_Support::handle_xhr_request() call. We can pass back the approval status by changing it to look like this:

add_filter( 'comment_post_redirect', function( $location, $comment ) {
	wp_send_json( array(
		'approved' => '1' === (string) $comment->comment_approved,
	) );
}, PHP_INT_MAX, 2 );

Then inside of AMP_Theme_Support::add_amp_comment_form_templates it can do a conditional message based on this data returned in the XHR handler, for example:

<div submit-success>
	<template type="amp-mustache">
		{{#approved}}
			<?php esc_html_e( 'Your comment has been posted.', 'amp' ); ?>
		{{/approved}}
		{{^approved}}
			<?php esc_html_e( 'Your comment is awaiting moderation.', 'default' ); ?>
		{{/approved}}
	</template>
</div>

Note how it's re-using the default moderation string in core. I couldn't find a default message regarding a comment being accepted, however.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to open a new PR to implement the alternative approach.

@westonruter
Copy link
Member

@DavidCramer on second thought, it may be better to return the message in the same way is being done for the die handler:

add_filter( 'comment_post_redirect', function( $location, $comment ) {
	if ( '1' === (string) $comment->comment_approved ) {
		$message = __( 'Your comment has been approved.', 'amp' );
	} else {
		$message = __( 'Your comment is awaiting moderation.', 'default' );
	}
	$message = apply_filters( 'amp_comment_submitted_message', $message, $comment );
	wp_send_json( array(
		'message' => $message,
		'approved' => $comment->comment_approved,
	) );
}, PHP_INT_MAX, 2 );

Then inside of AMP_Theme_Support::add_amp_comment_form_templates() it can just do:

<div submit-success>
	<template type="amp-mustache">
		{{{message}}}
	</template>
</div>

This will allow for the message to be filtered and for plugins to add support for custom comment statuses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants