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

[I18n] Label "Move %s" - up/down/left/right not translatable #20655

Closed
pedro-mendonca opened this issue Mar 5, 2020 · 8 comments · Fixed by #20664
Closed

[I18n] Label "Move %s" - up/down/left/right not translatable #20655

pedro-mendonca opened this issue Mar 5, 2020 · 8 comments · Fixed by #20664
Assignees
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@pedro-mendonca
Copy link
Contributor

Describe the bug
The labels of the buttons to move blocks, the label is partially missing i18n.
The string Move %s is correct, but the strings that can be included in the placeholder %s are not.
The direction strings up, down, left and right arent't translatable.

To reproduce
Steps to reproduce the behavior:

  1. Set your WP install to other locale than en_US
  2. Open block editor
  3. Create multiple blocks
  4. Hover the move arrows on the left of the block (see screenshot below)
  5. See error: Move is translatable, the direction isn't.

Expected behavior
The placeholder %s should include a translation of up, down, left or right.
Also, these very short strings MUST have a context.

Screenshots
Screenshot 2020-03-05 13 29 00

Additional context

  • Tested in WP 5.4 RC1, no standalone Gutenberg installed
@pedro-mendonca pedro-mendonca changed the title [I18n] Label "Move %s" - up/down/left/right strings miss i18n [I18n] Label "Move %s" - up/down/left/right not translatable Mar 5, 2020
@aduth
Copy link
Member

aduth commented Mar 5, 2020

This appears to have regressed as part of #16615.

Notably: The labels were previously rendered in their full (and translated) forms, e.g. __( 'Move up' ), etc.

https://github.com/WordPress/gutenberg/pull/16615/files#diff-4d796bdfde89804a60bfac1e1bf3ff9aL64

As described in the original issue, they are now concatenated, but the directional labels are not translatable:

label={ sprintf(
__( 'Move %s' ),
getMovementDirection( 'up' )
) }

const getMovementDirection = ( moveDirection ) => {
if ( moveDirection === 'up' ) {
if ( orientation === 'horizontal' ) {
return isRTL ? 'right' : 'left';
}
return 'up';
} else if ( moveDirection === 'down' ) {
if ( orientation === 'horizontal' ) {
return isRTL ? 'left' : 'right';
}
return 'down';
}
return null;
};

@aduth aduth added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release Internationalization (i18n) Issues or PRs related to internationalization efforts labels Mar 5, 2020
@aduth
Copy link
Member

aduth commented Mar 5, 2020

I added this to the "WordPress 5.4 Must Have" project, since this is a regression of WordPress 5.4. However, it's not clear to me whether it can be fixed at this point, since the 5.4 RC has passed and is typically considered a string freeze.

@aduth
Copy link
Member

aduth commented Mar 5, 2020

@pedro-mendonca Do you have a sense whether these labels would be better off implemented in their fully-expanded forms, vs. injected by placeholder?

i.e.

  • __( 'Move up' )
  • __( 'Move right' )
  • __( 'Move down' )
  • __( 'Move left' )

@pedro-mendonca
Copy link
Contributor Author

The current placeholder form generates very small strings that probably need context.
Simple strings as "up", "down", "left" or "right" may vary a lot depending of its context.
It needs specific context so translators know it is to concatenate with "Move %s".

In the end, the current form uses 5 different strings that need very clear and specific contextualization.

The simple __( 'Move up' ) form will end with just 4 strings, no mess.

So... in my opinion... bring back old simple unconcatenated form :-)

@aduth
Copy link
Member

aduth commented Mar 5, 2020

I opened a pull request to reintroduce full labels: #20664

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Mar 5, 2020
@pedro-mendonca
Copy link
Contributor Author

Awesome, thanks! 👍

@ocean90
Copy link
Member

ocean90 commented Mar 5, 2020

However, it's not clear to me whether it can be fixed at this point

Strings are available since today for translators. As mentioned in the post, string changes may still occur.

This case is something we should definitely get fixed for 5.4.

@pedro-mendonca
Copy link
Contributor Author

I agree, it’s a bug, not a enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants