- Status changed to Downport
almost 2 years ago 8:03am 2 February 2023 - 🇷🇴Romania claudiu.cristea Arad 🇷🇴
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 he 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. Luckily, @mherchel has already provided a patch in #91.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
OMG yes, and it hasn’t even been committed to 10.0.x either! 😳
Queued a
10.0.x
test of #83.I actually think that's because the "row" setting also never worked for CKEditor 4, so this is technically a feature … but for CKEditor 5, this helps overcome the "tiny initial size" problem, so in that sense it's a bug report.
So … let's see what core committers think 😊
- 🇺🇸United States mherchel Gainesville, FL, US
So, while i agree that this should be going in 9.5/10.0, when I talked to @lauriii, his thoughts where this is a feature request (as opposed to bug). The reasoning was that the previous help text exempted CKEditor from having this behavior.
- 🇷🇴Romania claudiu.cristea Arad 🇷🇴
@mherchel, I've backported in #101 but there's a Nightwatch failure that needs some attention. Sorry, but I've never worked w/ Nightwatch
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@mherchel thanks for confirming what I guessed in #102.
Up to committer's discretion then.
And actually, there is a string change:
+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php @@ -25,7 +25,7 @@ class TextareaWidget extends StringTextareaWidget { - $element['rows']['#description'] = $this->t('Text editors (like CKEditor) may override this setting.'); + $element['rows']['#description'] = $this->t('Text editors may override this setting.');
This was just omitted from @mherchel's 9.5 backport in #91, which led me to the wrong conclusion. And that is actually a reason that makes backporting this even less likely… 😅
- 🇮🇳India vikas shishodia
@claudiucristea I applied patch #101 with Drupal 9.5.7, ckeditor5 and it worked fine. Only there is one confusion, after applying this patch if we have set row as 5 in manage form display for the field then it is only showing as 2 on editor.
If we set 6 it is showing 2 rows.
If we set 7 it is showing 3 rows.
If we set 8 it is showing 3 rows.
If we set 9 it is showing 4 rows.
If we set 10 it is showing 4 rows.
and so on....So is there any reason behind this or this is a bug?
- last update
over 1 year ago Patch Failed to Apply - Status changed to Fixed
over 1 year ago 9:50am 26 May 2023 - 🇬🇧United Kingdom catch
We're approaching the 10.1.0 release candidate and there's a fair amount of logic here plus a string change, so I think we should leave this fixed in 10.1.0. It is useful to have the backport patch from #108 for people that want to run it on 10.0.x
Automatically closed - issue fixed for 2 weeks with no activity.
- Status changed to Fixed
over 1 year ago 2:34pm 17 August 2023 I can confirm that the patch at #101 is working correctly for me on Drupal 9.5.10, without the issues mentioned in #106 - thanks for that, and see below when Body field set to 5 rows, for an example:
- 🇦🇺Australia genebobmiller
In a recent 10.1.2 install I am seeing the field widget row settings respected on the Body field but not the Summary field.
Is that expected? - 🇮🇹Italy puidu
Patch at #101 works fine but only if the element is visible in the viewport, otherwise the calculated height is not correct.
It seems that the cause is that the clientHeight method applied to the 'p' html tag element created to calculate the height of the text area. If the textarea is not in the viewport, the calculated height for 'p' will be 0.
Pretty sure there is a much better solution (reason why I'm not posting a patch), but for now I solved replacing
var height = element.clientHeight
with
var height = element.clientHeight > 0 ? element.clientHeight : 16;
into core/modules/ckeditor5/js/ckeditor5.js - 🇺🇸United States edmund.dunn Olympia, WA
I went ahead and created a patch from the work @puidu did in #113. That worked for us; we have several instances where the initial render of one of the ckeditor5 instances was out of the viewport, and the height when it was revealed was set to 65px. Changing it to 24 gets us close to the 180pc height all of our other ckeditor instances have.
- 🇩🇪Germany Istari
Can confirm that patch #101 works for D9.5.11. :)
Thx for your work :) - 🇺🇸United States ccarnnia
Hi All,
patch #114 worked for D: 10.3.5 using `adminiml` admin theme. - 🇧🇪Belgium kevinvb
Could this be reopended as it is still an issue?
If you have a CKeditor widget within a tab field group it will be shown with a height of 0 because it isn't displayed at the moment ckeditor5.js is calculating the height.
If you apply the changes from #117 it does give the widget a proper height.So I guess this needs more work?
Could this be reopended as it is still an issue?
This issue has been closed for almost a year and a half. The best way forward is likely to create a follow-up/child issue noting that the height calculation is incorrect for CKEditor instances not in viewport on page load.