-
Notifications
You must be signed in to change notification settings - Fork 197
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
Translate title, description, keywords of a block #6994
Conversation
WordPress Dependencies ReportThe
This comment was automatically generated by the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #6994 +/- ##
=========================================
Coverage 51.21% 51.21%
Complexity 11188 11188
=========================================
Files 614 614
Lines 47228 47228
Branches 405 405
=========================================
Hits 24186 24186
Misses 22715 22715
Partials 327 327
Continue to review full report in Codecov by Sentry.
|
6dd8d69
to
707874e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as described
Resolved conflicts and tested that it still works. Return to open PR. |
@renatho could you take a look at the PR? I'm asking you as you might know a better way to translate problematic blocks properly? |
Thank you for pinging me. I think we shouldn't do those changes because they are the old way to do it. The issue is probably related to something else. Adding the I remember @m1r0 already fixed this for some blocks in the past, pinging him here too, just in case he knows the issue off the top of his head. Anyway, I'll keep this PR in my list to debug a little and see if I can find something. 😉 |
Thanks for the ping Renatho, I remember working on this and there were some hurdles. In #5782 I've tried to fix some translation issues by updating the block.json files. This is the new way of doing block translation as Renatho said. But this caused some errors with the Quiz block #5782 (comment). So I've reverted the change in #5905 but note that it wasn't a full revert. I'm not sure what's the correct approach here, but hopefully this helps. |
Phew! 😮💨 This was a very hard one to debug, but I found the difference between the blocks where it's working and the blocks where it's not working. We need to register the ones that are not working on the server side. For the Quiz, I noticed it has a class, but it's skipping on the admin side. As a test, I added it to the Sensei_Blocks::register_sensei_block(
'sensei-lms/course-actions',
[],
Sensei()->assets->src_path( 'blocks/course-actions-block/course-actions' )
);
Sensei_Blocks::register_sensei_block(
'sensei-lms/quiz',
[],
Sensei()->assets->src_path( 'blocks/quiz/quiz-block' )
); Those translations are coming from the HTML (probably from the core because I didn't find it in Gutenberg). It's a call to |
Resolves part of #6351
This PR is an attempt to "fix symptoms".
It is still not clear for me, why I had to remove title and description from block.json for one blocks, and didn't need to do that for others. Looks like it shadowed them somehow, but I didn't find an answer why.
Proposed Changes
Testing Instructions
npm run i18n:build && npm run build:assets
Pre-Merge Checklist