Opened 7 years ago

Closed 7 years ago

#9255 closed Task (fixed)

Styles.js - should it stay in the root folder?

Reported by: Wiktor Walc Owned by:
Priority: Normal Milestone:
Component: Core : Styles Version: 4.0
Keywords: Discussion Cc:

Description

styles.js looks to me like a leftover from FCKeditor for which we did not find a better place.

What's exactly the problem with this file?

It contains sample data used exclusively by two optional plugins (if I'm correct): stylescombo and div. In fact the default styles.js does not contain even a single sample for the div dialog, so it's just a sample file for the "Styles" combo.

If styles.js is not an essential part of core, it should be rather located in the plugin folder.

Proposed solution:

  1. Move styles.js into the folder of stylescombo plugin.
  2. The default value for stylesSet should be empty. Eventually it could be overwritten by the stylescombo plugin, because only then styles.js will be available. There is a hardcoded path to CKEDITOR.getUrl( 'styles.js' ) somewhere in core/styles.js.
  3. To make user's life easier, we can eventually move 'styles.js' into the root folder (using a special directive in the manifest file) of CKEditor, but only if the "stylescombo" plugin is included in the release.

Point (3) leads to some additional problems:

  1. If we go ahead with (3) why the templates plugin should be different? To specify the teplate one have to go into the templates' plugin folder.
  2. If we go ahead with (3) we will have different paths to styles.js in development, source and release modes.

Last word regarding core...

Comments in core/styles.js mention the "Styles" combo. It's quite unclear where does it come from if we treat plugins as separate things. I believe comments must be corrected as well to clearly specify which plugin provides it and how it is used.

Change History (5)

comment:1 Changed 7 years ago by Garry Yao

Component: GeneralCore : Styles
Status: newconfirmed

This's not caused by legacy code, the idea is that "styleSet" has become a core concept in v4, to make style-oriented plugin (e.g. "div" and "stylescombo") implemented with ease.

The styles.js (in root) file is supposed to be the bucket, to configure all editor's styles, from the style name.

I see currently this set of styles are used merely by the "stylescombo" plugin, but the real intention, I believe is to make it possible to overwrite all config.coreStyles_{styleName}.

So here are my proposal for this change:

  1. Have the current contents of styles.js, moved to stylescombo plugin, which is merely used there.
  2. Have the rest of plugins, define their own set of styles.
  3. Making styles.js file an skeleton file, like config.js, with the capability of overwriting all defined styles.

comment:2 Changed 7 years ago by Frederico Caldeira Knabben

Keywords: Discussion added
Milestone: CKEditor 4.0

The concept of "styles" is a key thing inside CKEditor. The API for it in fact has been moved into core, just because of it.

We want to make it possible for people to easily configure their styles. Then, if they are used by the styles combo or by any other plugin, is a matter of the plugin itself. But of course, being the Style plugin an almost-default one, it'll get more benefit out of it.

Just to exemplify, the image dialog could list the avaiable img tag styles, or even a theoretical "Paragraph Properties" plugin could take benefit of all "block styles".

So, having it in the root, make it as easy as config.js to handle and at the same time makes it available for all plugins.

In any case, we're now on a stabilisation period for the v4 beta, so it is not time for drastic changes. I'll still leave this ticket open though, but I think it should be closed.

comment:3 Changed 7 years ago by Frederico Caldeira Knabben

I've just made a minor comment change in that file (3efa127). More extensive "talk" can be in the file, but proper documentation must come together.

comment:4 Changed 7 years ago by Jakub Ś

Just a note - This file (concept) is not present in v3. This is v4 issue.

comment:5 Changed 7 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: confirmedclosed

I've added a few comments on style.js with 5e1a341.

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