New CKEditor skin

[Marek Lewandowski]

We could refresh our skin a little bit.

[kkrzton]

Changes in ​t/14569. Apart from adding all moono-lisa skin files, I also switched the default skin to moono-lisa, which means updating some moono files (skin.js, readme.md), adding icons folder so moono can be used in dev mode and also updating icons in plugin folder to ones used by a new skin.

I also added ​separate PR with changes in colorbutton plugin needed for the new default color palette.

[Olek Nowodziński]

Because editor.skin is an alias to CKEDITOR.skinName, not the other way around (​https://github.com/cksource/ckeditor-dev/blob/t/14569/core/editor.js#L387-L388), the dialog cover (​https://github.com/cksource/ckeditor-dev/blob/t/14569/plugins/dialog/plugin.js#L2111) fails to work in Moono-Lisa (editor.config.skin is undefined).

Check the global editor sample/index.html.

---

I guess this sort of issue may also affect other places where moono-lisa–specific assertions are used:

• ​https://github.com/cksource/ckeditor-dev/blob/t/14569/plugins/image/dialogs/image.js#L273
• ​https://github.com/cksource/ckeditor-dev/blob/t/14569/plugins/link/dialogs/link.js#L112

[Olek Nowodziński]

IMO ​template images must also be preserved in *.gif format (but with the new design) for smooth upgrade.

ATM, if developers update to 4.6.0 (they may still want to use Moono), templates they created using CKEDITOR.addTemplates will lose their images, because template1.gif has been replaced by template1.png.

WDYT?

[kkrzton]

@a.nowodzinski As I understand correctly, what you mean is that checks like this ​https://github.com/cksource/ckeditor-dev/blob/t/14569/plugins/dialog/plugin.js#L2111 should use CKEDITOR.skinName instead of config.skin to work correctly in all cases (because for default skin editor.config.skin is undefined)?

As for the templates you are right, it may break the usage for custom images for some users. Using .gif with a new design is a much safer approach.

[Olek Nowodziński]

It could be ( CKEDITOR.skinName || editor.config.skin ) – to be checked.

As for template images, we can use .png if they look better but .gif fallback must also be provided.

[Olek Nowodziński]

As for samples/skin/

• Don't you think that the features of samples/skin/normal.html belong here samples/old/uicolor.html?
• samples/skin/inline.html is the same as http://ckeditor.dev/samples/old/inlineall.html?

[Olek Nowodziński]

I see no support for balloonpanel plugin in moono-lisa (​http://tests.ckeditor.dev:1030/tests/plugins/balloonpanel/manual/balloonpanel).

Did you check Accessibility Checker with moono-lisa?

[kkrzton]

I started wondering if samples/skin/ are actually needed as we have lots of samples in samples/old/. Maybe we should remove whole samples/skin/ folder as it was created only for the purpose of presenting specific editor toolbar configurations in different modes?

As for the balloonpanel it is a part of Accessibility Checker so it will be styled along with the rest of the a11ychecker plugin.

[Olek Nowodziński]

I started wondering if samples/skin/ are actually needed as we have lots of samples in samples/old/. Maybe we should remove whole samples/skin/ folder as it was created only for the purpose of presenting specific editor toolbar configurations in different modes?

I guess so. Most of the features (RTL, inline, etc.) can be tested with old/ samples. Maybe except HC mode sample, but it can be enabled system–wide and editor will respect it. So, yep, ditch them.

As for the balloonpanel it is a part of Accessibility Checker so it will be styled along with the rest of the a11ychecker plugin.

I think it should be aligned to moono-lisa before 4.6.0 is released. It's the last piece and one of the best features of v4 line.

[kkrzton]

I have created a separate task for balloonpanel and a11ychecker plugin skin adjustments: ​http://dev.ckeditor.com/ticket/14763.

[Olek Nowodziński]

Replying to k.krzton:

I have created a separate task for balloonpanel and a11ychecker plugin skin adjustments: ​http://dev.ckeditor.com/ticket/14763.

Thanks, @k.krzton!

[kkrzton]

Fixed with ​3d35e39.

I added proper skin name checking, changed template images back to *.gif (no need for *.png support as those images are monochromatic so there is no visual difference between *.gif and *.png). I also removed samples/skin.

[kkrzton]

I also created a separate task for WebSpellChecker plugin skin adjustments: ​http://dev.ckeditor.com/ticket/14790.

[Olek Nowodziński]

I reviewed the changeset since ​http://dev.ckeditor.com/ticket/14569#comment:11 and it looks great to me.

IMO it's time we brought the new skin to the community to get more feedback from developers who integrate CKEditor in their applications. If any, further issues should be fixed as followups.

Good job!

[kkrzton]

Thanks @a.nowodzinski.

Just as a reminder, there is also a small ​PR related to colorbutton plugin which should be merged along with this ticket.

[kkrzton]

I added one small commit ​e75baa9 to the branch. It is one line of CSS adding white box-shadow like border. It was added to AC dialog to make contrast between green button and focus border accessible and while the same colors are used in other dialogs, those dialogs needed to be updated.

I suppose there is no need for re-reviewing this ticket as it is only a visual change which was already accepted.

[Marek Lewandowski]

Fixed with ​git:f5bad7c6c1 (merged to major). Great job!

[Olek Nowodziński]

Good job, guys! It's a huge progress for the project <3