Opened 12 years ago
Closed 12 years ago
#11300 closed Task (fixed)
Image2: polishing and refactoring of internals
| Reported by: | Olek Nowodziński | Owned by: | Olek Nowodziński |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.4.0 |
| Component: | General | Version: | 4.3 |
| Keywords: | Drupal | Cc: | wim.leers@… |
Description (last modified by )
Some refactoring got to be done to make Image2 flexible and "environment-friendly". Those changes are relevant for third-party developers, e.g. for D8.
- The internal form of the widget got to be styleable: instead of inline styles, we need to introduce classes. With inline styles, the look of the widget is not configurable and possibly incompatible with custom stylesheets during editor's lifetime.
- The non-captioned image should have no class.
- The class of captioned image must be configurable via
config.image2_captionedClassand applied to<figure class="...">. - Class "caption" is obsolete so it will be removed.
- Alignment also must be controlled with classes.
- Classes must be configurable
- Tons of tests to be aligned to the new API.
- We should guarantee that additional attributes (e.g.
data-*) are preserved, regardless of the internal state of the widget. Extra attributes must survive any kind of internal processing and re-emerge in editor's output (1:1).- Quite likely it's already working because the image node in DOM is passed along with states of a widget (needs tests).
- Alternatively, attributes can be serialized in upcast, saved in widget's data and exploded while downcasting. But let's stick to (a) as an easiest "solution" first.
Change History (19)
comment:1 Changed 12 years ago by
| Description: | modified (diff) |
|---|---|
| Summary: | Image2 in D8 → Image2: polishing and refactoring of internals |
comment:2 Changed 12 years ago by
| Description: | modified (diff) |
|---|
comment:3 Changed 12 years ago by
| Cc: | wim.leers@… added |
|---|---|
| Keywords: | Drupal added |
comment:5 Changed 12 years ago by
| Owner: | set to Olek Nowodziński |
|---|---|
| Status: | new → review |
Changes in dev branch:
- Internally, widget alignment is controlled with classes:
cke_image_left(replacesfloat:left)cke_image_right(replacesfloat:right)cke_image_center(replacestext-align:centeranddisplay:inline-blockof the figure).
- Captioned widget's wrapper has default class
image. It is available both internally and in data.- It's possible to alter the class with
CKEDITOR.config.image2_captionedClass.
- It's possible to alter the class with
- Default caption of captioned widget can be translated (also altered).
- Available via
editor.lang.image2.placeholder.
- Available via
Changes in test branch: aligned test to latest API, improved several tests.
comment:6 Changed 12 years ago by
| Milestone: | → CKEditor 4.3.2 |
|---|
comment:7 Changed 12 years ago by
| Milestone: | CKEditor 4.3.2 → CKEditor 4.3.3 |
|---|
We are still discussing the details and we're not yet sure that the change is complete, so I'm postponing this issue, to avoid making wrong decisions.
comment:8 Changed 12 years ago by
| Status: | review → assigned |
|---|
Development is continued in t/11300b rebased on latest master.
comment:9 Changed 12 years ago by
| Milestone: | CKEditor 4.3.3 → CKEditor 4.4 |
|---|
I changed milestone to 4.4, because it's a change that breaks backward compatibility.
I rebased and force-pushed both branches.
comment:10 Changed 12 years ago by
| Description: | modified (diff) |
|---|
comment:12 Changed 12 years ago by
| Status: | assigned → review |
|---|
comment:13 Changed 12 years ago by
| Status: | review → review_failed |
|---|
- Default aligned image classes cannot contain
cke_*prefix. They are meant to be reused everywhere, so should be generic likefigure.image. - The default value of
config.image2_alignClassesisnulllike for indent classes or justify classes. So by default editor produces styles not classes. - Regarding doc for
config.image2_alignClasses:- Developers are not meant to use
CKEDITOR.addCssfor that. In case of framed editor they indeed should useconfig.contentsCssand/orcontents.css. But in case of inline editor they should attach stylesheet normaly in the <head>. - We should mention what are the styles which CKEditor uses by default for alignment classes. Because when one sets
image2_alignClasseshe'll be lost now. How does he center an image? Is aligned captioned image styled like non-captioned?
- Developers are not meant to use
comment:14 Changed 12 years ago by
It turned out that we had a communication problem. So to make sure that we understand each other, we're talking to add following feature to the state of t/11300b from comment:11.
We want to add a config.image2_alignClasses option which will change the:
- inner format to use classes defined in this option instead of inline styles.
- output format to use classes defined in this option instead of inline styles.
The goal is to make it possible to style aligned images differently (depending on the alignment) in both cases - inside editor and outside it. The first part was actually already doable, but one needed to use CKEditor's classes. Now it will be possible to use the same CSS selectors inside and outside editor.
The default value of image2_alignClasses should be null meaning that inline styles are used in inner and output format.
Remember about good documentation for this option. It should contain an information about styles that one has to add to contents.css to make image2 working - preferably in copy&paste-ready format.
comment:15 follow-up: 16 Changed 12 years ago by
So:
- "inner format" = "downcasted format"
- "output format" = "upcasted format"
Right?
comment:16 Changed 12 years ago by
Replying to Wim Leers:
So:
- "inner format" = "downcasted format"
- "output format" = "upcasted format"
Right?
The "inner format" is the format used inside of the editor, after upcasting.
The "output format" is what you see when you call editor.getData() – a downcasted widget.
comment:17 Changed 12 years ago by
| Status: | review_failed → review |
|---|
Latest code pushed to branch:t/11300d (+tests).
- In this implementation
config.image2_alignClassesisnullby default (inline styles are produced). - If
image2_alignClassesare defined, editor drops support for inline styles and uses classes only. - Introduced
CKEDITOR.htmlParser.element.addClass(with tests).
comment:18 Changed 12 years ago by
| Status: | review → review_passed |
|---|
I force pushed rebased and a little bit enhanced branches. R+!
comment:19 Changed 12 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
git:1c0c524 landed in major (2d65984 tests).
Following with great interest!