Opened 6 years ago

Closed 6 years ago

#10208 closed New Feature (wontfix)

Need ability to override a value in CKEDITOR.skins.icons[]

Reported by: Zac Morris Owned by:
Priority: Normal Milestone:
Component: UI : Skins Version: 4.0 Beta
Keywords: Skins Cc:

Description

Currently there is no way to override a value that has already been registered in CKEDITOR.skins.icon[] via CKEDITOR.skins.addIcon.

I propose adding an additional parameter (after the parameter changes I proposed in Ticket #10206https://dev.ckeditor.com/ticket/10206) that would allow a call to addIcon to override any existing value.

This would allow "post build" changes of a skin (via skin.js of the skin), by the end user, without having to rebuild the skin to see the change. As it stands today, if a user wants to change even just a single icon they basically need to create a new skin from source, and either run that skin "unbuilt", or "build" the new skin to regenerate a new icons.png for the new skin.

That may be the desired "way" but I don't think it gives end-users enough flexibility in those situations where their corporate UI guidelines may require a certain look-and-feel that requires using pre-provided image files.

Adding the additional override parameter, provides the following example option:

I could edit the "built" version of the kama skin, and add the following to the top of its skin.js

CKEDITOR.skin.addIcon('Underline','{path to my corporate provided sprites file.png}',-52,-128,true);

such that when I init CKEDITOR this line would override the "default' kama icon to use my own. Yes, that may seem "unoptimized", but in most cases the page that I'm using CKEditor on, has already loaded this image file for other elements, meaning it's just as optimized as using the build generated icons.png.

Change History (9)

comment:1 Changed 6 years ago by Zac Morris

Keywords: Skins added
Type: BugNew Feature

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

Version: 4.1 (GitHub - major)4.0 Beta

4.0 Beta is the first version for which this ticket makes sense.

comment:4 Changed 6 years ago by Wiktor Walc

Changing the release version of skin.js to override the icon through CKEDITOR.skin.addIcon looks like a strange idea.

How about adding a simple CSS on the website that runs CKEditor? (it should work already)

.cke .cke_button__image_icon {
    background: url("/path/to/custom/icons/image.png") no-repeat scroll 0 0 transparent !important;
}

This method has the following advantage: the same installation of CKEditor can be shared across different websites with different themes - each website can override the default toolbar with its specific icons independently.

comment:5 Changed 6 years ago by Zac Morris

That worked with CKEditor3.x, but not with CKEditor4.x

Because the CKEditor4.x "build" process actually applies inline "style=" attributes to the UI icon elements, meaning you can't override the image url via "external" CSS(only)

That's why you need some kind of override, because as it stands right now, the "build" process pretty much makes the UI unchangeable without doing another "build".

comment:6 Changed 6 years ago by Zac Morris

Ah, I didn't scroll over to see your "!important" tagging. I'm assuming that would override even an "inline" style?

What I'm asking for is fully backwards compatible, so it only adds functionality, not subtracts. It also means that if you want to leverage CKEditor.skin.getIconStyle() calls to get icons, what would be returned is the "defined" icon, and not the one you've specified with CSS. Oddly enough, those methods have "overrides", but it means that you would need to do additional JavaScript logic to determine the "visible" src.

I admit it's a may be a rare use case, but the fact that a call to a method called CKEDITOR.skin.addIcon() might actually do "nothing" seems, odd...

comment:7 Changed 6 years ago by Wiktor Walc

Yes, !important should do the trick.

I decided to suggest the CSS approach not only because it's simpler, but also because I had doubts if changing the icon in addIcon() will have eny effect in release version.

To confirm that, I took a release version of CKEDITOR (a custom build created from ckeditor-dev, with the override setting in addIcon()) and added the following code to the replacebycode sample:

CKEDITOR.on('instanceCreated', function(ev) {
    CKEDITOR.skin.addIcon( 'bold', '/customicons/bold.png', 0, true );
})

(changing skin.js was just too ugly for me ;))

the result seemed to be correct when I inspected the button:

<span style="background-image:url(http://127.0.0.1/customicons/bold.png?t=D2KI);background-position:0 -1248px;" class="cke_button_icon cke_button__image_icon">&nbsp;</span>

but the following rule was still used by the browser:

.cke_button__image_icon {
    background: url("icons.png") no-repeat scroll 0 -1248px transparent !important;
}

(because of !important, which was missing in the inline style).

Can you confirm that the same thing happens on your side and that the flag actually does not solve the problem on your end?

The reason why it works this way is quite simple: the skin should have the final word when it comes to the look of the toolbar. If it provides any icons, the automatically created (by builder) CSS styles for icons in editor.css have the !important flag, in order to override any default values provided by plugins.

comment:8 Changed 6 years ago by Zac Morris

I understand where you're coming from, but I'm just not a fan of using "!important", plus managing all the necessary CSS override represents a lot of duplication of effort. Using "!important" just seems like more of a hack to me. So yes, while the CSS Rules are the final outcome, I still prefer using object methods (jQuery/DoJo/EXT/etc.) to make these sorts of changes.

I simply don't understand what "locking"(i.e. non-override-able) the CKEditor.skins.icons[] array at "build time" buys you? Ultimately I could still write JavaScript to override the array directly, but I'd rather use a method provided by the tool I'm working with, vs. doing that kind of manipulation.

There is also the fact that getIconStyle() has override symanitics, yet addIcon() does not.

It would be possible to add a new method changeIcon(), but that does away with one nice feature: When I create a skin wit this method in place (via custom skin.js in my skin dir) I use:

CKEDITOR.skin.addIcon('Bold', CKEDITOR.skin.iconpath+'icons.png',0,0,true);

Now if I "built" the skin, these get read by ckbuilder.jar and the icon "strip file" is created, but because of the "true" override this skin.js also overrides using the generated "stip" in case I wanted to use existing (multiple source) sprite files at run time (without have to rebuild), just having to add a "true", otherwise I have to have these calls here so that the skin builds, BUT I also have to manage all the individual CSS overrides [ugg].

So as a skin designer, yes I know that ultimately someone could create their own custom CSS to override my skin "logic", but having the override allows me to run an "unbuilt" skin, to make changes at the JavaScript level vs the external CSS level. Based on the amount of rework in CKEditor4 away from all the individual CSS element styling, I got the impression that's what CKEditor was trying to do as well?

That make better sense? -Zac

comment:9 Changed 6 years ago by Zac Morris

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