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: various fixes #59

Merged
merged 4 commits into from
Mar 27, 2018
Merged

I18n: various fixes #59

merged 4 commits into from
Mar 27, 2018

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Mar 15, 2018

I18n: fix incorrect text-domain

The text-domain for WordPress itself is default, not wordpress.

I18n: add missing translators comments

Includes minor efficiency fix for sprintf(): when using numbered replacements, you can use the same replacement twice, so no need to have '</a>' twice in the replacement list.

I18n: Make the text slightly more plugin agnostic

AFAIK WHIP is intended to also be usable by other (non-Yoast) plugins/themes.
There were currently two phrases which made this awkward.

This PR changes those.

For the first phrase, I've chosen to change WordPress and Yoast SEO to WordPress and all it's plugins and themes. This incidentally fixes issue #35. /cc @afercia
If it is preferred that Yoast SEO not be removed, I would suggest the following:

  • Move the name out of the text string and put it back in using sprintf().
  • Make the name filterable to allow other plugins to pass in their plugin name.
    • Potentially, it could even be turned into an array which could be passed into the string using implode() to allow for a number of plugins to be mentioned.

For the second phrase, just changing the our in vetted by our Yoast support team to the improves the re-usability.

Fixes #35

The text-domain for WordPress itself is `default`, not `wordpress`.
Includes minor efficiency fix for `sprintf()`: when using numbered replacements, you can use the same replacement twice, so no need to have `'</a>'` twice in the replacement list.
AFAIK WHIP is intended to also be usable by other (non-Yoast) plugins/themes.
There were currently two phrases which made this awkward.

This PR changes those.

For the first phrase, I've chosen to change `WordPress and Yoast SEO` to `WordPress and all it's plugins and themes`. This incidentally fixes issue 35.
If it is preferred that `Yoast SEO` not be removed, I would suggest the following:
* Move the name out of the text string and put it back in using `sprintf()`.
* Make the name filterable to allow other plugins to pass in their plugin name.
    - Potentially, it could even be turned into an array which could be passed into the string using `implode()` to allow for a number of plugins to be mentioned.

For the second phrase, just changing the `our` in `vetted by our Yoast support team` to `the` improves the re-usability.

Fixes 35
@@ -35,11 +35,12 @@ public function body() {
$message = array();

$message[] = Whip_MessageFormatter::strongParagraph( __( 'Your site could be faster and more secure with a newer PHP version.', $textdomain ) ) . '<br />';
$message[] = Whip_MessageFormatter::paragraph( __( 'Hey, we\'ve noticed that you\'re running an outdated version of PHP. PHP is the programming language that WordPress and Yoast SEO are built on. The version that is currently used for your site is no longer supported. Newer versions of PHP are both faster and more secure. In fact, your version of PHP no longer receives security updates, which is why we\'re sending you to this notice.', $textdomain ) );
$message[] = Whip_MessageFormatter::paragraph( __( 'Hey, we\'ve noticed that you\'re running an outdated version of PHP. PHP is the programming language that WordPress and all it\'s plugins and themes are built on. The version that is currently used for your site is no longer supported. Newer versions of PHP are both faster and more secure. In fact, your version of PHP no longer receives security updates, which is why we\'re sending you to this notice.', $textdomain ) );
Copy link

Choose a reason for hiding this comment

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

its, not it's

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that!

@jdevalk jdevalk merged commit e8b595a into master Mar 27, 2018
@jdevalk jdevalk deleted the JRF/CS/i18n-fixes branch March 27, 2018 08:41
@jrfnl jrfnl added this to the Next release milestone May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yoast SEO shouldn't be translatable
2 participants