CKEditor 5 should not grow to infinite height

@mherchel to avoid the:

The user can still inadvertently (or purposefully) scroll down and have a significant part of the editor out of screen. Not sure if this is a problem.

I introduced a small css tweak which resolved the issue for us in Drupal 9.5.x → :

.ck-editor__main > :is(.ck-source-editing-area) > textarea {
  overflow: auto;
}

Verified the patch #33 and tested it on Drupal version 10.1.x. The editor initially has a min height and when content is added, it stops growing after a certain max height. Then an internal scroll will appear within the editor. I have added the before and after screen recordings for reference.

The before vs after videos from @Nayana Ramakrishnan make it crystal clear that this is a huge leap forward in usability! 🤩

And magnificently, this is a CSS-only change! 🥳

This is a no-brainer improvement 👍

I think it's deserving of this 😇

Just updating tag

We need to understand whether 🐛 CKEditor 5 can grow past the viewport when there is a lot of content Closed: duplicate is a duplicate of this or has other purpose. Not very clear to me.

Still needs tests as per #33

I didn't even realize we could write tests for this; that used to be completely impossible. I guess it indeed is possible here. But given it's viewport-related I have some reservations about how reliable that test could work on headless Chrome DrupalCI testing 🤔

@mherchel & @lauriii: thoughts?

Over at 🐛 CKEditor 5 can grow past the viewport when there is a lot of content Closed: duplicate , I wrote:

Implement the solution that https://ckeditor.com/docs/ckeditor5/latest/installation/getting-started/... recommends:

Classic editor (CKEditor 5) no longer encapsulates the editing area in an <iframe>, which means that the height (and similar options) of the editing area can be easily controlled with CSS. For example, the minHeight and maxHeight settings can be achieved with the following code:

.ck.ck-content:not(.ck-comment__input *) {
/* Note: You can use min-height and max-height instead here. */
  height: 300px;
  overflow-y: auto;
}

… but combine this with JavaScript that computes the desired height, just like we did in #2986147: Autogrow for non-admin users causes CKEditor grow past the viewport when there is a lot of content in the CKEditor → .

And 🐛 CKEditor 5 isn't respecting field widgets row settings Fixed already implemented min-height, this issue is adding max-height. We are using a different selector than the official docs recommended, but given that we use the "classical" editor UI, I think .ck-editor__main is fine — plus, the official docs were written before the Source Editing plugin existed, so our addition to the selector just for that IMHO makes sense: .ck-editor__main > :is(.ck-editor__editable, .ck-source-editing-area).

Apologies for the duplicate issue 🙈

Transfering issue credit and fixing title.

Wim Leers → credited catch → .

Wim Leers → credited Reinmar → .

Wim Leers → credited saidatom → .

Wim Leers → credited frega → .

Same comment as in #3241295-99: CKEditor 5 isn't respecting field widgets row settings → : This is a bug that affects also Drupal 9.5 and 9.5 is still long time supported (until December 2023). Not porting this to 9.5 is affecting the CKEditor 5 adoption. Most of the sites are now on Drupal 9.5 (or even 9.4), preparing for D10. Adopting CKEditor 5 is one of steps they want (true, not mandatory, but highly recommended) to solve before migrating to Drupal 10. But this is the kind of bug that wouldn't make CKEditor 5 module production ready.

Let's backport this, at least, to Drupal 9.5 to ease the CKE5 adoption. It seems that the 9.x version requires 🐛 CKEditor 5 isn't respecting field widgets row settings Fixed to be fixed first because they touch the same area of code.

Re #33:

+  /* Set the max-height to not grow beyond the height of the viewport (minus
+   * any toolbars. */
+  max-height: calc(100vh - var(--drupal-displace-offset-top, 0px) - var(--drupal-displace-offset-bottom, 0px) - 20px);

Should the internal.drupal.ckeditor5 have also core/drupal.displace as dependency, in ckeditor5.libraries.yml?

This is a D9.5 tentative. It cumulates:

• #3256768: Drupal.displace() should set CSS Variables indicating displacement values →
• 🐛 CKEditor 5 isn't respecting field widgets row settings Fixed
• Fix from #33

...so not something that can be merged but it might be used as patch for D95 projects.

I am not a UX expert, so I am not sure if this is really problematic :). I'd like to point out though that the suggested max-height only considers the "ckeditor-editor part". The label, help text and text format selector will not be (fully) visible in the viewport when the CKEditor has reached it's maximum height.

I attached a video to demonstrate this. Demo styling has been applied: ckeditor (light yellow background) grows to fill the full viewport (minus drupal-specific "safe displace areas") but parts of the complete drupal field (light blue background) are pushed out of the full viewport.

Also not a CSS person, but 100vh used (?) to be problematic when used on mobile (_native_ not desktop-responsive) where relying on 100vh for "full viewport" can be tricky (e.g. https://chanind.github.io/javascript/2019/09/28/avoid-100vh-on-mobile-we...).

PS. I'd be happy to give a shot at amending the existing nightwatch test (adding "maximum height") in drupal/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js ...

Here a (functional js) test-only patch (failing) and a passing test with CSS attached.

As stated in #52 I see some UX issues, but this can easily be adjusted accordingly.

fix ccf error in #53

Sorry for the noise, coding standard fixes and cspell-related issue resolved. only-test.patch should fail the other one should pass.

Reacquainting myself with core contrib and its linting processes, sorry🤦.

The #56 patch is identical to @mherchel's in #33, just with added test coverage goodness! 🤩

Looks great!

@wimleers - a quick pointer to #52 so it doesn't lost. I am not sure the UX is quite optimal yet (body field label and text input selector pushed outside the viewport, see video in #52)

+++ b/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js
@@ -100,6 +102,30 @@ module.exports = {
+            window.Drupal.CKEditor5Instances.forEach((instance) => {
+              instance.setData('<p>Llamas are cute.</p>'.repeat(100));
+            });

❤️🦙

Committed c3a9cde and pushed to 10.1.x. Thanks!

Since this is a feature, it doesn't get a backport to 10.0.x or 9.5.x.

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

Just sharing a backported patch for 9.5.x in case anyone else needs it. Thanks to everyone who worked on this issue!

@Cottser Thank you

Added default height

I have a question about this issue.

First, the setting: site is running Drupal 9.5.9, PHP 8.1. I just switched to ckeditor5 as part of my preparations for migrating to Drupal 10.

I am trying to apply the patch released on this thread to prevent my editor fields from auto-growing. However, I'm finding a strange bug in the process.

If I apply a very simple patch that simply adds the relevant css to editor.css (or any of the patches on this thread, but I'm trying to keep things simple), the patch applies and it has the desired effect on auto-grow. But another thing happens, too...

The file docroot/core/modules/node/src/NodeViewBuilder.php also gets changed, like so:

--- a/docroot/core/modules/node/src/NodeViewBuilder.php
+++ b/docroot/core/modules/node/src/NodeViewBuilder.php
@@ -93,10 +93,15 @@ public static function renderLinks($node_entity_id, $view_mode, $langcode, $is_i
       '#attributes' => ['class' => ['links', 'inline']],
     ];
 
-    if (!$is_in_preview) {
+    if ($is_in_preview) {
       $storage = \Drupal::entityTypeManager()->getStorage('node');
       /** @var \Drupal\node\NodeInterface $revision */
       $revision = !isset($revision_id) ? $storage->load($node_entity_id) : $storage->loadRevision($revision_id);
+
+      if(!isset($revision)) {
+        return links;
+      }
+
       $entity = $revision->getTranslation($langcode);
       $links['node'] = static::buildLinks($entity, $view_mode);

These changes--I'm pretty sure it's these changes--cause Drupal to WSOD on node saves for some, but not all, content types. The stacktrace on these errors is like so:

2023/06/07 13:53:07 [error] 1890#1890: *13713 FastCGI sent in stderr: "PHP message: AssertionError: Cannot load the "node" entity with NULL ID. in /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php on line 295 #0 /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(295): assert(false, 'Cannot load the...')
#1 /var/www/html/docroot/core/modules/node/src/NodeViewBuilder.php(99): Drupal\Core\Entity\EntityStorageBase->load(NULL)
#2 [internal function]: Drupal\node\NodeViewBuilder::renderLinks(NULL, 'full', 'en', false, NULL)
#3 /var/www/html/docroot/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(101): call_user_func_array(Array, Array)
#4 /var/www/html/docroot/core/lib/Drupal/Core/Render/Renderer.php(788): Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #lazy_bu...', 'exception', 'Drupal\\Core\\Ren...')
#5 /var/www/html/docroot/core/lib/Drupal/Core/Render/Renderer.php(353): Drupal\Core\Render\Renderer->doCallback('#lazy_builder', Array, Array)
#6 /var/www/html/docroot/core/lib/Drupal/Core/Render/Rende" while reading response header from upstream, client: 172.18.0.7, server: , request: "POST /node/add/external_headline HTTP/1.1", upstream: "fastcgi://unix:/run/php-fpm.sock:", host: "prod.local.ddev.site", referrer: "https://prod.local.ddev.site/node/add/external_headline"
[07-Jun-2023 13:53:31] WARNING: [pool www] child 21176 said into stderr: "NOTICE: PHP message: AssertionError: Cannot load the "node" entity with NULL ID. in /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php on line 295 #0 /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(295): assert(false, 'Cannot load the...')"

Does anyone have insight into why a patch on a css file in ckeditor5 is causing changes in drupal/core?

In case it helps, I started with the patches listed on this thread. When I ran into problems, I removed them and used a minimal patch to only that single css file. But the outcome is the same: for some content types, after applying the patch, node saves bomb out.

I'd really love to disable that auto-grow feature, but so far this is making that impossible.

Do we have a patch working for drupal 9.5?

#66 Support 10.1.1

#66 Support 10.1.2

@sir_squall #66 works in 9.5.x

@solideogloria Thanks!

I have migrated to D10 where this issue is not there anymore, but I appreciate that you have answered ;)