Adjusting the row count in form display settings (e.g. in a content type form display) does not change the height of the text editor.

This is particularly bad for mobile users because there is no way to adjust the text editor height in touch - so mobile/touch users are forced to work in a narrow text editor no matter what, even when they have iPads etc with big resolution (my client is using iPad on the road for the next 5 months...).

CommentFileSizeAuthor
#73 interdiff-2717599-64-73.txt746 bytesshashikant_chauhan
#73 2717599-73.patch1.62 KBshashikant_chauhan
#64 interdiff-2717599-56-64.txt787 bytescriz
#64 2717599-64.patch1.67 KBcriz
#56 interdiff-2717599-51-56.txt833 byteshelenc
#56 interdiff-2717599-51-56.txt833 byteshelenc
#56 2717599-56.patch1.66 KBhelenc
#52 interdiff.txt731 byteseffulgentsia
#52 2717599-52.patch1.64 KBeffulgentsia
#51 2717599-32.patch1.63 KBWim Leers
#49 interdiff.txt1.39 KBeffulgentsia
#49 2717599-49.patch2.5 KBeffulgentsia
#40 2717599-40.patch1.97 KBthpoul
#40 interdiff-2717599-38-40.txt1.11 KBthpoul
#38 interdiff-2717599-37-38.txt813 bytesthpoul
#38 2717599-38.patch1.65 KBthpoul
#37 2717599-37.patch1.62 KBthpoul
#37 interdiff-2717599-32-37.txt791 bytesthpoul
#32 2717599-32.patch1.63 KBpashupathi nath gajawada
#28 interdiff-24-28.txt823 bytessnehi
#28 2717599-28.patch1.64 KBsnehi
#20 text-area-help-text.png9.74 KBifrik
#15 2717599-15.patch1.62 KBWim Leers
#14 text-area-rows-ckeditor.png15.66 KBifrik
#14 text-area-rows-no-editor.png13.91 KBifrik
#7 ckeditor_does_not_respect_rows-2717599-7.patch909 byteszserno
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jeff Burnz created an issue. See original summary.

Wim Leers’s picture

Assigned: Unassigned » mlewand
Issue tags: +Needs upstream bugfix

That sounds like a bug in CKEditor.

Wim Leers’s picture

Title: Long text row count setting does not work with CKEditor » CKEditor does not respect <textarea rows>
mlewand’s picture

Related issue in CKEditor's bug tracker can be found in issue #5153.

Btw as for Drupal tracker isn't this bug a dup of https://www.drupal.org/node/507696 or https://www.drupal.org/node/2410565?

Wim Leers’s picture

Title: CKEditor does not respect <textarea rows> » [upstream] CKEditor does not respect <textarea rows>
Status: Active » Postponed
Related issues: +#507696: Allow individual width/height per field, +#2410565: CKEditor height does not reflect the rows attribute

Reflecting that this is blocked on upstream.

#4: even if this is the same problem: this is Drupal 8 core, not the Drupal 7 Wysiwyg module. But I've added those as related issues. Thanks!

ifrik’s picture

Issue tags: +Usability

Is there anyway of solving this for Drupal? Given that the upstream CKEditor issue is about 6 years old?

This is a real UX problem because in the Manage Form Display it's possible to create a visual hierarchy with some fields have more rows then others.
This is displayed well when the text format does not use a text editor (such as the Restricted HTML format in Core) but all fields are displayed with the same 5 rows, when the text format uses CKEditor.
It also leaves sitebuilders wondering why they can set the number of rows, when it has no effect.

It's possible to override that by CSS, but fixing that manually for every single field that should have a number of rows is obviously not a good UX for site builders.

zserno’s picture

Status: Postponed » Needs work
Issue tags: +DevDaysMilan
FileSize
909 bytes

A pretty simple workaround could be if we would compute and pass the height of the textarea as a CKEditor config property in each field's editor attach callback method. Attached a draft of this idea.

Wim Leers’s picture

#7: nice hack :) You can do that in a theme. But it's not acceptable in Drupal core, because you make a lot of assumptions: line height to be 1.5em, a certain vertical margin, etc. That is a bit too simplistic, because depending on the theme (grep the codebase for ckeditor_stylesheets to know what I'm referring to), there may be completely different margins and line heights, or even just font sizes.

zserno’s picture

@Wim Leers You are totally right, I guess that is the reason why the original issue hasn't progressed in 6 years.
I can see one way to come around this: set the editor's height after it is attached to the textarea. That way we can measure line-height and margins. Afterall it won't always be precise (because custom CSS styles can do wild things in the editor) but I think that would already be a huge help for site builders to see that updating the row number is effecting the height of the editor.

Wim Leers’s picture

I guess that is the reason why the original issue hasn't progressed in 6 years.

Yeah, while I wrote #8, I was thinking the exact same thing… Depressing, isn't it? :(

At the same time, rows only really makes sense in a monospace/coding world, doesn't it? Because what if some of your rows are <h1>s, and others are <p>s? What would then even make sense?

Have you seen #2239419: Include CKEditor's AutoGrow plug-in? Wouldn't that already adequately address this issue?

ifrik’s picture

Even if the rows don't quite correlate with the exact number of rows depending on whether they are <h1> or <p>, at least the number of rows allows site builders to visually structure a form. This won't be the exact number of lines that the user fills in but gives users an indication of what kind of length is expected.

The autogrow functionality is a good usability improvement for the user who creates content later, but it does not solve the site builders issue of managing the form display.

zserno’s picture

Status: Needs work » Closed (duplicate)
Related issues: +#2729271: Change the default height

#10 Absolutely agreed, good point! This issue is actually a duplicate of #2729271: Change the default height which has become a duplicate of #2239419: Include CKEditor's AutoGrow plug-in.
So closing this one as a druplicate. :)

Jeff Burnz’s picture

Autogrow sounds great, thanks for closing this and pointing to the new issue.

ifrik’s picture

Issue summary: View changes
Status: Closed (duplicate) » Active
FileSize
13.91 KB
15.66 KB

Sorry, but Autogrow provides a different functionality, and it does not solve the issue at hand.

On the Manage form display, the number of rows is a required field. If a format with no text editor is chosen, that configuration is respected. If a format is chosen that uses CKEditor, it's broken.

If this can't get fixed, then the Manage Form Display needs to make clear that the display of a text area depends on which Text format/editor is used.

Autogrow is great for the user that has started adding content, but it is no solution for managing forms.
No Editor:

CKEditor:

Wim Leers’s picture

Title: [upstream] CKEditor does not respect <textarea rows> » CKEditor does not respect <textarea rows>
Issue tags: -Needs upstream bugfix
FileSize
1.62 KB

If this can't get fixed, then the Manage Form Display needs to make clear that the display of a text area depends on which Text format/editor is used.

+1

This is exactly the problem.

For \Drupal\Core\Field\Plugin\Field\FieldWidget\StringTextareaWidget, entering a number of rows is fine.

For \Drupal\text\Plugin\Field\FieldWidget\TextareaWidget, it is not. Because there, you can specify a text format, and the text format may mean that you may use a text editor (when using the editor module or not), which may in turn mean that the number of rows configured there is irrelevant. So, we need to update their descriptions to correctly inform the user.

Attached patch fixes that.

Wim Leers’s picture

Assigned: mlewand » Unassigned
Wim Leers’s picture

Title: CKEditor does not respect <textarea rows> » Text editors do not respect <textarea rows>: make that clear in the formatted text field widgets
Component: ckeditor.module » editor.module
Category: Bug report » Task
Priority: Normal » Minor

These changes should actually live in editor module, but:

  1. there's no sane hook_form_alter() that we can write to do this. That's why it lives in text module.
  2. people could be integrating text editors directly, without using the default text editor integration that D8 provides. This help text would then still be relevant. That's why I think it's acceptable for this to live in text module.
Wim Leers’s picture

Status: Active » Needs review

Such noob!

ifrik’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, this additional text means that site builders don't have to wonder any more what they might have done wrong.

ifrik’s picture

Issue summary: View changes
FileSize
9.74 KB

Screenshot for the review:

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs usability review

Let's get a usability review of the new text. My gut reaction is "That sounds like a problem. What do I do about it?" I think the answer is "nothing, you're screwed." :) But... dunno, feels it's missing something.

webchick’s picture

Like, would a better approach be to check to see if text editors are enabled, and if so just grey out this option?

Wim Leers’s picture

Like, would a better approach be to check to see if text editors are enabled, and if so just grey out this option?

We can't do that. Because a formatted text field (a field with value and format properties) allows the end user to select a Text Format, and it depends on the Text Format which Text Editor will be used, or even if any will be used. Some Text Editors — like the one in use here on Drupal.org, do still show a <textarea>.

So:

  1. end user can select Text Format
  2. Text Format determines Text Editor, if any
  3. only some Text Editors hide the original <textarea>

Hence this help text is the best we can do.

Bojhan’s picture

Issue tags: -Needs usability review

The text is not very optimal. It starts with a condition (we should try to avoid), and ends with "ignored" - which makes it quite complex to understand.

Can't we just say that "A text editor might choose a different number of rows to display optimally" or something like that? Talking about the effect, not too much the cause or technical implication.

Wim Leers’s picture

Issue tags: +php-novice, +Novice

#24: works for me.

ifrik’s picture

#24 also works for me.

As for

"That sounds like a problem. What do I do about it?" I think the answer is "nothing, you're screwed."

: At least it aknowledges that there is a technical problem, and that it's not the sitebuilder doing anything wrongly - which stops them going round in circles wondering whether they had actually set this rows, or whether they did at some time change something in the text editor configuration, or, or....

ifrik’s picture

Status: Needs review » Needs work
snehi’s picture

Status: Needs work » Needs review
FileSize
1.64 KB
823 bytes

Done as suggested in #24
Please review.

yoroy’s picture

Status: Needs review » Needs work

Thanks @snehi. I think we can do without the "optimally" word here, lets remove it. All text areas with the CKEditor show with the same height, this is far from optimal. Otherwise, this text looks ok (while still leaving us with an annoying state of affairs).

ifrik’s picture

Regarding the patch in #7 and the comment in #8:

If the approach would work but can't be done in core because it would need to take theme specific settings into account, could we then have a patch like this for the Seven admin theme?

Then at least then admin theme that is available in Drupal core would work somewhat correctly.

Wim Leers’s picture

Issue tags: +Needs reroll

This issue needs just one tiny thing: removing the word optimally in #29.

pashupathi nath gajawada’s picture

Status: Needs work » Needs review
FileSize
1.63 KB

Hi @Roy Scholten, @Wim Leers,

I have removed the word "optimally" as suggested in #29,#31 from the latest patch .
Please find the patch attached.

Thanks,
,

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Thanks! Could you please provide an interdiff next time? See https://www.drupal.org/documentation/git/interdiff.

I compared manually this time, and it looks great.

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Reviewed this with @xjm and @effulgentsia. We were concerned that:

'A text editor might choose a different number of rows to display.'

...might make it sound like a human editor of text is choosing something, rather than this setting being overridden by code.

So what about something more like (just a suggestion, feel free to iterate to find something better):

'Text editors might display a different number of rows.'

By linking to the Editor module help page, people can get more information on what that means.

Jeff Burnz’s picture

@zserno #7 thanks for the trick, I used this in my editor skin, cheers!

var height = document.body.querySelector('[data-editor-active-text-format]').getAttribute("rows");
height = (height * 1.5 + 1);
CKEDITOR.config.height = height + 'rem';
Wim Leers’s picture

Status: Needs review » Needs work

Let's do #34.

thpoul’s picture

Status: Needs work » Needs review
FileSize
791 bytes
1.62 KB
thpoul’s picture

I now see what @webchick actually meant. Sorry for the noise. Hope the link is right.

Wim Leers’s picture

Status: Needs review » Needs work

You can't hardcode that URL. Use :url.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.11 KB
1.97 KB

Oops!

Edit: Wrong interdiff posted, it's actually 32-40

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Yep, much better indeed!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 40: 2717599-40.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Reviewed & tested by the community

Random fail in ConfigImportAllTest.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 40: 2717599-40.patch, failed testing.

Wim Leers’s picture

Now random fails in ManageDisplayTest , NodeTypeInitialLanguageTest , DateTimeTest . With one very interesting exception in all of them:

[26-Jul-2016 01:04:15 Australia/Sydney] Uncaught PHP Exception Symfony\Component\Routing\Exception\RouteNotFoundException: "Route "help.page" does not exist." at /var/www/html/core/lib/Drupal/Core/Routing/RouteProvider.php line 187

Hm…

The last submitted patch, 40: 2717599-40.patch, failed testing.

Wim Leers’s picture

+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php
@@ -23,6 +24,15 @@ class TextareaWidget extends StringTextareaWidget {
+    $element['rows']['#description'] = $this->t('<a href=":url">Text editors</a> might display a different number of rows.', [':url' => Url::fromRoute('help.page', ['name' => 'editor'])->toString()]);

Oh hah, this is linking to the help.page route, even if it doesn't exist…

Basically, what @webchick asked for in #34 is impossible. It's only possible conditionally:

if ($this->moduleHandler->exists('help')) {
  $element['rows']['#description'] = $this->t('Text editors might display a different number of rows.');
}
else {
  $element['rows']['#description'] = $this->t('<a href=":url">Text editors</a> might display a different number of rows.', [':url' => Url::fromRoute('help.page', ['name' => 'editor'])->toString()]);
}

… I'm not sure whether that is something we actually want. Pinging @webchick.

effulgentsia’s picture

In ResponsiveImageStyleForm::form(), there's:

if (\Drupal::moduleHandler()->moduleExists('help')) {
  $description = $this->t('See the <a href=":responsive_image_help">Responsive Image help page</a> for information on the sizes attribute.', array(':responsive_image_help' => \Drupal::url('help.page', array('name' => 'responsive_image'))));
}
else {
  $description = $this->t('Enable the Help module for more information on the sizes attribute.');
}

But not much other precedent in HEAD for different help text based on whether help.module is enabled. I think #47 does make sense though.

Note that per above example, something more descriptive than :url would be nice. Perhaps :text_editor_help?

Pinging @webchick.

Yeah, since there's not much clear precedent for #description that varies by whether help.module is installed, probably a good idea to get her feedback, so tagging accordingly. Especially since it will mean 2 very similar strings (differing only by the presence of an <a> tag) needing to be translated by translators. If we introduce this type of same text, but linked or not based on help.module installation, I could imagine us wanting to use the same pattern elsewhere.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
2.5 KB
1.39 KB

Furthermore, editor.module might also not be enabled. Here's a patch that implements #47.

Especially since it will mean 2 very similar strings (differing only by the presence of an <a> tag) needing to be translated by translators.

In this patch, that text is now different, because I added an additional sentence in the case of help.module not being enabled.

webchick’s picture

Oh dear. Yeah, this is probably not worth all the hassle. @xjm's concern was that "text editors" might sound like "an editor (person) of text" to the non-initiated. Then again, this is a user 1-ish admin page, so not sure we need to be all that concerned about it. I suggested putting "WYSIWYG" in there somewhere but was resoundly shot down. :P But maybe something on that order?

Anyway, this doesn't need to be held up on me, nor does it need product manager review. It's a simple string of text.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.63 KB

Okay, so now what? We just go back to #32 AFAICT. Reuploading #32 and re-RTBC'ing.

(Like #50 says, that should be fine, since it's for user-1-ish users anyway.)

effulgentsia’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.64 KB
731 bytes

@xjm's concern was that "text editors" might sound like "an editor (person) of text" to the non-initiated

Per #34, it's not just the phrase "text editor" that is semi-ambiguous as to whether it refers to software or to a person, but also the word "choose" that furthers that ambiguity. So here's wording that's closer to #37 that I think is clearer that this isn't referring to a person making a choice.

Additionally, when I Google "text editor", Google returns this definition ahead of the list of website matches: "a system or program that allows a user to edit text." So I think we're using as good a terminology as we can here, even if people who are confused might not think to do that search. Linking to a help page was a good idea, but as #49 shows, too cumbersome.

I'm still not entirely +1 to this patch though. It seems to me there are 3 distinct reasons why a text editor might ignore this setting:

  1. A bug in the text editor or our integration code (e.g., see #4)
  2. The use of an autogrow plugin, but in this case, the Rows setting is still respected initially, before there's too much text.
  3. Different font sizes in use.

But this patch provides no information about these 3 possibilities, and therefore, how helpful is it really?

xjm’s picture

By the way, we do use conditionals for help page links based on whether or not the help page is available elsewhere in core, based on a permission, whether a module is enabled, etc. So that, in itself, is not too cumbersome, if it would help the user understand. And if they are configuring this setting, they likely also have access to that help page, so it is the 80% case.

I think the current string in #52 is definitely better, because it does not personify the text editor as much. However, the word "ignore" still has emotional connotations. How about "override"?

(@effulgentsia's other points are also worth discussing; just wanted to add this.)

Status: Needs review » Needs work

The last submitted patch, 52: 2717599-52.patch, failed testing.

helenc’s picture

First of all hello - I found this issue via issues tagged as 'novice'. I hope I am not sticking my oar in too much here.

1) I agree that 'text editor' as used in the wording 'A text editor might choose a different number of rows to display' sounds like a person. But if the link doesn't work as webchick suggested, we just need to go back to making clear that 'text editor' refers to a text format you can select, e.g.

"Note that this setting may be ignored if your chosen text format for the field uses a wysiwyg text editor'.

2) Why is the number of rows a required field at all: if it is sometimes ignored, it would make sense to make this optional.

3) Finally, if the number of rows is to remain a required field, I would suggest upping the default value from '3' for the body field - 3 is very short.
Edit: I see 3 is for the 'summary' part of the body - the main body area has 9.

I will attempt to make a patches for some of the above points.

helenc’s picture

Patch for item 1 in comment #55 attached. I removed the words "Note that" to make it slightly less wordy.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Manjit.Singh’s picture

Status: Needs work » Needs review

triggering test bot.

Status: Needs review » Needs work

The last submitted patch, 56: 2717599-56.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review

Java errors, not test failures:

00:00:00.304 ERROR: Build step failed with exception
00:00:00.304 java.lang.IllegalStateException: Invalid object ID 64 iota=65
00:00:00.308 	at hudson.remoting.ExportTable.diagnoseInvalidId(ExportTable.java:386)
00:00:00.309 	at hudson.remoting.ExportTable.get(ExportTable.java:330)
 

So, retesting.

Wim Leers’s picture

Status: Needs review » Needs work
+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php
@@ -23,6 +23,15 @@ class TextareaWidget extends StringTextareaWidget {
+    $element['rows']['#description'] = $this->t('This setting may be ignored if your chosen text format for the field uses a wysiwyg text editor.');

s/wysiwyg/text/

Because: at /admin/config/content/formats, we also say Text formats and editors, so we should be consistent.

Wim Leers’s picture

#2788905: "Number of rows" configuration on Page Body field not working reported exactly the same:

The default page content type has a body field that is set to "Text area with summary". If I go into "Manage form display" and click the gear icon to configure that field, I am presented with options to change "Rows" and "Summary rows". If I adjust summary rows from 3 to 1, click update, click save, then refresh the Page Edit screen I have open in another tab, I will see the summary field reduce in size to 1 row. If, however, I do the same process, but increase "Rows" from 10 to 50, then refresh my edit screen, the main Body field does not increase in size. It doesn't decrease, for that matter, if I reduce the "Rows" value to 1.

When I refresh the edit page, there is a split second where the fields show with all of their content as HTML source. After a very short time, the CKEditor snaps into place (presumably through some JS) and the WYSIWYG editors appear and the raw html source values of the fields is converted to human readable text. For that split second before the final UI snaps into place, I do see that the main body field has increased in size. For example, if I stop the page loading before the final UI snaps into place, I can see quite clearly that the body field has become bigger/smaller according to my configuration. But, if I allow the screen to load naturally, once the UI snaps into place, the main body field always reduces back to about 10 rows.

When I inspect the Summary field's text box after the final UI has loaded, I see a rows="1" attribute on the textarea element. I see a similar attribute on the main body's textarea element. However, if I edit the source in my browser inspector and bump the rows number in the summery field, I see the field get bigger; when I do the same for the main body field, nothing changes. So, is the main body field some how ignoring the "rows" attribute?

Bumping from minor to normal since it seems to be confusing more and more people.

valthebald’s picture

Issue tags: -php-novice, -Novice

Too long discussion to fit into 'Novice' category, removing Novice tags as part of Novice triage BoF Dublin 2016

criz’s picture

Status: Needs work » Needs review
FileSize
1.67 KB
787 bytes

Okay, so here is an updated version of the last patch that uses more consistent wording and makes it more clear by naming a text editor as example.

valthebald’s picture

+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php
@@ -23,6 +23,15 @@ class TextareaWidget extends StringTextareaWidget {
+    $element['rows']['#description'] = $this->t('This setting may be ignored if your chosen text format for the field uses a text editor (e.g. CKEditor).');

This could use better wording. My bet is 'This setting may be ignored if the text format chosen for the field uses WYSIWYG editor (i.e. CKEditor)' (or suggestion from #61)

criz’s picture

@valthebald As I understand #61 we should not use the term "WYSIWYG" to be consistent. It is called "Text editor". So adding your suggestion it would be "This setting may be ignored if the text format chosen for the field uses a text editor (e.g. CKEditor)."

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

#65: -1, we should not use "i.e.", because it's possible to install another text editor.

xjm’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php
@@ -23,6 +23,15 @@ class TextareaWidget extends StringTextareaWidget {
+    $element['rows']['#description'] = $this->t('This setting may be ignored if your chosen text format for the field uses a text editor (e.g. CKEditor).');

I think adding the example of CKEditor helps a lot to clarify what this is about! Good idea.

The confusion above about "i.e." versus "e.g." gets at the reasoning behind #2684683: [meta] Replace i.e. and e.g. with English words. So let's avoid it and use the English word "like".

The sentence is now harder to read due to the conditionals and passive voice, though, plus still uses the word "ignore". I'd suggest:

Text editors (like CKeditor) may override this setting.

While the information that it's specifically when the text format for the field is configured to use a WYSIWYG is more specific and useful, I don't think it's helpful enough in context to merit a long and more complex sentence with a bit of terminology overload.

Phew, usability is difficult. ;) Thanks everyone for iterating on this. I agree with making this normal priority.

xjm’s picture

Or maybe:

A configured text editor (for example, CKeditor) may override this setting.

Bojhan’s picture

I also wonder if "override" is clear in what it does?

Wim Leers’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll, +Novice, +php-novice

@xjm: it's CKEditor, not CKeditor :)

Other than that, +1 to #68. I think #68 is better than #69 for the reason given in #70.

ifrik’s picture

I'm also fine with #68.

On #70: Even if somebody wouldn't know exactly what "override" might mean when they first read it: they will have an indication what's going on when they stare at a form that doesn't look like they expected it....

shashikant_chauhan’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.62 KB
746 bytes

adding patch.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

  • xjm committed 4318d98 on 8.3.x
    Issue #2717599 by thpoul, effulgentsia, Wim Leers, helenc, criz,...

  • xjm committed fa58300 on 8.2.x
    Issue #2717599 by thpoul, effulgentsia, Wim Leers, helenc, criz,...

  • xjm committed f9b3161 on 8.2.x
    Revert "Issue #2717599 by thpoul, effulgentsia, Wim Leers, helenc, criz...
xjm’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +String change in 8.3.0

Really great collaboration, everyone.

Committed 4318d98 and pushed to 8.3.x. Thanks! NB: I accidentally pushed it to 8.2.x as well, which was an accident since this is a string addition. So that's what the revert message is about.

I love that the list of credited contributors for this issue is longer than the actual text. :)

Wim Leers’s picture

I love that the list of credited contributors for this issue is longer than the actual text. :)

:D

/me highfives everyone who helped!

Status: Fixed » Closed (fixed)

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

John Pitcairn’s picture

For anyone else stumbling across this, the hook_editor_js_settings_alter() solution (based on the text format) documented around the web no longer works in Drupal 8.3.x.

A pure css hack is possible. Set the editor height based on the textarea rows attribute for every row height you will use. If you enable the editor maximize button, override those heights when maximized:

/* Set appropriate editor heights based on the rows attribute. */
textarea[rows="3"] + .cke .cke_contents {
  height: 3rem !important;
}
textarea[rows="6"] + .cke .cke_contents {
  height: 6rem !important;
}
textarea[rows="12"] + .cke .cke_contents {
  height: 12rem !important;
}
textarea[rows="24"] + .cke .cke_contents {
  height: 24rem !important;
}

/* Allow the maximize button to still operate correctly. */
[style^="position: static"] textarea[rows] + .cke .cke_maximized {
  position: fixed !important;
  height: 100% !important;
}
[style^="position: static"] textarea[rows] + .cke .cke_contents {
  height: 100% !important;
}
geek-merlin’s picture

Reopened #2788905: "Number of rows" configuration on Page Body field not working to discuss about a real fix.
Maybe we can continue the work of #7..#10.

PhilY’s picture

Issue tags: -, -

For anyone landing here from search engines, see CKEditorHeight module.