This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Mini Cart block - fix translations handling #6153 #6158
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mini Cart block - fix translations handling
gigitux
added
skip-changelog
|
Size Change: +620 B (0%) Total Size: 862 kB
ℹ️ View Unchanged
|
|
Added as reviewers @dinhtungdu and @Aljullu that have participated in the discussion |
gigitux
removed
the
skip-changelog
dinhtungdu
reviewed
Mar 30, 2022
There was a problem hiding this comment.
Tested with deutsch and all strings are translated 🎉 . However, the translations are included in the server-rendered HTML with this approach. I think we should lazy load the translations instead. #6164 is my idea to lazy load the translations, can you please take a look?
src/BlockTypes/MiniCart.php
Outdated
| 'version' => $script->ver, | ||
| 'before' => $wp_scripts->print_inline_script( $script->handle, 'before', false ), | ||
| 'after' => $wp_scripts->print_inline_script( $script->handle, 'after', false ), | ||
| 'translations' => $wp_scripts->print_translations( $script->handle, false ), |
There was a problem hiding this comment.
Looking closer, I notice that the $wp_scripts->print_translations() of enqueued scripts cause the JS errors; even we don't echo them, they are still printed.
There was a problem hiding this comment.
Not sure if related, but I remember being confused by something similar and it was because we hook into pre_load_script_translations to print the translations, which runs no matter if you echo them or not:
https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/2d0270cc23121f79a5d3f1759a7e7df43f1323a4/woocommerce-gutenberg-products-block.php#L239
There was a problem hiding this comment.
You're the lifesaver @Aljullu! Thanks so much for the suggestion!
dinhtungdu
reviewed
Mar 30, 2022
Co-authored-by: Tung Du <[email protected]>
Aljullu
approved these changes
Mar 31, 2022
There was a problem hiding this comment.
Thanks for investigating this tricky issue, @gigitux and @dinhtungdu! I could verify this PR fixes the issue, and translations are loaded correctly, so I'm pre-approving. Besides that, I left a couple of questions of changes to help me understand the changes.
|
@Aljullu I updated the PR to load translations close to the associated scripts. Please take another look! Thanks so much! |
Aljullu
approved these changes
Apr 1, 2022
tjcafferkey
pushed a commit
that referenced
this pull request
Apr 5, 2022
* Mini Cart block - fix translations handling #6153 Mini Cart block - fix translations handling * Mini Cart block - fix translations handling (#6164) * loads translations for deps Co-authored-by: Tung Du <[email protected]> * address code review. load translations close to associated scripts Co-authored-by: Tung Du <[email protected]>
nielslange
added
the
focus: i18n
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes how the Mini Cart block loads the translations.
Fixes #6153
Technical details
I guess that the current method of how the Mini Cart block loads the translations is not correct. It seems that all the blocks using the method
register_chunk_translations.@dinhtungdu noticed that the function
append_script_and_deps_srcis used to lazy load the dependencies of Mini Cart and payment method scripts. Are we sure that the line about the translation is necessary?Another doubt that I have: what are the advantages to split the handling of the translations compared to the other scripts with the suffix
before? (here the code)Testing
Manual Testing
How to test the changes in this Pull Request:
Check out this branch
Mini Cartwidget to any Widget Area.wp.i18n.Mini Cartis translated (check with an empty cart and with a filled cart). Please, be sure that you are using a language that has translations for the Mini Cart Block. My suggestion is to use the Dutch language.Mini Cartblock in the header. Save.Mini Cartis translated (check with an empty cart and with a filled cart). Please, be sure that you are using a language that has translations for the Mini Cart Block. My suggestion is to use the Dutch language.User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
Changelog