Missing plural on activities for show all comments button

[danbp]

When modifying the default 5 comments
/* Hide long lists of activity comments, only show the latest five root comments. */ in buddypress.js:1898

 if ( jq(this).children('ul').children('li').length < 0 )

and line 1913

if ( i < comment_lis.length - 0 )

to get show comments(x) button as soon as there is 1 comment, the button text brings Show all 1 comments.

the gettext plural is needed in buddypress-functions.php:252

'show_x_comments' => __( 'Show all %d comments', 'buddypress' )

something like

'show_x_comments' => _n( 'Show %d comment', 'Show all %d comments', 'buddypress' )

Thank you. ;-)

[boonebgorges]

Good catch. Unfortunately, using _n() here is not possible, because we need the singular as well as the plural available when the page renders. In fact, I'm not sure we have a good way of doing this at all at the moment, given that some languages require more than one version of plurals. See ​https://core.trac.wordpress.org/ticket/22229. I guess, as a bandaid, we could fix this for languages like English and French that have only singular + plural?

[johnjamesjacoby]

Replying to boonebgorges:

as a bandaid, we could fix this for languages like English and French that have only singular + plural

I'm fine with this for now; it's better than it being blatantly grammatically incorrect.

[boonebgorges]

My point is that we'll remain blatantly grammatically correct in Russian, Japanese, and other widely used languages. But correct in some is, in the absence of a better solution, better than correct in none.

[johnjamesjacoby]

I understand your point, and my point is the same as your point, so we are in agreement.

[slaFFik]

In my opinion the easiest solution will be instead of this:

'show_all'          => __( 'Show all', 'buddypress' ),
'show_all_comments' => __( 'Show all comments for this thread', 'buddypress' ),
'show_x_comments'   => __( 'Show all %d comments', 'buddypress' ),

to have only 1 record:

'show_all' => __( 'Show all', 'buddypress' ),

As show_all includes all other possible special cases show_all_comments and show_x_comments.

What do you think? Such fix is general and quite easy, and will eliminate this problem.

[slaFFik]

Other possible solution:

'show_all' => __( 'Show all (%d)', 'buddypress' ),

Which will always display the actual number, if we want end-user give ability to decide whether to click or not based on the number of items.

[boonebgorges]

Replying to slaFFik:

Other possible solution:

'show_all' => __( 'Show all (%d)', 'buddypress' ),

Which will always display the actual number, if we want end-user give ability to decide whether to click or not based on the number of items.

I like this.

[slaFFik]

So, after some investigation here is what I've found:

'show_all' => __( 'Show all', 'buddypress' ), - not used at all, be it seems we need to leave it for BC (not sure).

'show_all_comments' => __( 'Show all comments for this thread', 'buddypress' ), - it's a title attribute, so it's good to leave it for accessibility (right, @mercime?)

'show_x_comments' => __( 'Show all %d comments', 'buddypress' ), - here is the source of the problem, when editing buddypress.js to show this show-more link, when there is 1 comment.
The simplest solution: 'show_x_comments' => __( 'Show all comments (%d)', 'buddypress' ),.

Attached images shows how it will look like, and patch - implement this change.

[boonebgorges]

This looks good.

[slaffik]

In 11369:

Internationalization: change show all comments button label.

Make 'Show all %d comments' a bit less dependent on the amount of comments, changed to 'Show all comments (%d)'. Thus 1 and 100 comments number will be displayed properly.

Props danbp.
Fixes #6275.