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.

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

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Jun 27, 2023

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

  • Make titles and descriptions for all Sensei blocks translatable.

Testing Instructions

  1. Run npm run i18n:build && npm run build:assets
  2. Install Loco Translate, change website language.
  3. Go to Loco Translate -> Sensei LMS -> Synchronize.
  4. Find strings "Quiz" and "Course Actions" and add translations for them.
  5. Create a new lesson and open Inserter. Check Quiz and Course Actions use your translation.
  6. Go further and translate descriptions for those blocks.
  7. Then add these blocks in the lesson (or course) and open the Inspector. Both the block title and the description should use your translations.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@github-actions
Copy link

github-actions bot commented Jun 27, 2023

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit 40645fe and trunk. Please review and confirm the following are correct before merging.

Script Handle Added Dependencies Removed Dependencies Total Size Size Diff
blocks/single-page.js 15.7 kB +61 B ( +0.39% 🔼 )
blocks/single-course.js 27.1 kB +106 B ( +0.4% 🔼 )
blocks/single-lesson.js 8.11 kB +69 B ( +0.86% 🔼 )
blocks/lesson-action-blocks.js 13.6 kB +38 B ( +0.29% 🔼 )
blocks/global-blocks.js 20.6 kB +96 B ( +0.47% 🔼 )
blocks/quiz/index.js 32.3 kB +78 B ( +0.25% 🔼 )
blocks/shared.js 14 kB +34 B ( +0.25% 🔼 )
blocks/core-pattern-polyfill/core-pattern-polyfill.js wp-i18n 5.07 kB +49 B ( +0.98% 🔼 )
course-theme/blocks/index.js 13.9 kB +47 B ( +0.34% 🔼 )

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f25df0) 51.21% compared to head (40645fe) 51.21%.
Report is 1 commits behind head on trunk.

Additional details and impacted files

Impacted file tree graph

@@            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           
Files Coverage Δ
...ssets/blocks/quiz/category-question-block/index.js 0.00% <ø> (ø)
assets/blocks/quiz/question-answers-block/index.js 0.00% <ø> (ø)
assets/blocks/quiz/question-block/index.js 0.00% <ø> (ø)
...ts/blocks/quiz/question-description-block/index.js 0.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e982589...40645fe. Read the comment docs.

@merkushin merkushin requested a review from a team June 29, 2023 21:55
@merkushin merkushin self-assigned this Jun 29, 2023
@merkushin merkushin marked this pull request as ready for review June 29, 2023 22:57
Imran92
Imran92 previously approved these changes Jul 11, 2023
Copy link
Contributor

@Imran92 Imran92 left a comment

Choose a reason for hiding this comment

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

Works as described

@merkushin merkushin added this to the 4.16.1 milestone Jul 24, 2023
@merkushin merkushin marked this pull request as draft July 27, 2023 09:06
@donnapep donnapep modified the milestones: 4.16.1, 4.16.2 Aug 14, 2023
@donnapep donnapep modified the milestones: 4.17.0, 4.17.1 Sep 12, 2023
@donnapep donnapep modified the milestones: 4.18.0, 4.19.0 Oct 11, 2023
@merkushin
Copy link
Member Author

Resolved conflicts and tested that it still works. Return to open PR.

@merkushin merkushin marked this pull request as ready for review February 2, 2024 06:58
@merkushin
Copy link
Member Author

@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?

@renatho
Copy link
Contributor

renatho commented Feb 5, 2024

could you take a look at the PR?

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 textdomain should be enough: https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#internationalization

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. 😉

@m1r0
Copy link
Member

m1r0 commented Feb 6, 2024

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.

@m1r0 m1r0 modified the milestones: 4.20.2, 4.20.3 Feb 7, 2024
@renatho
Copy link
Contributor

renatho commented Feb 9, 2024

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 register_block method of the Sensei_Course_Progress_Block class:

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 wp.blocks.unstable__bootstrapServerSideBlockDefinitions, which registers the blocks from the server, but it only happens for the ones registered through PHP.

@merkushin merkushin modified the milestones: 4.21.0, 4.21.1 Feb 27, 2024
@donnapep donnapep removed this from the 4.22.0 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants