Skip to content

Add context-splitting for the string "none"#22095

Closed
audrasjb wants to merge 5 commits into
WordPress:trunkfrom
audrasjb:fix/contextualize-none-strings
Closed

Add context-splitting for the string "none"#22095
audrasjb wants to merge 5 commits into
WordPress:trunkfrom
audrasjb:fix/contextualize-none-strings

Conversation

@audrasjb

@audrasjb audrasjb commented May 5, 2020

Copy link
Copy Markdown
Contributor

Description

The strings "None" and "none" are used in many places.
Unfortunately, this string is impossible to translate correctly in many languages.
The reason is that depending on what noun is described, this word may need to be presented in various forms.
For reference, see the original ticket on Trac.

This changeset replaces __( 'None' ) occurrences with _x( 'None', 'Some context )

Fixes #22094

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.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@audrasjb

audrasjb commented May 5, 2020

Copy link
Copy Markdown
Contributor Author

Fixes #22094

@audrasjb

audrasjb commented May 7, 2020

Copy link
Copy Markdown
Contributor Author

I fixed one of the 3 linting errors, but I can't see what I need to do to fix the two last ones. Could someone explain me the issue please? thanks 🙂

@audrasjb

audrasjb commented May 8, 2020

Copy link
Copy Markdown
Contributor Author

Ok checks are green now 🍏 🙂

@youknowriad

youknowriad commented May 12, 2020

Copy link
Copy Markdown
Contributor

Seems good but I don't have a good i18n knowledge to judge the context splitting here. ccc @swissspidy @mcsf

label={ __( 'Link Rel' ) }
value={ rel || '' }
valuePlaceholder={ __( 'None' ) }
valuePlaceholder={ _x( 'None', 'Rel attribute value' ) }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
valuePlaceholder={ _x( 'None', 'Rel attribute value' ) }
valuePlaceholder={ _x( 'None', 'Link rel attribute value placeholder' ) }

@swissspidy

Copy link
Copy Markdown
Member

One suggestion and 1 conflict that needs to be resolved, otherwise LGTM

{ value: 'none', label: __( 'None' ) },
{
value: 'none',
label: _x( 'None', 'Preload value' ),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps:

Suggested change
label: _x( 'None', 'Preload value' ),
label: _x( 'None', '"Preload" value' ),

would make it more explicit.

I was also going to suggest appending "for Audio block" but I see that Video is meant to reuse this context.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Speaking of which, that __( 'Preload' ) just a few lines above seems like a quick win to add to this PR. 😉

_x( 'Preload', 'noun; Audio block parameter' )

@youknowriad

Copy link
Copy Markdown
Contributor

Looks like this is approved and went a bit stale? Can we refresh nd land it?

@pinarol pinarol removed their request for review October 1, 2020 14:36
Base automatically changed from master to trunk March 1, 2021 15:43
@aristath

Copy link
Copy Markdown
Member

I merged trunk and resolved conflicts in trunk...fix/contextualize-none-strings
@audrasjb you can merge that branch in your fork if you want to update it (audrasjb/gutenberg@fix/contextualize-none-strings...WordPress:fix/contextualize-none-strings), this one would be nice to land 👍 🚢

@ramonjd ramonjd added the Internationalization (i18n) Issues or PRs related to internationalization efforts label Aug 26, 2021
@ramonjd

ramonjd commented Aug 26, 2021

Copy link
Copy Markdown
Member

@audrasjb I'm happy to carry this one forward if you don't have time. Agree with other folks that it would be good to land to make life for our translators a little easier 😄

@audrasjb

Copy link
Copy Markdown
Contributor Author

Thank you for the proposal @ramonjd. Very appreciated cause I'm currently on holidays 😅

@ramonjd

ramonjd commented Aug 26, 2021

Copy link
Copy Markdown
Member

Thank you for the proposal @ramonjd. Very appreciated cause I'm currently on holidays 😅

Nice! Hope you have a great time. Wish I were on holidays too 🤣

I'll spin up a new PR soon. Thanks for your work on this.

ramonjd added a commit that referenced this pull request Aug 27, 2021
…rom #22095

This is because "None" can be translated differently in languages other than English depending on the context.
@ramonjd

ramonjd commented Aug 27, 2021

Copy link
Copy Markdown
Member

Updated PR #34341

Closing this one.

Thank you!

@ramonjd ramonjd closed this Aug 27, 2021
ramonjd added a commit that referenced this pull request Aug 27, 2021
* Adding context to 'none' strings, and also implementing suggestions from #22095
This is because "None" can be translated differently in languages other than English depending on the context.

* As always, my linter was asleep
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Context-splitting needed for the string "none"

6 participants