Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4463 closed New Feature (fixed)

V3 : Inline CSS support

Reported by: Frederico Caldeira Knabben Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.1
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

With FCKeditor we're able to include the plain CSS for some configuration options directly in the configuration file, avoiding having CSS files being downloaded. This can be used by the skin and the content area CSS, for example. This feature is to be ported to V3.

Attachments (4)

4463.patch (2.1 KB) - added by Garry Yao 8 years ago.
skin.js (45.9 KB) - added by Garry Yao 8 years ago.
Inline CSS sample skin
4463_2.patch (12.1 KB) - added by Garry Yao 8 years ago.
4463_3.patch (14.7 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by Garry Yao

Attachment: 4463.patch added

Changed 8 years ago by Garry Yao

Attachment: skin.js added

Inline CSS sample skin

comment:1 Changed 8 years ago by Garry Yao

Keywords: Review? added
Owner: set to Garry Yao
Status: newassigned

The attached sample skin file could be used for test.

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The contents CSS, panels and dialogs are still missing.

Changed 8 years ago by Garry Yao

Attachment: 4463_2.patch added

comment:3 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

The new patch introduce inline css support for all sorts of panels, menu, preview window, note that it also change the editor loading order to load skin before others (plugins, lang, theme), so plugins could easily reuse those inline styles defined in skin file.
Manual TCs added at : http://ckeditor.t/tt/4463/1.html

comment:4 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Nice job. The loading order also looks good. Some small notes:

  • Instead of having CKEDITOR.skins, let's have "editor.skin", which contains the definition for that editor instance. It will be simpler to use. In any case if we would use the CKEDITOR.skins object, we should add the "get" method to it, just like we have for plugins.
  • The new CKEDITOR.fireOnce( 'pluginsLoaded' ) is wrong. It gives the sense that all plugin have been loaded, but the truth is that only the plugins for a specific instance have been loaded. This should be really an instance event only. For the skin.js needs, we could have CKEDITOR.on( 'dialogPlugin' ), which is fired by the dialog plugin file.

Changed 8 years ago by Garry Yao

Attachment: 4463_3.patch added

comment:5 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

For the skin.js needs, we could have CKEDITOR.on( 'dialogPlugin' ), which is fired by the dialog plugin file.

The skin file may have dependencies on other plugin as well, even the theme, so a generic solution of firing an 'resource+type+ready' event is proposed in the patch.

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

The 'resource+type+ready' is a nice idea.

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

This feature is to be committed into the 3.1 branch, not on trunk.

comment:8 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4464] on 3.1.x branch.

comment:9 Changed 8 years ago by Garry Yao

Oops, seems my committed result is missing certain chunk from the patch, re-fix with [4470] and [4471].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy