Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

Improve discussion label #218

Closed
dimadin opened this issue Oct 20, 2018 · 2 comments
Closed

Improve discussion label #218

dimadin opened this issue Oct 20, 2018 · 2 comments
Labels
bug Something isn't working

Comments

@dimadin
Copy link
Contributor

dimadin commented Oct 20, 2018

In /template-parts/post/discussion-meta.php there is a label below list of avatars. Its code is:

/* Get data from current discussion on post. */
$discussion = twentynineteen_get_discussion_data();

$comments_number = get_comments_number();
$has_responses   = $discussion->responses > 0;

if ( $has_responses ) {
	/* translators: %1(X responses)$s from %2(X others)$s */
	$meta_label = sprintf(
		'%1$s from %2$s.',
		sprintf( _n( '%d response', '%d responses', $discussion->responses, 'twentynineteen' ), $discussion->responses ),
		sprintf( _n( '%d other', '%d others', $discussion->commenters, 'twentynineteen' ), $discussion->commenters )
		);
} elseif ( $comments_number > 0 ) {
	/* Show comment count if not enough discussion information */

	$meta_label = sprintf( _n( '%d Comment', '%d Comments', $comments_number, 'twentynineteen' ), $comments_number );
} else {
	$meta_label = __( 'No comments', 'twentynineteen' );
}

There are two problems that I see. First, it is confusing since it doesn't seems clear when is what used. Second, there is issue with i18n in the first case. It's not just that '%1$s from %2$s.' isn't marked for translation, it's also that all potential strings here need context and translator comment where are they used so that translated string looks good.

I would make PR for second case, but I'm not sure about "other" word. For example, when I left comment on the post (that I'm author of) with one more comment from another user, label was "1 response from 1 other". Should it be that way? Should we say just "1 other"?

@allancole
Copy link
Collaborator

I’m starting to think this may be a bit too complex for a Default theme. The wording has been confusing for a few others, and to fix it properly, we’d need to add more conditionals to account for more logged-in vs logged-out states, and change the label even more.

I think I’d be okay if the label simply showed a comment count. See here: #213 (comment)

@allancole allancole added the bug Something isn't working label Oct 21, 2018
@kjellr
Copy link
Collaborator

kjellr commented Nov 13, 2018

The %1$s from %2$s others label referenced here was removed in #525. The theme currently only shows the comment count:

screen shot 2018-11-13 at 4 15 02 pm

I'm going to close this issue. 👍 Please let me know if we should re-open for any reason.

@kjellr kjellr closed this as completed Nov 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants