Opened 5 years ago

Closed 5 years ago

#11297 closed New Feature (fixed)

Introduce styles applicable to widgets

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.4.0
Component: Core : Styles Version: 4.3
Keywords: Cc: wim.leers@…

Description (last modified by Piotrek Koszuliński)

Problem

It's not possible to apply style to widgets using styles combo. Styles are either applied around widget or inside its nested editables.

Simple solution (outdated)

The simplest solution I can think of is to focus on classes only. A style with a special property denoting that it can be applied to widgets (or somehow generalised "subject"), could only set class(es). That class would be applied to the widget element (only) and then we or widgets authors should make sure that classes are preserved during downcasting.

Classes would of course make it possible to style all parts of widgets, not only widget element.

Why am I proposing only classes? Because styles and other attributes would make this much more complicated. First of all, they could not be applied on elements inside widget and e.g. img is not a widget element in some cases, so how would that style apply a border to it? Second of all, there are cases in which it would not be easy to ensure that all inline styles and attributes are kept during downcasting. A "border" style for image2 for some people should behave differently whether image is captioned or not and that can be achieved only by classes.

Change History (13)

comment:1 Changed 5 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:2 Changed 5 years ago by Olek Nowodziński

cc

comment:3 Changed 5 years ago by Jakub Ś

Status: newconfirmed

comment:4 Changed 5 years ago by Jakub Ś

#11332 was marked as duplicate.

comment:5 Changed 5 years ago by Piotrek Koszuliński

Description: modified (diff)
Milestone: CKEditor 4.4

comment:6 Changed 5 years ago by Piotrek Koszuliński

Spec v1.

Requirements

The goal is to allow applying styles (CKEDITOR.style) to widgets. Remarks:

  1. Editor should be able to check if style can be applied on certain widget.
  2. Editor should be able to check if style is already applied on certain widget.
  3. Editor should be able to apply style and remove style from widget.
  4. Only widgets themselves know how to apply style to them.
  5. Styles system cannot know about widgets system so there has to be an intermediate level between them.
  6. It's too complex for our widgets to handle inline styles, so by default we will only accept classes. However, if it's feasible, we should make it possible to apply all kind of styles.
  7. We need to remember about integration with ACF.

Architecture

  • New editor.addCustomStyleHandler method. It will be used to define new style types. Currently we have inline, block and object styles, which are element styles and are defined by integer flags CKEDITOR.STYLE_*. The custom style types need to be strings in order to make it simple to make them unique (within editor).

@param {String} type Name of the new type. Must be unique within editor instance.
@paream {Object} handler The object containing handlers.
@param {Function} handler.checkApplicable The same as {@link CKEDITOR.style#checkApplicable}.
@param {Function} handler.checkActive The same as {@link CKEDITOR.style#checkActive}.
@param {Function} handler.apply Applies the style to contents of the editor.
@param {CKEDITOR.editor} handler.apply.editor The editor instance in which style will be applied.
@param {Function} handler.remove Removes the style from contents of the editor.
@param {CKEDITOR.editor} handler.remove.editor The editor instance in which style will be removed.
@param {Function} toFeature See the integration with ACF part.

  • When handling style (e.g. checking if it can be applied to current selection) styles system checks:
    • If the style is one of default types (its definition had the element property defined), the standard handlers are used.
    • If the style is one of custom types (its type is not one of default), then a corresponding style handler is retrieved and its methods are used.

  • Widgets system will add a widget style type. How a style definition will look like then? We definitely need a widget property there which will tell to which widget that style may be applied. Therefore a style definition will look like this:

{ name: 'Thick border', type: 'widget', widget: 'image2', ... the attributes ... }

  • It's now up to widgets system how "the attributes" part should look like. Since we should rather not block a possibility to handle more than classes by some 3rd party widgets (or widgets implemented in the future), the attributes part should be identical to the default styles definitions. However, we'll handle only attributes.class for now.

CKEDITOR.plugins.widget will have 6 new simple methods: applyStyle, removeStyle, checkStyleActive, addClass, removeClass and hasClass. First two will simply get attributes.class from style definition and use 4th and 5th method, 3rd will check if style is applied by using 6th method. Thanks to the fact that all methods are overridable each widget can customize that behaviour. E.g. image2 will override apply/remove/hasClass methods to apply classes not as by default to widget element, but only to img or figure tags (so not to centering wrapper). Other widgets can override applyStyle to handle more than classes.

  • The trickiest part is an integration with the ACF. ACF is able to create allowed content rules from styles because they contain information about elements and their attributes. Custom styles will not provide such information and therefore need to inform ACF what they represent. We may use similar mechanism to `feature#toFeature`, so style handler will actually be a CKEDITOR.feature.

However, this is still not enough. Widgets system will define the toFeature method, but how does it know the prototypes of elements (name + additional stuff like match) to which styles can be applied (both things together create allowed content rule)? Since only widgets themselves know what elements are styleable, they have to contain such information in styleableElements {String} property. Defining such element (as string - e.g. 'img figure') will be the easiest way to make widget styleable and integrated with ACF. However, it may happen that widget overrides applyStyle so much that it's not possible to create allowed content rules from styleable elements and style attributes or that someone wants to define more precise element prototype (e.g. use match). Therefore we need another option - a method which based on style definition will return allowed content rule - styleToAllowedContentRule (I'm open for better names :D).

Not clear yet

  1. How the custom styles will be displayed in styles dropdown? In which group? Widgets styles are a kind of object styles, but we cannot assume that all custom styles will be. We can solve this in two ways:
    1. custom style handler will define to which default group it belongs (I think that it may be a callback too, because then style handler will have full control in which context and when which style of this type is assigned to which group - of course usually it will always return the same value so to KISS it may be a const too),
    2. we'll have custom styles rendered in a separate, 4th group in styles dropdown.

I like the first option more.

Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:7 in reply to:  6 Changed 5 years ago by Frederico Caldeira Knabben

Replying to Reinmar:

The proposal sounds really good. Well done!

Not clear yet

  1. How the custom styles will be displayed in styles dropdown? In which group? Widgets styles are a kind of object styles, but we cannot assume that all custom styles will be. We can solve this in two ways:
    1. custom style handler will define to which default group it belongs (I think that it may be a callback too, because then style handler will have full control in which context and when which style of this type is assigned to which group - of course usually it will always return the same value so to KISS it may be a const too),
    2. we'll have custom styles rendered in a separate, 4th group in styles dropdown.

I'm for option 1, with a constant values, defaulting to "Object" to make it totally optional.

comment:8 Changed 5 years ago by Piotrek Koszuliński

I'm for option 1, with a constant values, defaulting to "Object" to make it totally optional.

Cool.


Spec v2.

Revisions: https://gist.github.com/Reinmar/15989ded9535a15f0fea/revisions

Requirements

The goal is to allow applying styles (CKEDITOR.style) to widgets. Remarks:

  1. Editor should be able to check if style can be applied on certain widget.
  2. Editor should be able to check if style is already applied on certain widget.
  3. Editor should be able to apply style and remove style from widget.
  4. Only widgets themselves know how to apply style to them.
  5. Styles system cannot know about widgets system so there has to be an intermediate level between them.
  6. It's too complex for our widgets to handle inline styles, so by default we will only accept classes. However, if it's feasible, we should make it possible to apply all kind of styles.
  7. We need to remember about integration with ACF.

Architecture

  • New editor.addCustomStyleHandler method. It will be used to define new style types. Currently we have inline, block and object styles, which are element styles and are defined by integer flags CKEDITOR.STYLE_*. The custom style types need to be strings in order to make it simple to make them unique (within editor).

@param {String} type Name of the new type. Must be unique within editor instance.
@paream {Object} handler The object containing handlers.
@param {Function} handler.checkApplicable The same as {@link CKEDITOR.style#checkApplicable}.
@param {Function} handler.checkActive The same as {@link CKEDITOR.style#checkActive}.
@param {Function} handler.apply Applies the style to contents of the editor.
@param {CKEDITOR.editor} handler.apply.editor The editor instance in which style will be applied.
@param {Function} handler.remove Removes the style from contents of the editor.
@param {CKEDITOR.editor} handler.remove.editor The editor instance in which style will be removed.
@param {Function} [toFeature] See the integration with ACF part.
@param {Integer} [assignedTo=CKEDITOR.STYLE_OBJECT] The group (one of default ones) to which this styles of this type will be assigned (e.g. in styles dropdown). Other idea for a name: `belongsTo`.

  • When handling style (e.g. checking if it can be applied to current selection) styles system checks:
    • If the style is one of default types (its definition had the element property defined), the standard handlers are used.
    • If the style is one of custom types (its type is not one of default), then a corresponding style handler is retrieved and its methods are used.

  • Widgets system will add a widget style type. How a style definition will look like then? We definitely need a widget property there which will tell to which widget that style may be applied. Therefore a style definition will look like this:

{ name: 'Thick border', type: 'widget', widget: 'image2', ... the attributes ... }

  • It's now up to widgets system how "the attributes" part should look like. Since we should rather not block a possibility to handle more than classes by some 3rd party widgets (or widgets implemented in the future), the attributes part should be identical to the default styles definitions. However, we'll handle only attributes.class for now.

CKEDITOR.plugins.widget will have 6 new simple methods: applyStyle, removeStyle, checkStyleActive, addClass, removeClass and hasClass. First two will simply get attributes.class from style definition and use 4th and 5th method, 3rd will check if style is applied by using 6th method. Thanks to the fact that all methods are overridable each widget can customize that behaviour. E.g. image2 will override apply/remove/hasClass methods to apply classes not as by default to widget element, but only to img or figure tags (so not to centering wrapper). Other widgets can override applyStyle to handle more than classes.

  • The trickiest part is an integration with the ACF. ACF is able to create allowed content rules from styles because they contain information about elements and their attributes. Custom styles will not provide such information and therefore need to inform ACF what they represent. We may use similar mechanism to `feature#toFeature`, so style handler will actually be a CKEDITOR.feature.

However, this is still not enough. Widgets system will define the toFeature method, but how does it know the prototypes of elements (name + additional stuff like match) to which styles can be applied (both things together create allowed content rule)? Since only widgets themselves know what elements are styleable, they have to contain such information in styleableElements {String} property. Defining such element (as string - e.g. 'img figure') will be the easiest way to make widget styleable and integrated with ACF. However, it may happen that widget overrides applyStyle so much that it's not possible to create allowed content rules from styleable elements and style attributes or that someone wants to define more precise element prototype (e.g. use match). Therefore we need another option - a method which based on style definition will return allowed content rule - styleToAllowedContentRule (I'm open for better names :D).

comment:9 Changed 5 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:10 Changed 5 years ago by Wim Leers

Cc: wim.leers@… added

comment:11 Changed 5 years ago by Olek Nowodziński

I find overriding widgetDef#add/has/getClass(es) quite confusing so I propose some improvements in #11771

comment:12 Changed 5 years ago by Piotrek Koszuliński

Yes, it could be simplified. At the beginning we had 3 methods operating on classes, what was already a lot, now we have four, so they indeed could use some common "get styleable element" logic. Not a blocker now though.

comment:13 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: assignedclosed

The reviews were handled in parts by Fred, Olek and me (for Olek's parts). We reached collective R+ :).

Introduced on major with git:721055c on dev and 45140b7 on tests.

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