Problem/Motivation

Apologies if this has already been filed but I couldn't find anything.

This is one of the last blockers for us upgrading to Drupal 10. When using a CKEditor 5 editor in a block type in layout builder, the contents of the WYSIWYG are squashed together. This does not happen on 9.5.

There was a good deal of off canvas changes in D10 so I assume that's what has caused it.

Steps to reproduce

  1. Install Umami on 10.0
  2. Edit the Layout of a Recipe page (e.g /en/node/3/layout)
  3. Add a Basic Block
  4. Add some paragraphs
  5. Notice there is no linebreak between the paragraphs.

Drupal 10
WYSIWYG
Drupal 10 WYSIWYG
SOURCE
Drupal 10 WYSIWYG Source

Drupal 9.5
Drupal 9.5 WYSIWYG

CommentFileSizeAuthor
#22 3341737-22-D9.patch3.21 KBgordon
#11 3341737-11.patch879 byteslauriii
#10 3341737-10.patch1.39 KBacbramley
#9 3341737-nr-bot.txt2.52 KBneeds-review-queue-bot
#7 3341737-7.patch1.07 KBacbramley
#5 Screen Shot 2023-02-16 at 9.20.44 am.png574.43 KBacbramley
Screen Shot 2023-02-15 at 9.35.54 am.png25.94 KBacbramley
Screen Shot 2023-02-15 at 9.09.22 am.png30.76 KBacbramley
Screen Shot 2023-02-15 at 9.09.16 am.png26.53 KBacbramley

Issue fork drupal-3341737

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:

About issue forks

Comments

acbramley created an issue. See original summary.

acbramley’s picture

acbramley commented

@jptaranto has been helping to try and track this down so it would be great to get credit added for him.

He has pointed to https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/cked... in 9.5.x which does not seem to exist on 10.0.x https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/modules/cke...

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Postponed (maintainer needs more info)
Related issues: +#3291797: Refactor Drupal 10 settings tray / off-canvas to use modern CSS

@acbramley: thanks for that research! That made it simpler to find where this change between 9.5 and 10 was introduced, and that issue was #3291797: Refactor Drupal 10 settings tray / off-canvas to use modern CSS.

That being said … as a non-user of Settings Tray, it's not immediately clear to me what exactly the bug here is? 😅🙈 Could you please annotate the screenshot? 🙏

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Postponed (maintainer needs more info) » Active

I think the last step in the steps to reproduce explains what is happening in the screenshot: "Notice there is no linebreak between the paragraphs".

acbramley’s picture

acbramley commented
StatusFileSize
new Screen Shot 2023-02-16 at 9.20.44 am.png574.43 KB

@Wim Leers this is using Layout builder, it's when adding a new custom block with a WYSIWYG field on it, i.e the Basic block provided in the Umami demo profile.

Here's a screenshot of the whole page for more context.

Press Add Block (this opens the off canvas tray) > Create Custom Block (button at the top of the tray) > Basic Block

Full page view

acbramley’s picture

acbramley commented

When inspecting elements inside the CKEditor, if I uncheck this CSS rule it fixes the display:

#drupal-off-canvas-wrapper [data-drupal-ck-style-fence] .ck.ck-content * {
  display: initial;
}
acbramley’s picture

acbramley commented
Version: 10.0.x-dev » 10.1.x-dev
Component: layout_builder.module » ckeditor5.module
StatusFileSize
new 3341737-7.patch1.07 KB

Reverting this removal fixes the display for me https://git.drupalcode.org/project/drupal/-/commit/983562f41496eed45777a...

Here's a patch.

acbramley’s picture

acbramley commented
Status: Active » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3341737-nr-bot.txt2.52 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

acbramley’s picture

acbramley commented
Status: Needs work » Needs review
StatusFileSize
new 3341737-10.patch1.39 KB

Lint.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3341737-11.patch879 bytes

Here's another approach which solves the root cause to this problem. We could use revert for the reset to get the correct default values per element.

@acbramley thoughts on this approach?

Edit: If you are wondering why I kept color with initial, that's because by default color is inherited and we don't want that.

acbramley’s picture

acbramley commented

@lauriii that does look cleaner, I am no CSS expert though so I don't really know which is more correct. It does fix the issue for me :)

jptaranto’s picture

jptaranto
Primary language English
Location Sydney
commented

Hey @lauriii - nice one! Yes I think this is the way do go. display: initial is causing all these block level elements to get display: inline instead!

Here's a little explainer for anyone interested: https://css-tricks.com/inherit-initial-unset-revert/

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Thanks @jptaranto! If you think the change is ready, feel free to change the status to "Reviewed & tested by the community" 😊

jptaranto’s picture

jptaranto
Primary language English
Location Sydney
commented
Status: Needs review » Reviewed & tested by the community
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: CKE5 content squashed in off canvas sidebar » CKEditor 5 content squashed in off-canvas sidebar
Issue tags: +CSS, +Usability

Wow, a beautifully simple patch in the end! 🤩

kunal_sahu made their first commit to this issue’s fork.

kunal_sahu opened merge request !3528

kunal_sahu’s picture

kunal_sahu commented

Hi I have created a MR. Please merge. Thanks

acbramley’s picture

acbramley commented

@kunal_sahu there is no need to create an MR for a issue that already has an RTBC patch. This just creates more noise on the issue and confusion for the committers.

quietone’s picture

quietone commented

@kunal_sahu, I am removing credit as explained in #20 and per How is credit granted for Drupal core issues.

gordon’s picture

gordon commented
StatusFileSize
new 3341737-22-D9.patch3.21 KB

Thanks for this. It fixed a a lot of problems.

There are still other problems with colours. the Source view and the hr are still there.

I have also added a patch for Drupal 9.5

  • larowlan committed f609615f on 10.0.x 
    Issue #3341737 by acbramley, lauriii, gordon, jptaranto, Wim Leers:...

  • larowlan committed e64297ee on 10.1.x 
    Issue #3341737 by acbramley, lauriii, gordon, jptaranto, Wim Leers:...

  • larowlan committed 379c2266 on 9.5.x 
    Issue #3341737 by acbramley, lauriii, gordon, jptaranto, Wim Leers:...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 10.1.x and backported to 10.0.x and 9.5.x (via separate patch)

Thanks all

Status: Fixed » Closed (fixed)

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