Problem/Motivation

Sibling of #3228464: API for contrib projects to load CKEditor translations.

See https://git.drupalcode.org/project/ckeditor5/-/merge_requests/56#note_39145.

This will need tests similar to \Drupal\Tests\ckeditor\Kernel\CKEditorTest::testJSTranslation() for CKEditor 4 in core.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork drupal-3228778

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Issue summary: View changes
Issue tags: +Needs tests
Wim Leers’s picture

Project: CKEditor 5 » Drupal core
Version: 1.0.x-dev » 9.3.x-dev
Component: Code » ckeditor5.module
Issue tags: +JavaScript, +i18n

Devil's advocate: should we really do this?

The strongest reason to do this is to make it simpler to provide translations for the strings in the Drupal-specific CKEditor 5 plugins that are in core/modules/ckeditor5/js.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nod_’s picture

nod_’s picture

All right so at least, Using Drupal.t inside plugin code works. It is kept as-is after the build step and Drupal correctly picks up the string in the minified file (with and without a context defined).

So for me I don't think we have anything more to do than make use of Drupal.t inside plugins and adding the dependency to core/drupal in the drupal library that defines the ckeditor5 plugin assets.

Wim Leers’s picture

Status: Active » Needs work

All right so at least, Using Drupal.t inside plugin code works. It is kept as-is after the build step and Drupal correctly picks up the string in the minified file (with and without a context defined).

So for me I don't think we have anything more to do than make use of Drupal.t inside plugins and adding the dependency to core/drupal in the drupal library that defines the ckeditor5 plugin assets.

That sounds good!

The changes in the merge request look great — zero remarks 🤓

The only thing that remains here: test coverage, similar to CKE4's \Drupal\Tests\ckeditor\Kernel\CKEditorTest::testJSTranslation().

nod_’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
Wim Leers’s picture

Status: Needs review » Needs work
nod_’s picture

Status: Needs work » Needs review

Thanks for the text suggestions, always having trouble with that :)

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +stable blocker

Yeah I wish that were just part of phpcs … some day!

No more remarks here.

lauriii’s picture

Version: 9.4.x-dev » 10.0.x-dev
Status: Reviewed & tested by the community » Needs work

We need a Drupal 10 version of the MR too 😇

nod_’s picture

Status: Needs work » Needs review

10.x MR opened

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

The two MRs are making identical functional changes, the only differences are in the compiled JS.

  • lauriii committed cf6f4be on 10.0.x
    Issue #3228778 by nod_, Wim Leers: Drupal-specific CKEditor 5 plugins...

  • lauriii committed 6ec3148 on 9.4.x
    Issue #3228778 by nod_, Wim Leers: Drupal-specific CKEditor 5 plugins...

  • lauriii committed d672dfa on 9.3.x
    Issue #3228778 by nod_, Wim Leers: Drupal-specific CKEditor 5 plugins...
lauriii’s picture

Version: 10.0.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed cf6f4be and pushed to 10.0.x. Also committed the 9.x patch to 9.4.x and cherry-picked to 9.3.x because this only impacts CKEditor 5 which is experimental, and this makes some strings translatable that previously weren't which is nice improvement. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.