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.

Sign up for GitHub

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

Jump to bottom

Removed an i18n override for Gutenberg. #13432

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Removed an i18n override for Gutenberg. #13432

merged 1 commit into from
Sep 20, 2019

Conversation

zinigor
Copy link
Member

@zinigor zinigor commented Sep 5, 2019 •
edited by kraftbj
Loading

Fixes #11491
Previously we were using our own generated files to provide translations for Gutenberg blocks. But since Core can already do it for us, we just remove the override mechanism and let blocks load translations from the Jetpack textdomain. This fixes broken block translations. Props to @jeherve for the find!

Changes proposed in this Pull Request:

  • Removes an override filter added to use a JSON file shipped with the plugin for Gutenberg block translations.

Testing instructions:

  • Switch to a language other than English.
  • Open the post editor, add some Jetpack blocks.
  • See that there are no translations available for the master branch.
  • Apply this branch, update translations if you have that option in the updates section of the dashboard.
  • Reload the post editor, see that the translations are there.

Proposed changelog entry for your changes:

  • N/A

@zinigor
Removed an i18n override for Gutenberg.
e944613 
Previously we were using our own generated files to provide translations for Gutenberg blocks. But since Core can already do it for us, we just remove the override mechanism and let blocks load translations from the Jetpack textdomain. This fixes broken block translations.
@zinigor zinigor added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Focus] i18n Internationalization / i18n, adaptation to different languages labels Sep 5, 2019
@zinigor zinigor requested review from simison, jeherve and a team September 5, 2019 11:15
@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against e944613

@zinigor
Copy link
Member Author

zinigor commented Sep 5, 2019

Apparently fixes #11491

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello zinigor! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D32524-code before merging this PR. Thank you!

@simison simison requested a review from deBhal September 5, 2019 12:07
@jeherve jeherve added this to the 7.8 milestone Sep 19, 2019
kraftbj
kraftbj approved these changes Sep 19, 2019
View reviewed changes
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Works like a charm. Tested against Jetpack master in Spanish. On master, blocks were still in English. On this branch, works like a charm and in Spanish.

jeherve
jeherve approved these changes Sep 20, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work. Merging.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 20, 2019
@jeherve jeherve merged commit c497d8c into master Sep 20, 2019
@jeherve jeherve deleted the remove/core-js-i18n-override branch September 20, 2019 08:03
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
@jeherve
Changelog: add #13432
4e210dd
@akirk
Copy link
Member

akirk commented Sep 23, 2019

Should this PR also have included the removal of all *1bac79e646a8bf4081a5011ab72d5807 *.json files under https://github.com/Automattic/jetpack/tree/master/languages/json ?

@jeherve
Copy link
Member

jeherve commented Sep 23, 2019

@akirk I believe those are still used to translate strings in the Jetpack dashboard, outside of Gutenberg:

jetpack/_inc/lib/admin-pages/class.jetpack-react-page.php

Line 289 in 50fe8a7

'locale' => Jetpack::get_i18n_data_json(),

jeherve added a commit that referenced this pull request Sep 24, 2019
@jeherve
Changelog: add #13432
5a42e24
jeherve added a commit that referenced this pull request Sep 24, 2019
@jeherve
Changelog: changes for 7.8 (#13402)
d52a0f3 
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
@jeherve
[not verified] Changelog: changes for 7.8 (#13402)
e50dedc 
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

@kraftbj kraftbj kraftbj approved these changes

@simison simison Awaiting requested review from simison

@deBhal deBhal Awaiting requested review from deBhal

Assignees

@zinigor zinigor

Labels
[Focus] i18n Internationalization / i18n, adaptation to different languages Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
7.8
Development

Successfully merging this pull request may close these issues.

Translations Not Fully Loading for Blocks
6 participants
@zinigor @jetpackbot @matticbot @akirk @jeherve @kraftbj