Skip to content

Specify domain when extracting strings from Jetpack and Layout Grid#4409

Merged
fluiddot merged 2 commits into
developfrom
update/make-pot-plugins-commands
Jan 5, 2022
Merged

Specify domain when extracting strings from Jetpack and Layout Grid#4409
fluiddot merged 2 commits into
developfrom
update/make-pot-plugins-commands

Conversation

@fluiddot
Copy link
Copy Markdown
Contributor

@fluiddot fluiddot commented Dec 29, 2021

I noticed that we're adding the string Cancel in the localization strings files, which is referenced in the Layout Grid plugin (file reference). The string is retrieved from the default domain, which in this case it would be Gutenberg, and it's already translated in the GlotPress project (reference), so we shouldn't really add it.

The makepot commands we use for extracting the strings from plugins like Jetpack and Layout Grid were ignoring the domain, which like the Cancel string case, they might include undesired extra strings into the localization strings files. This PR updates those commands and specify a domain for those plugins to prevent this behavior.

NOTE: A side effect of this change, although I guess it's expected, is that strings referencing a different domain than the one associated with the plugin (i.e. Jetpack references the jetpack domain) retrieved in *.native.js files of plugins, won't be included in the localization strings files.

To test:

  1. Run command npm run genstrings.
  2. Observe that the localization strings files (bundle/ios/GutenbergNativeTranslations.swift and bundle/android/strings.xml) remain unmodified.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot requested a review from jhnstn December 29, 2021 14:27
@fluiddot fluiddot self-assigned this Dec 29, 2021
@fluiddot fluiddot added this to the 1.69.0 (19.0) milestone Dec 29, 2021
<string name="gutenberg_native_build_layouts" tools:ignore="UnusedResources">Build layouts</string>
<string name="gutenberg_native_button_link_url" tools:ignore="UnusedResources">Button Link URL</string>
<string name="gutenberg_native_button_position" tools:ignore="UnusedResources">Button position</string>
<string name="gutenberg_native_cancel" tools:ignore="UnusedResources">Cancel</string>
Copy link
Copy Markdown
Contributor Author

@fluiddot fluiddot Dec 29, 2021

Choose a reason for hiding this comment

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

  • Referenced in block-experiments/blocks/layout-grid/src/grid/variation-control/index.native.js (reference)
  • Translated in Gutenberg GlotPress project (reference)

<string name="gutenberg_native_take_a_video" tools:ignore="UnusedResources">Take a Video</string>
<string name="gutenberg_native_tap_here_to_show_help" tools:ignore="UnusedResources">Tap here to show help</string>
<string name="gutenberg_native_tap_to_hide_the_keyboard" tools:ignore="UnusedResources">Tap to hide the keyboard</string>
<string name="gutenberg_native_text_color" tools:ignore="UnusedResources">Text color</string>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a new string that has been recently added (PR reference).

  • Referenced in gutenberg/packages/format-library/src/text-color/index.native.js (reference)
  • Already translated in Gutenberg GlotPress project (reference) but since it's only referenced in a *.native.js file, it will most likely be removed eventually.

@ttahmouch
Copy link
Copy Markdown
Contributor

Hey, @fluiddot . We will cut the 1.69.0 release on Thursday, January 6, 2022. We plan to circle back and bump this PR to the next milestone then, but please let me know if you'd rather us work to include this PR in 1.69.0.

Copy link
Copy Markdown
Member

@jhnstn jhnstn left a comment

Choose a reason for hiding this comment

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

Looks good, npm run genstrings executed as expected.

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.

3 participants