Problem/Motivation

CKEditor 5 will be the best core editing experience going forward. Umami should demonstrate the best features of Drupal, and its rich text editor is no exception.

Steps to reproduce

N/A

Proposed resolution

Configure Umami to use CKEditor 5

Remaining tasks

  1. Edit configuration to use CKEditor5 for existing text formats
  2. Change CKEditor stylesheets for CKEditor 5

User interface changes

Umami to replace all instances of CKEditor 4 in its interface with CKEditor 5.

API changes

None

Data model changes

None that we care about.

Release notes snippet

CommentFileSizeAuthor
#17 reroll_diff_14-17.txt5.54 KBravi.shankar
#17 3273665-17.patch6.07 KBravi.shankar
#14 3273665-patch-ported-14.patch6.16 KBimmaculatexavier
#10 3273665-10-6_interdiff.txt1.02 KBeli-t
#10 3273665-10_remove_span_and_code_from_filter_format.patch6.19 KBeli-t
#9 3273665-9-6_interdiff.txt85.25 KBeli-t
#9 3273665-9_remove_span_and_code_from_filter_format.patch91.16 KBeli-t
#6 interdiff-3273665-6-5.txt1.16 KBeli-t
#6 3273665-6-remove_code_and_language.patch4.99 KBeli-t
#5 interdiff-3273665-5-3.txt340 byteseli-t
#5 3273665-5--config_for_ckeditor5_remove_css.patch5.07 KBeli-t
#3 3273665-config_for_ckeditor5.patch4.58 KBeli-t

Comments

Eli-T created an issue. See original summary.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Issue summary: View changes
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Active » Needs review
StatusFileSize
new 3273665-config_for_ckeditor5.patch4.58 KB

Patch updating config to use CKEditor 5.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Related issues: +#3273746: Move media-embed-error class styling from core themes to CKEditor 5
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new 3273665-5--config_for_ckeditor5_remove_css.patch5.07 KB
new interdiff-3273665-5-3.txt340 bytes

Removed the ckeditor_stylesheets component from umami.info.yml.

This needs some action as described in this CR: https://www.drupal.org/node/3259165

However, the styling applied here is no longer required because CKEditor 5 renders this error in an acceptable way, so rather than amend to work with CKEditor 5 it is simply removed instead.

#3273746: Move media-embed-error class styling from core themes to CKEditor 5 has been raised to centralise this styling in CKEditor 5 should it later be required.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new 3273665-6-remove_code_and_language.patch4.99 KB
new interdiff-3273665-6-5.txt1.16 KB

The automated migration from CKE4->5 settings included language and code blocks, which are not required. This new patch removes them.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Issue summary: View changes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +ddd2022

#6: doesn't that one cause validation errors? I think you modified that manually?

  1. Umami's filter_html is configured to allow <span> but nothing in CKEditor 5 can create it. That's why the textPartLanguage button was automatically added.

    If you remove that button, you should also remove the <span>.

  2. I see you also removed code from Umami's "Basic HTML" text editor. Similar story here: the text format allows <code>, therefore we need a button for it. Alternatively, stop allowing <code>.
  3. Regarding codeBlock being added: that baffles me! It should have added the code button, not codeBlock. Can you reliably reproduce this? Ah, I see it was added for the Full HTML text format! That's why. The CKEditor 4 "Formats" dropdown allowed code blocks to be created too when using Full HTML. So we try to match that. Choosing to remove that is totally fine! 👍

Needs work for the first two points. But … almost there! 🥳

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
StatusFileSize
new 3273665-9_remove_span_and_code_from_filter_format.patch91.16 KB
new 3273665-9-6_interdiff.txt85.25 KB

This patch addresses the comments in #6

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new 3273665-10_remove_span_and_code_from_filter_format.patch6.19 KB
new 3273665-10-6_interdiff.txt1.02 KB

Previous patch is nonsense.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Patch (to be ported)

I would RTBC this 😄

… but Eli pointed out yesterday that it can’t be committed yet, since Umami can only use stable functionality, and CKEditor 5 is not yet declared stable.

So … picking the closest matching state instead 🤓 (Not sure if this is appropriate.)

@Eli-T One final question: do you intend to also get this committed to 9.4?

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

@Eli-T One final question: do you intend to also get this committed to 9.4?

Hell yeah.

Even if it isn't default in standard install profile, if it's stable in 9.4 we can and should use it in Umami.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

👍🚀

immaculatexavier’s picture

immaculatexavier commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 3273665-patch-ported-14.patch6.16 KB

Patch ported for 9.4

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Postponed

This is not yet committable. Marking Postponed.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Postponed » Needs work
Issue tags: +Needs reroll

Due to #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins having modified the existing Text Editor config (for CKEditor 4) slightly, this needs a reroll.

ravi.shankar’s picture

ravi.shankar commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 3273665-17.patch6.07 KB
new reroll_diff_14-17.txt5.54 KB

Added reroll of patch #14 on Drupal 9.4.x.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Looks perfect!

  • lauriii committed 6df89ba on 10.1.x 
    Issue #3273665 by Eli-T, ravi.shankar, immaculatexavier, Wim Leers: Use...

  • lauriii committed 9624a9a on 10.0.x 
    Issue #3273665 by Eli-T, ravi.shankar, immaculatexavier, Wim Leers: Use...

  • lauriii committed 71f4dc4 on 9.5.x 
    Issue #3273665 by Eli-T, ravi.shankar, immaculatexavier, Wim Leers: Use...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 10.0.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

The Umami subsystem maintainers have made it clear that Umami is for demonstrating stable features. We are making an exception committing this regardless of CKEditor 5 being experimental because CKEditor 5 is a hard requirement for Drupal 9.5.0 and 10.0.0 to be released. This means that even though Umami is temporarily depending on an experimental feature, it will be stable by the time we tag a release.

Committed 6df89ba and pushed to 10.1.x. Also cherry-picked to 10.0.x and 9.5.x. Thanks!

Status: Fixed » Closed (fixed)

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