Problem/Motivation
Per https://ckeditor.com/blog/ckeditor-5-v26.0.0-with-extensible-builds-inli..., CKEditor 5 v26 brings back the "language of parts" plugin from CKEditor 4 — see https://github.com/ckeditor/ckeditor5/issues/8989.
Drupal 8 core added this in the form of \Drupal\ckeditor\Plugin\CKEditorPlugin\Language in #1993928: Language of parts: Introduce a language toolbar button.
This is important to not regress in functionality compared to CKEditor 4.
We already updated to CKEditor 5 v26 in #3202334: Update to CKEditor5 26.0.0, so this should be relatively straightforward.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | Screenshot 2021-05-11 at 6.58.17 PM.png | 125.97 KB | anand.toshniwal93 |
| #12 | Screenshot 2021-05-11 at 6.57.31 PM.png | 74.56 KB | anand.toshniwal93 |
| #11 | Screenshot 2021-04-28 at 1.53.14 AM.png | 153.19 KB | anand.toshniwal93 |
| #11 | Screenshot 2021-04-28 at 2.04.33 AM.png | 169.74 KB | anand.toshniwal93 |
| #9 | Screenshot 2021-04-28 at 2.04.33 AM.png | 169.74 KB | anand.toshniwal93 |
Issue fork ckeditor5-3205565
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:
Comments
Comment #2
zrpnrTagging for Contribution event
Comment #3
gábor hojtsyFixing tags.
Comment #4
anand.toshniwal93 commentedworking on this so assigning to myself
Comment #6
anand.toshniwal93 commentedI have converted PNG to SVG for the language icon as CKEditor doesn't provide an icon for it. Please check if we need to update it.
Comment #7
anand.toshniwal93 commentedNeed to provide TextPartLanguageOption.
Comment #8
anand.toshniwal93 commentedComment #9
anand.toshniwal93 commentedI was trying to follow the same pattern as "imageUpload" for id but facing the below error so I set the plugin id to "language".
Drupal\Core\Config\ConfigValueException: ckeditor5.language key contains a dot which is not supported. in Drupal\Core\Config\ConfigBase->validateKeys() (line 211 of core/lib/Drupal/Core/Config/ConfigBase.php).Comment #10
wim leersI wanted to RTBC this, and while this looks great already … the configuration parts of this do not yet work 😅 Any chance you could fix that? 😊🙏
Comment #11
anand.toshniwal93 commented@Wim Leers Forgot to upload the screenshot showing config works, please check the attached screenshots. The screenshot previously attached were before creating the configuration form.
will work on this.
Comment #12
anand.toshniwal93 commentedComment #13
wim leersOh, wow, that does appear to work — which is super surprising because it means that the configuration value that CKEditor 5 accepts is identical to that for CKEditor 4! 🤯
Any chance you could add a test similar to
\Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5ContribTest::testContribPluginAdded(), where you add this CKEditor 5 plugin, configure it, and check that the expected languages appear in the dropdown when interacting with CKEditor 5 as a content author? 🙏🤓Comment #14
anand.toshniwal93 commentedI will work on the test case part.
Comment #15
wim leersThanks, you rock @anand.toshniwal93! 🥳
Comment #16
anand.toshniwal93 commentedworking on this so assigning to myself
Comment #17
wim leers🥳
Comment #18
anand.toshniwal93 commentedAfter adding this plugin kernel test case is failing due to the order of plugins in case 2
This issue might come up with each new plugin added, is it ok to sort an array and assert them or the order of the plugin is important here?
Comment #19
anand.toshniwal93 commentedIn test case we are using
$plugin_ids = array_keys($this->manager->getEnabledDefinitions($editor));getEnabledDefinitionsfunction sorting plugin ids so I think we can use the code from comment 12 .Comment #20
anand.toshniwal93 commentedThis is fixed.
Added tests for the language of parts plugin.
Fixed failing kernel test after adding a plugin.
Comment #21
zrpnrThis works well, the settings form lets a user pick which set of languages to use, which matches the lists CKEditor 4 loads.
I'm also excited to see the `settingsForm` used here, I think it's the first new plugin that has configuration (besides ImageUpload)
I left some feedback in the issue fork for a few changes, but this is really close, nice work @anand.toshniwal93 !
Comment #22
anand.toshniwal93 commentedworking on this so assigning to myself
Comment #23
wim leersWhile reviewing this, I opened two issues:
@anand.toshniwal93's excellent work here had made these problems apparent 👍🙏
There is still one important regression compared to #1993928: Language of parts: Introduce a language toolbar button though, but we can't fix that here. Opened #3215603: Follow-up for #3205565: bring back visual annotation and contacted the CKEditor team for that.
Comment #24
wim leersAs @zrpnr wrote:
Plugin system maintainer @tim.plunkett opened an issue exactly for the problem you encountered — that'd have helped prevent this design choice early on: #3215662: Putting "." in a plugin ID will cause problems if you attempt to store configured plugins via the Configuration API.
We'll open an issue + merge request shortly to change this.
Comment #25
zrpnrWhen I test
testLanguageOfPartsPluginlocally it fails, but it seems to be because the dropdown is opening up and it's cut off.Curious if the testbot will pass still, I can't see where anything changed.
Comment #26
zrpnrThere's also an incomplete schema exception for
editor.editor.ckeditor5:settings.plugins.language"missing schema"Comment #27
anand.toshniwal93 commentedComment #28
zrpnrThis should be back to needs review, I added the schema and rebased off the latest 1.0.x.
Comment #29
wim leersManually tested one more time.
Relevant follow-ups:
🚀
Comment #31
zrpnrUpdated the postponed issues, thanks!