#2828494: Update CKEditor library to 4.6 is bringing CKEditor 4.6 to Drupal 8.3. A key new feature is the new default skin. See http://ckeditor.com/blog/CKEditor-4.6-released.

We can adopt it, but it has consequences:

  1. all screenshots in documentation (online & printed) are outdated
  2. #2090937: Seven theme: style CKEditor-native dialogs to match Drupal-native dialogs may need to be redone partially

#1 is the primary concern by far. On the upside, it looks more modern.

This is the Moono skin

This is the Moono-Lisa skin

CommentFileSizeAuthor
#32 cke_moono_lisa-2831442-32.patch1.16 MBwim leers
#26 Bildschirmfoto von »2017-02-09 16-01-35«.png108.5 KBtstoeckler
#19 cke_moono_lisa-2831442-19.patch952.79 KBwim leers
#19 interdiff.txt619 byteswim leers
#17 cke_moono_lisa-2831442-17.patch952.22 KBwim leers
#12 block-create-custom-add-custom-block.png113.79 KBtkoleary
#11 content-in-place-edit-save-box.png32.13 KBtkoleary
#11 create-basic-page.png169.76 KBtkoleary
#10 manage-content-4.png93.7 KBtkoleary
#10 manage-content-3.png140.41 KBtkoleary
#9 Screen Shot 2017-02-06 at 11.22.03 AM.png109.31 KBtkoleary
#9 Screen Shot 2017-02-06 at 11.23.11 AM.png137.87 KBtkoleary

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Needs screenshots

This needs screenshots to judge the before vs after.

mattlt’s picture

mattlt
he/him/they/them
Primary language English
Location Freeport, MN
commented

Here are the product pages for screenshots.

Moono-Lisa…

http://ckeditor.com/addon/moono-lisa

Moono…

http://ckeditor.com/addon/moono

I would say that Moono-Lisa looks lighter, more modern (flatter), and less opinionated in its appearance.

Thanks,

•• matt

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Category: Task » Feature request

.

tkoleary’s picture

tkoleary commented
Assigned: Unassigned » tkoleary

I would be happy to update those screenshots.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Awesome, thanks Kevin!

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Consider switching to CKEditor 4.6's new "Moono-Lisa" skin » Consider switching to CKEditor 4.6's new "Moono-Lisa" skin

#2828494: Update CKEditor library to 4.6 is in. We can now do this.

tkoleary’s picture

tkoleary commented
Issue summary: View changes
StatusFileSize
new Screen Shot 2017-02-06 at 11.23.11 AM.png137.87 KB
new Screen Shot 2017-02-06 at 11.22.03 AM.png109.31 KB
tkoleary’s picture

tkoleary commented
Issue summary: View changes
StatusFileSize
new manage-content-3.png140.41 KB
new manage-content-4.png93.7 KB

For the documentation pages, I looked here: https://www.drupal.org/docs/8/core/modules/ckeditor/overview, and on the pages that link from there and there do not appear to be any screenshots associated with the module itself.

There are two screenshots that feature the moono skin here: https://www.drupal.org/docs/8/administering-drupal-8-site/managing-content. The two images there can be replaced with these:


Which have the same names.

tkoleary’s picture

tkoleary commented
Issue summary: View changes
StatusFileSize
new create-basic-page.png169.76 KB
new content-in-place-edit-save-box.png32.13 KB

There is also a screenshot in the user guide here: https://www.drupal.org/docs/user_guide/en/content-create.html

Which can be replaced with this one:

And in the user guide there is a page on in-place editing here: https://www.drupal.org/docs/user_guide/en/content-in-place-edit.html

The second image there can be replaced with this one:

Thus far that's all I can find, but there are doubtless others.

tkoleary’s picture

tkoleary commented
Issue summary: View changes
StatusFileSize
new block-create-custom-add-custom-block.png113.79 KB

Found another one here: https://www.drupal.org/docs/user_guide/en/block-create-custom.html

The image there can be replaced with this one:

tkoleary’s picture

tkoleary commented

And there's the image here: https://www.drupal.org/docs/user_guide/en/structure-text-format-config.html

But this one is dependent on updating the configuration UI, which I think should be a sub issue.

tkoleary’s picture

tkoleary commented

Added the sub-issue #2850508: Update CKEditor config UI to match moono-lisa skin with a patch.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

There was unanimous consensus on our UX meeting today that this is a good direction for us to move in for 8.4 (and 8.3 if by some miracle we get it in in time :)).

First, we need a patch, though.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Consider switching to CKEditor 4.6's new "Moono-Lisa" skin » Switch to CKEditor 4.6's new "Moono-Lisa" skin

#15: oh, that's exciting!

Updating issue title accordingly.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new cke_moono_lisa-2831442-17.patch952.22 KB

This updates Drupal 8's CKEditor build to use Moono Lisa instead of Moono. In my manual testing, this worked great. Nothing else needs to be done AFAICT.

Status: Needs review » Needs work

The last submitted patch, 17: cke_moono_lisa-2831442-17.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt619 bytes
new cke_moono_lisa-2831442-19.patch952.79 KB

Thanks god for test coverage. I would never have noticed that.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Usability, +Needs manual testing
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Tried this out, it looks great!

Two notes:

  1. I think we need to update the CKEditor toolbar configuration as well, as that is styled with the old theme
  2. When you hover over the toolbar buttons in the editor there is some "twitching" caused by margin/border/padding differences between the hover and non-hover state. This is with Chromium 55.0.2883.87 Built on Ubuntu , running on Ubuntu 16.04 (64-bit). This does not happen with the old theme without the patch.
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
  1. For that, there is #2850508: Update CKEditor config UI to match moono-lisa skin. Do you think it should be blocking? I don't think it should be. In fact, I almost like that they're styled differently… it helps convey that it's just a structural preview, not a pixel-perfect preview (in the config UI).
  2. Interesting. I did not notice that (Chromium 56.0.2924.87 on macOS Sierra).
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I think #22.2 can be a follow-up bugfix BTW. If this lands, then Drupal 8.3 will ship with this new CKEditor skin. Otherwise it'll have to wait until 8.4. That's a 6-month difference.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

I agree with #23, especially if it apparently doesn't affect everyone. I'll try find some time to look into the exact problem.

I don't feel like I can RTBC this, but I did test pretty much everything I could think of BTW, i.e. the dialog all the different buttons, etc.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Did you also test Quick Edit?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue summary: View changes
StatusFileSize
new Bildschirmfoto von »2017-02-09 16-01-35«.png108.5 KB

I did now. ;-) It works great and I actually think it looks slicker now, as well. Screenshot attached. I also added a bunch of non-default buttons this time and didn't find any problems neither in the normal mode nor in quickedit, including images, etc.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Yep, that's how it worked for me too :) I agree it looks much better. Just great to see that confirmed by somebody else! :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots, -Needs manual testing

I think this has had enough manual testing. And in fact, I think you've effectively RTBC'd this issue by doing that. We already have UX team sign-off (see #15). @tstoeckler then did all the manual testing work. I don't think we need anything else.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

OK, sure, awesome!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +8.3.0 release notes
tkoleary’s picture

tkoleary commented

Awesome!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new cke_moono_lisa-2831442-32.patch1.16 MB

#2848215: Update CKEditor library to 4.6.2 landed, which means this needs a reroll. Doing that.

tkoleary’s picture

tkoleary commented

Manual testing

Tested #32 on Simplytest.me in chrome, mac, sierra; firefox, mac, sierra.

  • Tested node/edit in Bartik, Seven, Stark with full HTML
  • Tested comment in Bartik, Seven, Stark with full HTML
  • Tested quickedit in Bartik, Stark with full HTML
  • Tested all of the above at 1400px viewport and 400px viewport

No issues.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks!

  • webchick committed 509ab4c on 8.4.x 
    Issue #2831442 by Wim Leers, tkoleary, tstoeckler, mattlt: Switch to...

  • webchick committed c5244eb on 8.3.x 
    Issue #2831442 by Wim Leers, tkoleary, tstoeckler, mattlt: Switch to...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Ok. Normally this would not be eligible for a minor release, given we are between alpha and beta. However, discussed with the other core committers yesterday and given that this is the new _default_ skin for CKE 4.6+, and given that as a result, bug fixes and other maintenance is far more likely to happen there than in the older version, we should go ahead and get this in. Also, on the plus side, pretty! ;)

Committed and pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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