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

Added option to display comment author's full name in reply link. #237

Merged
merged 5 commits into from Jul 19, 2016

Conversation

@vskjefst
Copy link
Contributor

@vskjefst vskjefst commented Jul 16, 2016

Not sure how you usually handle new strings that needs to be translated?

@@ -354,11 +365,12 @@ function independent_publisher_author_comment_reply_link( $link, $args, $comment
}

// If the user provided more than a first name, use only first name
if ( strpos( $author, ' ' ) ) {
$independent_publisher_general_options = get_option( 'independent_publisher_general_options' );

This comment has been minimized.

@raamdev

raamdev Jul 16, 2016
Owner

This line can be removed; $independent_publisher_general_options is not used here.

$wp_customize->add_control(
'show_comment_authors_full_name_in_reply_link', array(
'settings' => 'independent_publisher_general_options[show_comment_authors_full_name_in_reply_link]',
'label' => __( 'Show comment author\'s full name in reply-link', 'independent-publisher' ),

This comment has been minimized.

@raamdev

raamdev Jul 16, 2016
Owner

I suggest changing the label here to "Show Full Name in Comment Reply-to":

2016-07-16_14-24-37

That will fit better in the Customizer than what you have now:

2016-07-16_14-25-04


// Show comment author's full name in reply-link
$wp_customize->add_setting(
'independent_publisher_general_options[show_comment_authors_full_name_in_reply_link]', array(

This comment has been minimized.

@raamdev

raamdev Jul 16, 2016
Owner

I suggest changing the option key to show_full_name_comment_reply_to (this will need to be updated anywhere this key is used).

@raamdev
Copy link
Owner

@raamdev raamdev commented Jul 16, 2016

@vskjefst Thanks so much for the Pull Request! I tested this out and it works great. :-) I left you a few comments above with a few change requests.

Not sure how you usually handle new strings that needs to be translated?

There's no need to worry about that—I'll regenerate the translation POT file right before I do the next release.

@vskjefst
Copy link
Contributor Author

@vskjefst vskjefst commented Jul 16, 2016

@raamdev The necessary changes are now pushed to the PR branch.

@@ -333,9 +333,20 @@ function independent_publisher_comment_count( $count ) {
}
}

if ( ! function_exists( 'independent_publisher_show_comment_authors_full_name_in_reply_link' ) ):

This comment has been minimized.

@raamdev

raamdev Jul 16, 2016
Owner

@vskjefst Could you also rename this function to independent_publisher_show_full_name_comment_reply_to?

@vskjefst
Copy link
Contributor Author

@vskjefst vskjefst commented Jul 16, 2016

@raamdev yup, done.

@raamdev raamdev merged commit ef37fab into raamdev:master Jul 19, 2016
@raamdev
Copy link
Owner

@raamdev raamdev commented Jul 19, 2016

Woohoo! Merged. Thank you! 😄

@raamdev
Copy link
Owner

@raamdev raamdev commented Jul 19, 2016

Posting a screenshot of this new option for future reference:

2016-07-16_14-24-37

@vskjefst vskjefst deleted the vskjefst:feature/full-name-in-reply-link branch Jul 19, 2016
@raamdev raamdev added this to the Next Release milestone Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants