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

Make a11y string properly localizable #15161

Merged
merged 5 commits into from
Apr 25, 2019
Merged

Conversation

swissspidy
Copy link
Member

Description

Fixes the non-localizable code introduced by #15144.

This is a friendly reminder to think about internationalization best practices :-)

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@swissspidy swissspidy added the Internationalization (i18n) Issues or PRs related to internationalization efforts label Apr 24, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you @swissspidy for helping us with this improvement!

I wanted to ask if it's possible to use _x() instead of __() to be able to send a more accurate context to the translators, letting them know that this string is intended to be used be the Screen Reader engine.

Something like:
_x( 'Heading block. Level %s. Empty.', 'Spoken out loud by the screen reader' )

@swissspidy
Copy link
Member Author

In WordPress itself we usually use a /* translators: accessibility text */ comment for that. So perhaps just accessibility text as a context in this case.

@etoledom
Copy link
Contributor

just accessibility text as a context in this case

That sounds great! Thank you 🙏

@gziolo gziolo added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Apr 25, 2019
@pinarol pinarol self-requested a review April 25, 2019 10:44
@pinarol pinarol self-assigned this Apr 25, 2019
Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

I tested this on iOS and it works OK.

@pinarol
Copy link
Contributor

pinarol commented Apr 25, 2019

hey @Tug 👋 I updated the position of translator comments

Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pinarol pinarol merged commit da09d36 into master Apr 25, 2019
@pinarol pinarol deleted the fix/string-concatenation branch April 25, 2019 13:45
hypest pushed a commit that referenced this pull request May 2, 2019
* Make a11y string properly localizable

* Remove extra . after the content

* Add `accessibility text` to the translators comment

* Fix the wrong parameter

* Change the position of translator comments
@youknowriad youknowriad added this to the 5.6 (Gutenberg) milestone May 13, 2019
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 Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants