Opened 10 years ago
Closed 10 years ago
#12932 closed Bug (invalid)
Support for plugin icons to be retina / better addButton documentation
Reported by: | Chris Graham | Owned by: | |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | Documentation & Samples | Version: | |
Keywords: | Cc: |
Description (last modified by )
The api-doc for addButton is very vague, and incomplete. 'icon' is not mentioned for example.
About 2 years ago I made my first CKEditor plugin and worked it out via a comment on the forum. Coming back to it now I see that the code is buried deep away from the addButton definition, in plugins/button/plugin.js -- so, very hard to find. I think all reasonably useful options should get documented for addButton.
Now, the reason I was looking at all this...
We need to support Retina. So I think we need something like an 'icon_2x' property, which does not currently exist. My understanding is that the inbuilt plugins draw from a precompiled master sprite somehow, which exists in both retina and non-retina versions.
If I am missing something, please let me know. Perhaps for example somehow retina support is compiled in at build-time only and for sprites only, with it auto-searching the filesystem when building the sprites. I don't know, because I think it's not documented ;-).
We are missing document explaining how to support Retina. Please see comment:2 for more details.
Change History (6)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Component: | UI : Toolbar → Documentation & Samples |
---|---|
Status: | new → confirmed |
That is true. It seems that we have missed SDK when implementing retina http://dev.ckeditor.com/ticket/9923#comment:13.
Thanks for pointing this out.
Ok, to help you out a little bit here is what you can do:
- Please add
hidpi : true
to plugin definition (http://docs.ckeditor.com/#!/api/CKEDITOR.pluginDefinition-property-hidpi). - Create hidpi folder inside icons folder.
To get better idea what I'm talking about, please see https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/blockquote/plugin.js#L237 and https://github.com/ckeditor/ckeditor-dev/tree/master/plugins/blockquote/icons
comment:3 Changed 10 years ago by
That's helpful, thank you.
However we still seem to have two outstanding problems...
1) Before I read the Plugin SDK document I was using the 'icon' dictionary item going through via addButton (and as mentioned above, it was never documented). The Plugin SDK document told me to use the 'icons' dictionary item to plugins.add. However, I think this is only the case for CKBuilder-built addons (i.e. bundled optimised ones), not ones that are added in after-the-fact (although I suppose perhaps you do know what core icons you have and assume non-core ones are in a plugin images directory). I got "undefined is not a function" when I tried to do it the Plugin SDK way when testing now.
2) I think Retina doesn't work when using the 'icon' dictionary item going through via addButton. It certainly seems to be loading the non-retina asset for me even when I have followed your instructions above.
So as a summary, I think the following actions are needed...
a) Formally document hidpi development b) Document addButton better c) Support hidpi with addButton icon specification d) Fix the Plugin SDK document to say how custom icons can be referenced (i.e. via addButton)
AND/OR, correct me if I'm going wrong somewhere.
comment:4 Changed 10 years ago by
ah, "undefined is not a function" was because it needs a comma-separated list in a string, not a JS list. However when doing that it doesn't work because it requires the cke_button{iconname}_icon class defining. That's okay for me, I'll do that in our CMS, but the plugin guide doesn't mention it (probably it works for you guys as you go through your build process, but doesn't work for plugin distributors or CMSs that are not doing their own custom CKEditor builds).
comment:5 Changed 10 years ago by
Description: | modified (diff) |
---|
To use Retina there is nothing more than I have written. You use hidpi property and have correct folder structure with 2x icons. These icons will be loaded on display which supports Retina.
The Plugin SDK document told me to use the 'icons' dictionary item to plugins.add.
I have absolutely no idea what you are talking about (what is icon dictionary?). Perhaps if you would have shown it in code, it would be more understandable.
About icons. I'm not sure if this is what you had problem with but:
When using below, icon name has to be the same as button name and starting with lower case.
CKEDITOR.plugins.add( pluginName, { icons: 'buttonName',
Another way to add icon to editor is (in this case name of the icon is what you imagine):
editor.ui.addButton && editor.ui.addButton( 'ButonName', { icon: this.path + 'images/icon.png'
I have told you about some code example. I don't think however that it is necessarry because icons are not related to retina support in this case and I would like to keep this ticket clean.
- Formally document hidpi development - missing and we have this confirmed
- Support hidpi with addButton icon specification - it is not related. Correct icon gets loaded on proper display.
- Document addButton better
- Fix the Plugin SDK document to say how custom icons can be referenced (i.e. via addButton)
I believe it is documented properly. If not, please open new ticket for it.
comment:6 Changed 10 years ago by
Resolution: | → invalid |
---|---|
Status: | confirmed → closed |
Sorry for the confused ticket. I think I was driving you a little mad :P. I was learning/rushing, while coming up against various stumbling blocks, and also trying to push your hidpi support (we're planning on supplying Kama hidpi icons for you and I want to understand things enough to test it). I am closing this and opening some much clearer ones. I'm going to make a lot of subtle points in my new tickets that say what I've learned, because it's kind of been detective work and I want you to be able to get these details through to the docs.
Ah, I should have been looking at your Plugin SDK documentation, not the API docs. Partly my bad for using Google too much, rather than RTFM ;).
Still, I think my point is still valid that there are many useful dictionary parameters going through addButton not mentioned in the apidoc.
I think my point about Retina is still valid; if there is an existing solution for a plugin that isn't sprited in, it would be good to document in the Plugin SDK.