Problem/Motivation

Right now in Drupal 8, if you go to node/add/article, type the highlights of your article in a few words, click the "Image" button, upload an image, hit "Save", you get this:

So we have a great UX for everything up to the point and including uploading & inserting an image. But once it's inserted, it's… awkward.

(The same problem exists when you have finished writing your article — even if it's text-only — you have to manually resize CKEditor.)

Proposed resolution

Use CKEditor's (GPL!) AutoGrow plug-in. Then it would have looked like this instead:

A demo of the AutoGrow plug-in is available online: http://ckeditor.com/demo#auto-grow

Remaining tasks

  1. Build consensus that this is desirable.
  2. Wait for #2039163: Update CKEditor library to 4.4.
  3. Wait for CKEditor to add support for "smart" autogrowing: https://dev.ckeditor.com/ticket/12120
  4. Create a new build that includes it.
  5. Accessibility review: See testing of the zoom behavior in #122. Might be good to get accessibility signoff on the feature or see if any other accessibility testing is needed.
  6. Usability review: the usability team already agreed on this feature, in general, being a UX improvement.
  7. Add screenshots to the summary comparing autogrow and resize.
  8. Test thoroughly on mobile and post screenshots (including uploading a large image and posting long text, watching the autogrow, saving the node, and revisiting the node.
  9. Decide whether to use the autoGrow_onStartup to make the behavior more consistent on node load (see #98, #115, #118.
  10. Decide whether the loss of resize and subsequent regression on large screens (#114) is an acceptable tradeoff for the usability improvement in the 80% case (usability team signoff). See this screenshot for the unused screen space without resize.
  11. Decide whether to expose configuration options in the UI (this could also be a followup).

User interface changes

None, other than that the CKEditor iframe instances will grow along automatically.

API changes

None.

Files: 

Comments

Wim Leers’s picture

Status: Postponed » Active
Bojhan’s picture

This sounds like a really good idea. While images is a large usecase, I imagine a even bigger usecase is all the content writers who paste in their long text :) - they will love this feature.

I'd love to see a patch!

Wim Leers’s picture

Status: Active » Needs review
FileSize
358.55 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,578 pass(es). View

This was even simpler than I thought! :)

Yay for CKEditor's build system!

Wim Leers’s picture

FileSize
360.02 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,575 pass(es). View
1.54 KB

Bojhan noticed that the autogrowing was happening too late: only after giving focus to CKEditor, which led to a jarring experience. Fortunately, there's a setting for doing the growing on startup instead :)

Bojhan’s picture

This seems to work quite well now, I've tested the text in various ways and it expands as expected. I would RTBC this, but I am sure it could benefit from another pair of eyes on the code.

Wim Leers’s picture

What code? There is no code to speak of :)

We're just including another CKEditor plugin in our default build. I only changed configuration, no code was written! :)

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Tested, such a great improvement. Well done.

sun’s picture

Hm. The autogrow pattern is not always a desired/expected behavior in my experience.

Somehow I expected this patch to just expose an option in the editor UI...?

I'm also surprised that we need to change so much (precompiled) code in order to include a "new" default plugin that's supposed to be there by default. Can't we include and make all of the default plugins available in core, and leave the aggregation to the aggregation?

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Those seem like good questions to sort out.

nod_’s picture

Auto growing should be restricted to viewport size, having to scroll the page to see all of your content should only be possible with quick edit.

By scrolling inside the ckeditor area you should be able to see all of you content, without scrolling the page. It's such a pain when that happens because the toolbar doesn't scroll with the page.

As far as sun comment I'm pretty sure we're in a jquery-like situation. We can have custom builds but they're not dynamic.

Agreed with option to enable/diable autogrow.

Bojhan’s picture

Interesting. There are indeed usecases where auto-grow makes less sense, such as the mobile viewport.

I agree that we should make this an option, but I do think that we can have it on by default. There are usecases where this makes less sense, but I think for the majority of common content editing and writing usecases this change will be beneficial. Perhaps we can manage the usecases where it makes less sense, such as mobile?

hass’s picture

We should also not forget that node edit form may use a different theme and width than the final public node... Not sure how this affects ckeditor here, but just had this idea.

LewisNyman’s picture

Issue tags: +frontend
Wim Leers’s picture

#8:
We could make it an option. But like Bojhan, I think this should be the default.

I'm also surprised that we need to change so much (precompiled) code in order to include a "new" default plugin that's supposed to be there by default. Can't
we include and make all of the default plugins available in core, and leave the aggregation to the aggregation?

This is for a few reasons:

  1. Drupal core still doesn't support JS minification. In the case of CKEditor, shipping with the unminified source is… pretty much performance suicide.
  2. The assumption that we've been working with for years now, is that the majority of Drupal users will not see the need to make modifications to the default WYSIWYG editing experience. That also means not installing additional plugins. In that case, it make sense to download CKEditor with a single HTTP request, hence a single JS file, hence the update to this indeed enormous file.

I'm working on making JS minification in Drupal 8 core a reality, but then I need reviews on the blocking issues, which are #2276219: Asset libraries should declare their license and #2258313: Implement JS Web License Labels to allow for JS minification. Once that's in, we can reconsider our approach of shipping CKEditor.


#10 and #11 make good points:

Auto growing should be restricted to viewport size, having to scroll the page to see all of your content should only be possible with quick edit.

By scrolling inside the ckeditor area you should be able to see all of you content, without scrolling the page. It's such a pain when that happens because the toolbar doesn't scroll with the page.

Perhaps we can manage the usecases where it makes less sense, such as mobile?

I propose that we use the config.autoGrow_maxHeight setting, and that we that in Drupal.editors.ckeditor.attach based on window.innerHeight and the vertical offsets (Drupal.displace.offsets.(top|bottom) — the top offset is set by the D8 toolbar, but could be set by an alternative toolbar also, or anything else).

We could then formulate a default formula for the maximum autogrow height. E.g.:

0.8 * (window.innerHeight - Drupal.displace.offsets.top - Drupal.displace.offsets.bottom)

(That'd be 80% of the effectively available vertical screen estate.)

Thoughts?

hass’s picture

I do not think that #2258313: Implement JS Web License Labels to allow for JS minification or #2276219: Asset libraries should declare their license is a blocker for JS minification. We also do not care about CSS framework licenses for years and just remove all comments (including the license comments).

Wim Leers’s picture

#15: I'm afraid license handling is a JS minification blocker — the community agreed on that with many people, a long, long time ago. If you disagree with that, then comment on the relevant JS minification issue(s), not here, and not on either of those issues. It's also off-topic here.

Bojhan’s picture

I think the idea of adjusting it to window.innerheight makes sense, but I think this needs a patch to truly evaluate this suggestion. Because it might come down to fine tuning that percentage.

oleq’s picture

We could then formulate a default formula for the maximum autogrow height. E.g.:

0.8 * (window.innerHeight - Drupal.displace.offsets.top - Drupal.displace.offsets.bottom)

(That'd be 80% of the effectively available vertical screen estate.)
Thoughts?

What if the size of the window changes?

In my opinion config.autoGrow_maxHeight should be controlled dynamically, relative to the space around the editor. At the moment it is possible to override editor height with editor#autoGrow event, but it's buggy. I submitted some ideas to CKEditor bug tracker.

Wim Leers’s picture

Issue summary: View changes
Status: Needs review » Postponed

#18: you're right. What you're describing sounds ideal.

I'm marking this as postponed on https://dev.ckeditor.com/ticket/12120 — it doesn't make sense to add a half-baked solution, if all CKEditor users can benefit from an upstream solution.

OFF’s picture

Well, we can't use this plugin?

Wim Leers’s picture

Version: 8.0.x-dev » 8.1.x-dev
Wim Leers’s picture

Category: Task » Feature request
Issue tags: +Needs upstream feature

Pinged the CKEditor team about https://dev.ckeditor.com/ticket/12120.

Wim Leers’s picture

Status: Postponed » Active
Issue tags: -Needs upstream feature

See http://dev.ckeditor.com/ticket/12120#comment:4, until this is explicitly supported in the autogrow plugin, we can use the work-around provided there. We can just open an issue to remove that hack in the future, when we update to CKEditor 4.6.0.

Let's do this!

thpoul’s picture

Let's try and move this forward. In this patch I have simply updated the CKEditor 4.5.5 to include the autogrow plugin (haven't changed anything mentioned at #23 and I am having an issue already.

When I focus the editor and start typing it continuously gains height with each key I press. Attaching the patch and a gif showing the strange behavior.

Autogrow

Wim Leers’s picture

Wow, WTF!

I'll look into this.

Wim Leers’s picture

I cannot reproduce the behavior in #24 using Chrome 47, Firefox 44 or Safari 9…

  1. What browser are you using?
  2. Try disabling all browser extensions you have installed.
thpoul’s picture

I'm still getting the same behavior on Incognito Chrome 48 without plugins, Safari 9 and Firefox 43. Have you applied #24 and cleared caches? I also have js and css aggregation off.

yoroy’s picture

I'm seeing it in Firefox on simplytest. Even clicking inside the text area to give it focus already starts to expand the box. Even backspaces and pressing only modifier keys make the box expand.

Wim Leers’s picture

How is it even possible we're seeing such vastly different behaviors?! Pinging the CKEditor folks, because I can't make sense of this.

mlewand’s picture

@strong>@w m-leers you've been unable to repro this with applying #24's patch - right? And what is the purpose of this patch? I see minified code and license updates, so I assume it's a version update, but that's just a guess.

thpoul’s picture

@wim leers crossing fingers I didn't mess up the build.sh procedure

@mlewand the updates you see are just 2015 to 2016 copyright updates plus the addition of the autogrow plugin.

Wim Leers’s picture

Assigned: Wim Leers » Unassigned

#30: Yes, with patch #24. Try it on simplytest.me using this link: https://simplytest.me/project/drupal/8.1.x?patch[]=https://www.drupal.or... (which I also gave you in chat).

And as #31 says, this is simply adding the autogrow CKEditor plugin to Drupal's build of CKEditor.

mlewand’s picture

Assigned: Unassigned » mlewand

Let me check it

thpoul’s picture

Problem seems to be here at refreshCache(); where the markerContainer is set: markerContainer = doc[ CKEDITOR.env.ie ? 'getBody' : 'getDocumentElement' ]();

Using markerContainer = doc.getBody(); seems to help solve the issue.

mlewand’s picture

Assigned: mlewand » Unassigned

Well the trouble maker are these styles:

autogrow inserts an element after the body to get the height of the content. Doing so by appending block element to the body element seems to be fine, however all-non ie uses documentElement for that (looks like a hack, but don’t know the reason yet). It's a risky change.

@thpoul you're right, it seems to work for this case, however while it fixed this case I was able to break the scrolling by mashing keybaord on Firefox. (you the content has a scroll bar, but end user can't see it since autogrow sets overflow-y to hidden)

scrolled editor area

The quickest way here seems to fix the CSS so that it doesn't style CKEditor's html element.

mlewand’s picture

Assigned: Unassigned » mlewand
Issue summary: View changes
FileSize
48.03 KB
mlewand’s picture

Issue summary: View changes
thpoul’s picture

@mlewand thank you so much for looking into it!

@Wim Leers should we open a child issue for the css change or handle it here in the same issue?

Wim Leers’s picture

Assigned: mlewand » emma.maria
Issue tags: +Bartik

The two CSS styles that @mlewand pointed to are:

html {
  height: 100%;
}
body {
  min-height: 100%;
  …
}

And the reason those two particular styles are loaded in CKEditor's iframe:

ckeditor_stylesheets:
  - css/base/elements.css

So, to fix this, we'd need Bartik's element.css to be split up. Or come up with some alternative solution. Assigning to @emma.maria for feedback, she's the Bartik maintainer.

darketaine’s picture

Actually it would just need

html {
  height: auto;
}

If we can override that (for the ckeditor only) it's ok

Wim Leers’s picture

I wonder if CKEditor shouldn't just always do #40, or at least let the autogrow plugin always do that? Clearly that's necessary for robustness…

darketaine’s picture

I think it would need to have a file for drupal where it could "fix" all these things. Like an 'extension' to its theme. It would be a solution to many things I guess like #2090937: Seven theme: style CKEditor-native dialogs to match Drupal-native dialogs

Wim Leers’s picture

#42: no, because this is generic/theme-agnostic. #2090937 is specific to each theme. We could solve it in Drupal, but it should be solved in CKEditor.

thpoul’s picture

#43 So I guess this is postponed until it's resolved in CKEditor?

mlewand’s picture

We can't blame editor for this, in this case that's just wrong CSS applied to the editor. contentsCss should be used solely for content styling. Using it for html / body element styling just breaks the editor.

Classic (framed) editor can safely assume that it's elements are not styled in such way, that's the reason why it's sandboxed in an iframe.

Wim Leers’s picture

The documentation you linked to says:

The CSS file(s) to be used to apply style to editor content. It should reflect the CSS used in the target pages where the content is to be displayed

That's exactly what Drupal does.

We can't expect every theme to be aware of the fact that CKEditor will break if they have certain CSS declarations for html or body.

Wim Leers’s picture

Version: 8.1.x-dev » 8.2.x-dev
thpoul’s picture

Status: Active » Needs review
FileSize
427.54 KB

Rerolled the patch for 8.2.x (now having CKEditor 4.5.7) so it doesn't remain in the backlog just for that. Problem at #24 is still here.

I wish we could move this forward, therefore changing status to NR to continue the discussion.

Wim Leers’s picture

Assigned: emma.maria » mlewand
Issue tags: -Bartik +Needs upstream bugfix

The build-config.js changes are missing.

Assigning to @mlewand because IMO this needs to be fixed upstream. Since this is too late for 8.1 anyway, let's aim to get this in 8.2 and get it fixed in CKEditor 4.5.8 or 4.5.9.

Wim Leers’s picture

Title: Include CKEditor's AutoGrow plug-in » [upstream] Include CKEditor's AutoGrow plug-in
Status: Needs review » Postponed
Wim Leers’s picture

Status: Postponed » Active

8.1 is out. Let's do this.

thpoul’s picture

Rerolled for 8.2.x. Same problem.

Wim Leers’s picture

Yep. I pinged the CKEditor team about that. They know it's blocked on them.

mlewand’s picture

Well, all right. We'll provide a hotfix for this issue in our dedicated ticket: #14620 - stay tuned.

Wim Leers’s picture

Status: Active » Postponed

#52 means we have a patch that is ready. This is still blocked on upstream, so reflecting that.

thpoul’s picture

FileSize
157.61 KB

This fixes the issue.

This is now blocked from the CKEditor 4.5.9 release which includes the above fix.

Autogrow

Wim Leers’s picture

Awesome :) Now we just wait for the CKEditor 4.5.9 release!

Wim Leers’s picture

Title: [upstream] Include CKEditor's AutoGrow plug-in » [PP-1] Include CKEditor's AutoGrow plug-in
Issue tags: -Needs upstream bugfix

Reflecting that we are no longer blocked on an upstream bugfix, but just need to update to CKEditor to version 4.5.9 first.

mlewand’s picture

Assigned: mlewand » Wim Leers
Status: Postponed » Needs review

CKEditor 4.5.9 version is out. Please confirm if it fixes the problem for you.

The last submitted patch, 24: 2239419-24.patch, failed testing.

thpoul’s picture

Status: Needs review » Postponed
Related issues: -#2039163: Update CKEditor library to 4.4 +#2724225: Update CKEditor library to 4.5.9

Thank you mlewand!! Reflecting that this is blocked from #2724225: Update CKEditor library to 4.5.9

Status: Postponed » Needs work

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

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Status: Needs work » Postponed

Thanks, @mlewand!

Wim Leers’s picture

Wim Leers’s picture

Title: [PP-1] Include CKEditor's AutoGrow plug-in » Include CKEditor's AutoGrow plug-in
Status: Postponed » Needs work

#2724225: Update CKEditor library to 4.5.9 landed, this is now no longer blocked! Let's drive this home.

Patch needs a reroll because Drupal 8 now has CKEditor 4.5.9.

thpoul’s picture

Assigned: Unassigned » thpoul

On it.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.17 MB

Here it is :)

thpoul’s picture

Assigned: thpoul » Unassigned

Oops sorry for the noise.

Wim Leers’s picture

Status: Needs review » Needs work
  1. So, between #24 and #58 this got sidetracked by a bug in CKEditor. That was fixed in CKEditor 4.5.9.
  2. The patch at #67 does nothing else besides adding the autogrow plugin to our default build, causing it to be enabled by default.
  3. I've just done a round of manual testing, and it works great. It works both when typing (it grows as you need more vertical space), and when starting to edit existing content (it grows to accommodate the vertical space).
  4. BUT we still need to address the concerns raised. Let me repeat what I said in #14:

    #10 and #11 make good points:

    Auto growing should be restricted to viewport size, having to scroll the page to see all of your content should only be possible with quick edit.

    By scrolling inside the ckeditor area you should be able to see all of you content, without scrolling the page. It's such a pain when that happens because the toolbar doesn't scroll with the page.

    Perhaps we can manage the usecases where it makes less sense, such as mobile?

    I propose that we use the config.autoGrow_maxHeight setting, and that we that in Drupal.editors.ckeditor.attach based on window.innerHeight and the vertical offsets (Drupal.displace.offsets.(top|bottom) — the top offset is set by the D8 toolbar, but could be set by an alternative toolbar also, or anything else).

    We could then formulate a default formula for the maximum autogrow height. E.g.:

    0.8 * (window.innerHeight - Drupal.displace.offsets.top - Drupal.displace.offsets.bottom)
    

    (That'd be 80% of the effectively available vertical screen estate.)

    Thoughts?

  5. To that, a CKEditor maintainer responsed in #18 that window height might change. So we need to adjust it dynamically. For that, see #23:

    See http://dev.ckeditor.com/ticket/12120#comment:4, until this is explicitly supported in the autogrow plugin, we can use the work-around provided there. We can just open an issue to remove that hack in the future, when we update to CKEditor 4.6.0.

These last two points still need to be addressed.

Finally: note that these can explicitly not be tested using \Drupal\FunctionalJavascriptTests\JavascriptTestBase, because that uses PhantomJS, which specifically uses a particular screen resolution. IOW: window height never changes in our tests, and cannot be changed.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.77 KB
1.17 MB

Let's try and move this forward. It needs more input as I'm having problem calculating Drupal.displace correctly within Drupal.editors.ckeditor.attach and I'm almost certain that my "on window load" method is wrong.

Wim Leers’s picture

Issue tags: +JavaScript
Wim Leers’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/ckeditor/js/ckeditor.js
    @@ -85,6 +85,22 @@
    +        // @todo Remove at CKEditor version 4.6.0.
    

    This should say "Remove when http://dev.ckeditor.com/ticket/12120 is fixed." — it's not certain this will be fixed in CKEditor 4.6.0.

  2. +++ b/core/modules/ckeditor/js/ckeditor.js
    @@ -85,6 +85,22 @@
    +        editor.on('autoGrow', function( evt ) {
    

    We need this to not run in case of an inline CKEditor instance. See http://dev.ckeditor.com/ticket/12120#comment:5.

  3. +++ b/core/modules/ckeditor/js/ckeditor.js
    @@ -273,7 +289,13 @@
    +  $(window).on('load', function () {
    

    You need $(document).on('drupalViewportOffsetChange', eventData, autoResize);

  4. +++ b/core/modules/ckeditor/js/ckeditor.js
    @@ -273,7 +289,13 @@
    +    displace();
    

    And then you don't have to do this.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.97 KB
1.17 MB

Thank you Wim for the review. Here is the updated patch.

Wim Leers’s picture

Status: Needs review » Needs work
Issue tags: +Needs manual testing

Now we mainly need this patch to pass eslint without errors. And it'd be great to get manual testing from someone else than me.

I think this does not need JS tests, because 99% of the complexity here is tested by the CKEditor team upstream already.

So, after manual testing and JS code style adjustments, I think this is ready.

Thank you so much, @thpoul!

tstoeckler’s picture

FileSize
335.8 KB

I tried this out (Ubuntu 16.04 / FF 47.0 ) and noticed a bit of strange behavior. Not sure if this was covered above already, I apologize if that is the case.

The textarea does correctly grow and shrink according to the contained text, however:
During expansion I briefly see a vertical scroll-bar in the right side of the textarea. Also the text jumps up about half a line-height for a brief period of time during the expansion. Afterwards everything is fine.

I attached a small screengrab of the behavior.

Wim Leers’s picture

#75: pinging the CKEditor team for feedback regarding that, to check if it's expected behavior or not. To be thorough, could you perhaps also test http://ckeditor.com/demo#auto-grow, to check if it's something Drupal-specific?

tstoeckler’s picture

Great idea!

The "twitching" of the text itself I can actually reproduce with the demo. The vertical scrollbar, however, does not appear there, so that seems to be Drupal-specific.

tstoeckler’s picture

Ahh I'm sorry. I now tried the second demo on that page, and there I get the scrollbars as well. So I guess it's related to the max-height.

So it is indeed an upstream issue. That doesn't mean we need to block this on that, but I guess it's good to be aware of and maybe track upstream.

Wim Leers’s picture

Thanks for the testing! Pinged @mlewand with this new information. Hopefully he'll chime in soon.

Feuerwagen’s picture

I can confirm this behavior (twitching in general, vertical scrollbar with max-height set) on OSX / Chrome 51. Both in Drupal and with the CKEditor Demo.

Wim Leers’s picture

@Feuerwagen: thanks!

Tade0’s picture

I've created an issue in https://dev.ckeditor.com for that problem with the vertical bar:
https://dev.ckeditor.com/ticket/14698

EDIT: A potential solution has been implemented: https://dev.ckeditor.com/ticket/14698#comment:2 The question remaining is whether it will hold up to scrutiny.

Wim Leers’s picture

Weird, I don't know how your first comment got unpublished. I've never seen that happen before! I re-published your original comment and unpublished your "WTF my comment disappeared" comment :)

Great to see that this is being fixed! Unfortunately this means we would once again be blocked on a CKEditor bugfix release that includes that fix. Hopefully we can land this feature now in D8, and then we automatically get that upstream bugfix the next time we update CKEditor. IMO that'd be best.

tstoeckler’s picture

Yeah, I personally don't see that as a big enough issue to hold this up on that, but then I'm not a usability expert...

Wim Leers’s picture

@tstoeckler: Let's not forget this only goes into Drupal 8.2.x. 8.2.0 is going to be released ~3 months from now. That leaves 3 months for CKEditor to fix this upstream and us to update our CKEditor. Should be fine.

Thanks for sharing your thoughts!

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.85 KB
1.17 MB

And here are the fixes from the eslint :)

Wim Leers’s picture

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

This patch was manually tested by @tstoeckler in #75. He found a cosmetic issue that he confirmed (#78) also exists outside of Drupal.

I can confirm this happens in OS X in Firefox and Chrome. But it's far less obtrusive there, due to the much more subtle scrollbar that OS X uses.

The CKEditor team intends to fix (https://dev.ckeditor.com/ticket/14698) this in CKEditor 4.5.10, and has committed to fixing this before 4.6.0 for sure. 4.5.10 is going to be released on June 29. CKEditor 4.6.0 will be released on July 27. That still leaves several months until Drupal 8.2 is released. We will update CKEditor to those versions anyway, and doing so will then also fix this cosmetic issue.

In summary: I think holding this UX-enhancing feature back for a minor cosmetic problem that is being fixed upstream would be wrong.

thpoul’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

This can't go in 8.1.x whereas #2765751: Update CKEditor library to 4.5.10 can, and has. Also this issue is affected by a bug fixed in 4.5.10 so I really think that one had to go first.

Also there was discussion early in the issue about make this optional that I can't see how it was resolved.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.02 MB
1.92 KB

Here is the reroll after #2765751: Update CKEditor library to 4.5.10 commit. The 2239419-ckeditorjs.txt is just the diff without the vendor/ckeditor folder, for easier review.

tstoeckler’s picture

+        // @todo Remove when http://dev.ckeditor.com/ticket/12120 is fixed.

I think we can now say "Remove when CKEditor is upgraded to 4.6." that is clearer IMO.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Also there was discussion early in the issue about make this optional that I can't see how it was resolved.

See #11 + #13. Bojhan thinks this should be the default, and so do I. So that's what this patch does.

Rather than exposing Yet Another Checkbox and Yet Another Setting, I think it makes sense to make the decision to just enable this for everybody always. If people find it annoying, we can still choose to add a setting during the 8.2.x beta phase.


Manually tested in simplytest.me again. Still works great. Back to RTBC.

Wim Leers’s picture

#92: No, sometimes CKEditor moves a certain ticket to a different release (just like we do). It's not 100% certain that it'll be fixed in 4.6. It's 100% certain it'll be fixed once that ticket is fixed. Better to be more specific.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs manual testing
FileSize
65.72 KB
246.18 KB
566.49 KB

Hm. When I tried to manually test this on simplytest.me, it did not have the desired effect, or it did, but it was a bit finicky.

1) I uploaded http://assets.worldwildlife.org/photos/1139/images/carousel_small/elepha... on an article add form:

CKEditor image upload field

2) However, the text area did not "grow" accordingly (this is in the latest version of Chrome on OS X 51.0.2704.106):

Textarea doesn't grow

3) If I click the image, it also doesn't grow. However, if I click just to the side of the image and give the actual text area focus, then it works as intended:

Yay, growy elephant!

Also got some really funky stuff happening in OSX Safari Version 9.1.1 (11601.6.17), like no JS being loaded on the page at all, WYSIWYG editor not showing up, and the drop-button one long image. But that could've just been a simplytest.me glitch.

Additionally, when going back to edit the node, it looks like screenshot 2 again. Is that intended?

So it seems like we need to auto-focus the text field when adding a new image with no text in the box. Also, seems like when editing, it would be better to start at the "autogrow" size?

Tagging for manual testing also, to see if others can replicate the Safari weirdness.

Wim Leers’s picture

Status: Needs work » Reviewed & tested by the community

#95: I've always been testing with text (paragraphs) not enormous images. I suspect you may be triggering an edge case here. I pinged the CKEditor team about this.

I'd urge you to test again with text. It still means a significant UX improvement. Most bodies are filled with … body text. It's possible it can be improved further for images, but if this is a step forward for at least the common case, why are we pushing back against it?

RE: funky JS stuff in Safari 9.1.1 — that's definitely unrelated to this patch.

So it seems like we need to auto-focus the text field when adding a new image with no text in the box.

I don't understand what this means.

Also, seems like when editing, it would be better to start at the "autogrow" size?

I don't understand this either. How can we "start" at the "autogrow size". There's no "autogrow size". The whole point is that it grows to match the height of your content: with 5 vs 10 paragraphs, it'll look very different. The only fixed height that exists, is the maximum: we ensure that it doesn't grow to accommodate e.g. a 100-paragraph piece of content, because then it gets in the way of navigation usability (scrolling to the next field would become extraordinarily painful then).

xjm’s picture

@Wim Leers, I believe @webchick means on node edit when we talk about "starting" with the autogrow size, for a WYSIWYG field that already has a large something in it.

I tested this with text as well. When you save a node with a long something (text or image or whatever), then go back and edit it, the WYSIWYG is tiny on the new page load until it receives focus. So the reasonable (I think) questions are:

  1. Should the WYSIWYG autogrow be triggered on a new page load, or is it sufficient to do when the WYSIWYG receives focus?
  2. Even if the answer to #2 that first question is no, is there something we can do with the JS we've added to Drupal in order to make the WYSIWYG receive focus when an image is added, so that the autogrow is triggered? The fact that clicking on the image does not cause the WYSIWYG to receive focus may be a CKEditor bug, but the fact that the image editing dialog can result in it not having focus immediately afterward seems like it could be something we can fix in this patch.

The difficulty with UX improvements for dynamic interactions is that if they are (or feel to the end user) inconsistent, the improved UX can be unintentionally frustrating as well. So I think these are valid questions to ask.

xjm’s picture

Should the WYSIWYG autogrow be triggered on a new page load, or is it sufficient to do when the WYSIWYG receives focus?

Elaborating on this, once you've interacted with the WYSIWYG, it does not "auto-shrink" when it loses focus. Which is fair enough. But that also means that it feels buggy for it to be shrunken on page load but never go back to that state after it receives focus.

Wim Leers’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for the clarifications! That helps a lot. We should have answers for those.

Tade0’s picture

#95 @webchick I tried to reproduce the behaviour you described in Chrome 51@OS X in CKEditor, but failed to do so.

Could you tell which specific sandbox on simplytest.me you used to get this effect?

xjm’s picture

Could you tell which specific sandbox on simplytest.me you used to get this effect?

I'm kind of confused by this question; maybe I've misunderstood. The sandboxes go away after 30 minutes so one can't go back and visit it (but they are all built the same way so it should not matter). @webchick clicked the "simplytest.me" button Dreditor provides next to the patch.

I can also reproduce it in Chrome by :

  1. Clicking the simplytest.me button next to the latest patch.
  2. Installing that Drupal site normally.
  3. Going to "Add content" and selecting "Article".
  4. Clicking the image icon in the CKeditor toolbar.
  5. Clicking "choose file.." and select a large photo.
  6. Clicking on the alt text field and entering an alt text.
  7. Clicking the save button.

If I stop there and touch nothing else, the text field does not grow to accommodate the image. It's only when I click on the field, select another browser tab, etc. that the auto-grow is triggered. I also reproduced this with keyboard navigation for the above steps. Interestingly, it does not seem to happen for a local installation in an Incognito window using the same steps after the first.

The bit about the editor not auto-growing upon the edit form being loaded though is reproducible in both cases.

Jeff Burnz’s picture

Re #101 I can reproduce this in Opera also. Only when I edit the node does simply clicking on the image fire autogrow, when first adding the image you have to either click off or a button.

I tested this with both core default skin and my own on simplytest.me - same result.

Tade0’s picture

I followed the steps from #101(via Dreditor) in Chrome 52.0.2743/OSX and the field did grow in my case, although trying later to change the size of the editor by clicking on the image/editable area failed and I think I know why:

I noticed that both the autogrow and resize plugins are loaded, which may contribute to the problem since these two don't work together well.

Unfortunately this fact is only mentioned in the sample shipped with the autogrow plugin - I'm sorry for that. I'll see to it that it appears in the documentation.

I believe disabling the resize plugin should help in this case.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.36 MB
1.36 MB

Rerolled patch without resize plugin.

Jeff Burnz’s picture

I think it would be a huge mistake to remove resize capability, the UX is terrible. If it can't work then I suppose we need to file upstream issues or not have autogrow, or find some other solution, I don't see the removal of resize as a solution (but a major regression).

Wim Leers’s picture

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

Manually tested #104. The scenario described in #101 (particularly If I stop there and touch nothing else, the text field does not grow to accommodate the image.) is no longer reproducible, i.e. it's now fixed!


Ideally we have both active at the same time:

  1. autogrow to ensure sensible defaults
  2. resize to allow the user to customize to their liking

But for technical reasons that turns out to be impossible. We have to choose one.

  • Bojhan said:
    15:16:08 <Bojhan> I really can't choose
    15:16:19 <Bojhan> autogrow seems better to me
    
  • The CKEditor team also recommends autogrow: lead developer Piotrek said t looking strictly from user’s perspective I’d choose autogrow. Project founder FredCK said:
    That’s a case by case analysis… you guys should check if the autogrow limit makes the editor big enough so that resizing is less necessary. I believe that resizing becomes a requirement when the editor is small and writing on it gets annoying because of it (like not being able to properly see images and tables). So if the editor doesn’t stay too short with that limit, losing resizing may not be a big loss.
  • thpoul said:
    from my personal experience (working with dozens of greek news portals with drupal 7 + ckeditor), authors ​*always*​ ask for more height in their body fields (ckeditor instances). The don’t ​_manualy_​ resize it, they want to be able to just use it (most of the times paste text into it). But then again, this is just the journalist’s perspective. I image more experienced users would like to have control over the size the editor occupies on their screen.

In other words: we all fully expect that autogrow is better than resize for the majority of users.

Therefore I propose the following:

  1. We ship Drupal 8.2 with autogrow enabled by default, without an ability to go back to resize.
  2. If user feedback during beta/RC/release is negative, we add back resize, and provide an option to opt out from autogrow, so you get resize.

i.e. we try it, and we expect that it'll be positively received, but then we can still go back to give the user the option if necessary.

To make the UX better, we sometimes have to make choices. Providing yet another setting in the UI to choose either is something that we should avoid if possible. Drupal should have an opinionated, great UX. This is one of those issues where we get to do that.

tstoeckler’s picture

Providing yet another setting in the UI to choose either is something that we should avoid if possible.

Can you elaborate on that? Seems to me making that configurable in the editor settings wouldn't be too bad of an idea?

Jeff Burnz’s picture

If I add a large image I can no longer resize the editor to view my content with the image in context, I have to preview it, the resize autogrow max hight is too small, I have a huge screen resolution, but I still get this smaller than my viewport height box to mess about in. The same goes for long tables, I am now forced to scroll around in a box.

EDIT: don't get me wrong on autogrow - it's certainly awesome and I'm with Bojhan, it's a hard decision, on a balance autogrow may well be better for some users, but it's a harsh price to pay.

xjm’s picture

Status: Reviewed & tested by the community » Needs review

Unfortunately #108 sounds worse to me than not having autogrow. It sounds like it would make it totally unusable in some cases. I don't think we can ship like that, and it sounds worse than the bug Angie identified. I also don't think a UI configuration for it is a good solution, if it is choosing between these two options.

Is there not some way we can just work around the bug by triggering the autogrow with some small Drupal JS? All it has to do is put focus on the field after one of these dialog interactions and on node load, or otherwise manually kick the autogrow into action. We could ship with that, and then remove the workaround once it is fixed upstream in CKEditor.

xjm’s picture

Status: Needs review » Needs work

And actually the bug with it not autogrowing is still reproducible on simplytest.me with the current patch. So unfortunately this is not ready; the current patch introduces a significant regression without fixing the bug with autogrow either.

Wim Leers’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.38 MB

#108:

I have a huge screen resolution, but I still get this smaller than my viewport height box to mess about in.

Yes. If CKEditor (or any text editor) has a vertical height that's greater than the viewport, then that is guaranteed to have a terrible user experience. Because now you need to find space horizontally either on the left or the right of CKEditor and scroll there. Otherwise you're stuck, you have to continue to scroll within CKEditor forever. This is a problem you'd have with certain themes, but also on just about every smartphone.

It sounds like it would make it totally unusable in some cases.

Quite the opposite. It prevents unusable cases that a user can manually put themselves in, by using the functionality that resize offers. i.e. resize (in HEAD) allows user to shoot themselves in the foot.

Is there not some way we can just work around the bug by triggering the autogrow with some small Drupal JS?

The CKEditor team says no; you must choose between either of the two.

And actually the bug with it not autogrowing is still reproducible on simplytest.me with the current patch.

Attached is a screencast of it working correctly. Note that if you upload an image that is higher than CKEDITOR.config.autoGrow_maxHeight = 0.7 * (window.innerHeight - displace.offsets.top - displace.offsets.bottom);, that it indeed will not show the entire image. The whole point is that you do not end up with something that has multiple viewport heights worth of scrolling. This is one of the things that is very bad about resize, as described earlier.


Since IMO #109 misinterpreted #108, and #108 itself is misrepresenting things (it's intentional that we don't grow beyond the viewport height, because growing beyond that automatically, or resizing beyond that manually is itself hugely problematic), and this comment explains all of that, I'm moving this back to RTBC just this once, so that it is again evaluated by committers.

Jeff Burnz’s picture

Yes. If CKEditor (or any text editor) has a vertical height that's greater than the viewport

No one said anything about being higher than the viewport.

Otherwise you're stuck, you have to continue to scroll within CKEditor forever. This is a problem you'd have with certain themes, but also on just about every smartphone.

But I am not on a Smartphone Wim, and neither are the majority of actual content authors. In any case, I believe the term we should reach for here is "responsive design".

It prevents unusable cases that a user can manually put themselves in

Is that actually a problem, in the real world, if it is, it would be the first time I ever heard it being an actual "unusable case".

And I did NOT misrepresent anything at all - YOU misinterpreted and misrepresented my comment!

Wim Leers’s picture

No one said anything about being higher than the viewport.

Isn't that what this is saying?

If I add a large image I can no longer resize the editor to view my content with the image in context, I have to preview it, the autogrow max hight is too small, I have a huge screen resolution, but I still get this smaller than my viewport height box to mess about in.

(Emphasis mine.)

But I am not on a Smartphone Wim, and neither are the majority of actual content authors.

True, most content authors are not on smart phones. But my point still stands: when you have a CKEditor instance that is A) almost as high as the viewport height, B) equally high as the viewport height, or C) higher than the viewport height, then it is very difficult/annoying to scroll down to other fields, or even to the "Save" button. And that difficult/annoying situation is only possible with resize, it's impossible with autogrow.

In any case, I believe the term we should reach for here is "responsive design".

Yes! :) And that's what autogrow does: its maximum vertical height (maximum growth) is automatically limited by the current viewport dimensions. And when the viewport dimensions change, so does the maximum vertical height of CKEditor.

Is that actually a problem, in the real world, if it is, it would be the first time I ever heard it being an actual "unusable case".

I've experienced this on many sites, many times, both Drupal and non-Drupal. Where you can keep scrolling within an iframe (whether CKEditor, another text editor, or some other iframed content) for a very long time, even though you really just want to get to the thing below the iframe/CKEditor.

And I did NOT misrepresent anything at all - YOU misinterpreted and misrepresented my comment!

I'm sorry. I meant that in my opinion, it's a very bad thing to be able to have a CKEditor instance that is higher than the viewport height, but as far as I can tell you're arguing in #108 that this is an important lost feature. In my opinion, it's a win, in your opinion, it's a loss. Is that a better representation? :)

Jeff Burnz’s picture

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

I want to use the whole viewport but autogrow does not allow that. I've added a screenshot to show how on one of my monitors (the smallest - Dell 25" 2560x1440) autogrow does not allow me to use about 420px of that height.

Mobile users will likely benefit from autogrow, but for large screen users this may not always be the case - we loose control and we're the major group of content authors. Autogrow is a good feature, but there are costs and those need proper review.

This needs a browserstack and accessibility review. For example it appears autogrow does not respect browser zoom, so if the browser is zoomed autogrow grows off the screen (below the fold etc). I've only had a change to test this in Safari.

xjm’s picture

@Jeff Burnz, can you provide some additional screenshots further illustrating the problems you describe? Edit: to clarify I'm still not understanding what the scrolling-around-forever problem is or isn't. E.g., before-and-after the patch.

It's difficult as a release manager to evaluate long, conflicting text arguments about the issue, and the fact that there is the disagreement at all indicates risk with this change to a stable feature.

Also, please note that the fact that autogrow does not happen until the field receives focus on node load is always reproducible and has never been addressed, even though I have mentioned it in nearly every issue comment I've made. If this were not the case, this screenshot should not be possible:

I am also still seeing the behavior Angie first noticed in her review intermittently when I test #104 on simplytest.me. I don't why @Wim Leers can't also reproduce it, but it's starting to feel a bit Emperor's New Clothes. Perhaps this intermittent bug is not a big deal, a trivial problem in itself and we should add the feature even if it still happens under weird circumstances we haven't figured out, but it's also not cool to dismiss that I can reproduce it. (Like, really, am I hallucinating or making things up?)

Finally, it is legitimate to be concerned about usability and accessibility impacts of this change, and to be concerned that all aspects of the change were not taken into account for the usability review. Let's take a step back and recognize that everyone contributing to this issue is doing so because they care about this feature enough to have tested it, and want to make CKEditor even more awesome to use.

It would be really helpful to not RTBC this issue again until these points have been addressed (and addressed by a third party reviewer). When we disagree, we consult others. This is a valuable feature, but since it's a change to stable code, it is fair to be careful of introducing regressions in the process of adding it. (That, or go down the route of making it an experimental feature, which would require significant changes to the patch also.) And the RTBC wars are really draining. There are 40 minutes between #108 and #111; I really don't think that's enough time to objectively consider the concern and get feedback about it. If this feature doesn't make it into 8.2.x because we had to spend some extra time untangling these questions, that is okay. Drupal 8 minor development is always open.

Thanks for your ongoing investment in this issue.

xjm’s picture

Issue tags: +Needs screenshots

Also, before-and-after screenshots on mobile, including indicating the difference between autogrow and resize, would also help evaluate this.

Tade0’s picture

I tested #104 on Chrome 52/OSX and here are my results:

If this is what happens consistently everywhere then setting autoGrow_onStartup to true in the config should be enough to have the editable area always grow as expected.

thpoul’s picture

This patch sets autoGrow_onStartup to true. Now the editor automatically grows the moment it is created.

Jeff Burnz’s picture

#115 Yes I can commit some time to accessibility testing and providing screenshots. I am rather busy this week so if anyone else has time that would be great also.


Side not: It is helpful if others do simple tests such as zooming the page 200% and leave that setting in place while you work through different scenarios. Accessibility needs to be everyones concern.


#117
#118

autoGrow_onStartup true certainly needs some discussion, are we assuming to much, or not enough if false?

maczizek’s picture

I'm sprinting at DrupalCorn Camp and will test the zooming in my browser.

jayhawkfan75’s picture

@ma.czizek: I too am sprinting at DrupalCorn Camp and will test the zooming in my browser:-)

maczizek’s picture

FileSize
174.73 KB
165.52 KB

It seems that if the page is already zoomed in (200%) and then the content is added, the body field will autogrow but does not extend “below the fold.”
body field autogrows but doesn't extend beyond viewport

However, if enough content is added to a page which is at 100% zoom so that the body field autogrows and then the page is zoomed in to 200%, the body field will extend “below the fold.”
body field below the fold

Tested in Google Chrome version 52.0.2743.116 , Firefox version 47.0 , and IE version 11.0.9600.18376. Results were the same across the browsers; only text content was tested.

Jeff Burnz’s picture

#122 that's great news and a good start. I don't think the 2nd scenario is a problem.

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.

thpoul’s picture

#119 imho if autoGrow is the only option (ie. no configuration available to the user) then it should have autoGrow_onStartup = true.

Since this issue didn't manage to get into 8.2 (such a shame), in order to move it forward and get it into 8.3.x, I suggest making it fully configurable for the users and let them decide whether they need it or not.

Configuration options could be:

  • Enable autogrow (thus disable resize)
  • Disable autogrow (thus enable resize)
  • Enable autoGrow_onStartup
  • Disable autoGrow_onStartup
Wim Leers’s picture

#125 Exposing those 4 configuration options will make this far, far too difficult to understand for users.

Jeff Burnz, how do you want to move this forward?

xjm’s picture

Issue summary: View changes

Nice progress on this issue!

I don't necessarily think #125 should be off the table. I do agree that giving users too many configuration options is confusing. However, if we did something smart with the form (like replacing the thingy about "number of lines" that has the confusing text about WYSIWYGs) with these options when a WYSIWYG is actually enabled, and gave them sensible labels text that would reduce the burden of giving the user a choice. That could also be a followup, especially now that we are not under the wire.

I also do think it's at least worth discussing whether autoGrow_onStartup should be enabled by default, if that fixes the disconnect I describe in #98.

I added some remaining tasks to the summary. The first two un-crossed-off tasks might be complete already from before.

Wim Leers’s picture

Issue summary: View changes
thpoul’s picture

if we did something smart with the form (like replacing the thingy about "number of lines" that has the confusing text about WYSIWYGs) with these options when a WYSIWYG is actually enabled, and gave them sensible labels text that would reduce the burden of giving the user a choice

This is a pretty nice idea!

Yesterday I brought up this issue at the Drupal Usability weekly hangout and @gábor-hojtsy suggested that all concerns should be summarized at the issue summary in order to be easier for people to help figure out what is the best solution for this.

...especially now that we are not under the wire.

We might not be under the wire, but it would be great if we could manage and have this committed in 8.3.x at DrupalCon.

tkoleary’s picture

Issue summary: View changes

This is a nice feature, but of all the usability feedback I've had on CKeditor this is not at the top of the list.

I can think of a few off the top of my head that would have greater impact, including ck blocks, better code snippets and highlighting, two-pane source and preview.

Have we looked closely at overall impact?

xjm’s picture

@tkoleary, sure, but none of that is in scope for this issue, which is specifically about adding this feature.

xjm’s picture

@tkoleary Also, why did you revert my (extensive) summary changes from #127? That was a committer review; please don't undo those changes without summarizing how the points have been addressed? The points should be left there and the response to each documented so that committers know whether and how these review points have been addressed. If it was a crosspost, please be more careful.

xjm’s picture

Issue summary: View changes

Restoring previous summary.

@thpoul, I tried to make a start at summarizing the outstanding work; see the summary diff in #127 (and again here). Let's start with that?

Wim Leers’s picture

@tkoleary The reason we are trying to get this in, is because this is a usability win that basically only requires us to leverage the work the CKEditor team has done. i.e. it's very little work, and easy to maintain.

So, it's medium impact, minimal effort. The things you list (I don't understand all of them) would be great too, and may very well have a bigger impact, but they require huge effort.

tkoleary’s picture

@xjm

If it was a crosspost, please be more careful.

I'm sure that was the case, sorry.

yoroy’s picture

Feedback from UX meeting with @thpoul, @webchick @bojhan and @yoroy.

Autogrow is the way to go (vs. resize). The patch is basically there, but:
Can we look at immediately autogrowing on load of the editor instead of on focus of the text area?

It would be great if this wouldn't attract that much attention to itself by growing on focus (a user input), ideally it would not grow then but already be grown (on load of the wysiwyg editor) to the maximum available height in the viewport.

thpoul’s picture

Can we look at immediately autogrowing on load of the editor instead of on focus of the text area?

#118 autogrows the CKEditor on startup.

It would be great if this wouldn't attract that much attention to itself by growing on focus (a user input), ideally it would not grow then but already be grown (on load of the wysiwyg editor) to the maximum available height in the viewport.

+1

There was also one more thing that @webchick suggested to look into, regarding the "hiccup" of the initial <textarea> height just before the CKEditor loads and autogrows.

maxilein’s picture

Hi,

I'd like to add another approach: Remember size of resizable textarea on a user and node basis on every save

Problem: When working with longer texts the textarea has to be manually increased. After a save it defaults back to its spezified size and one has to increase it manually again!

Proposed Solution: Remember the size of the expander and save it on a per user+field+node basis in order to reload that value whenever the node gets loaded. Could be stored in DB or in a cookie.

R, Max

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -needs accessibility review, -Needs screenshots

#136 has so much common sense. I'm absolutely +1 for that.

… and as @thpoul said in #137, that's exactly what #118 already does. Jeff Burnz also confirmed he liked that change in #123.


Per #119, #122 and #123: accessibility review happened. And screenshots were provided.

(Many thanks!)


I just manually tested again, and it's still working great, including autogrow-on-startup. A week has passed since #136 (the UX team's desired direction) was posted. No more concerns were raised since then.

So AFAICT we're all in agreement, at least mostly. I'd call that consensus. This patch is definitely a step forward. Let's add this to 8.3.x now, so we can hear about problems with this in the coming six months.


#138: That is out of scope. Besides, for any content that isn't very very very long, that will effectively already be the case. Feel free to create a new issue for that though.

Wim Leers’s picture

Status: Reviewed & tested by the community » Needs work

#2797427: Update CKEditor library to 4.5.11 landed, this now needs to be rerolled.

thpoul’s picture

Status: Needs work » Needs review
FileSize
1.29 MB

Here is the reroll :)

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC!

Jeff Burnz’s picture

autogrow on startup +1, looking good!

Wim Leers’s picture

YAY!!!!!!!!!!

Very, very happy to see your +1, @Jeff!

webchick’s picture

Credit.

  • webchick committed 7393e42 on 8.3.x
    Issue #2239419 by thpoul, Wim Leers, webchick, maczizek, xjm, mlewand,...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Excellent work on this, everyone!

Seems like the earlier concerns about making this optional are addressed since the AutoGrow plugin effectively replaces the old version of doing the same thing.

We tested this LIVE on UX meeting this week and everything works as expected. Great work, everyone!

Committed and pushed to 8.3.x. Thanks!

Status: Fixed » Needs work

The last submitted patch, 141: 2239419-141.patch, failed testing.

webchick’s picture

Status: Needs work » Fixed
Issue tags: +8.3.0 release notes

Your FACE failed to apply a patch.

Also tagging for 8.3.0 release notes.

xjm’s picture

I don't see any documentation of the mobile testing?

Status: Fixed » Closed (fixed)

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

ressa’s picture

Since the Resize plugin was removed, and Autogrow didn't make it into Drupal 8.2, the current official release now doesn't have neither. I have created an issue about it here: #2819009: Restore CKEditor Editor Resize plugin, until Autogrow is working

ressa’s picture

The Resize plugin wasn't removed, only the icon was hidden with a bit of CSS. A patch has been posted by @altrugon here: #2820200: Display resize icon for CKEditor on Seven theme.