Icon storage strategy is needed

[oleq]

ATM basic-styles provides own icons https://github.com/ckeditor/ckeditor5-basic-styles/tree/master/theme/icons, which duplicate with Lark's https://github.com/ckeditor/ckeditor5-theme-lark/tree/master/theme/icons. Also visible in the MT

OTOH other modules like Link (or List) use Lark's icons https://github.com/ckeditor/ckeditor5-link/tree/master/theme.

1. Which strategy is right?
2. Should we keep icons in Lark?
3. Should we keep icons in features?

I discovered this inconsistency when satisfying https://github.com/ckeditor/ckeditor5-image/issues/21. Where should I put them?

/cc @Reinmar

[fredck]

In my opinion, icons should be provided by features.

If the Theme provides an icon for a feature it overrides the feature icon. This feature doesn't have to be included right now, though.

[Reinmar]

Totally agree with @fredck.

[oskarwrobel]

I have a problem to imagine how (after introducing inline svg icons imported inside plugin) theme will override plugin icon.

[oleq]

That's a good point, @oskarwrobel. Do you have any solution?

[Reinmar]

I've just noticed that you added object-left/center/right icons to the image feature, which means that e.g. a media feature will not be able to use them (without depending on the image feature). Now I understand why a theme might need to provide some icons. And in case of generic ones like those, I feel that perhaps it'll be better if they land in the theme.

I agreed with Fred's idea that theme's icons should override feature icons, but indeed – this won't be possible with imports and an external bundler (like Webpack) resolving paths... unless, we'll add an option to our plugins for these bundlers to resolve paths in a certain way. Which is something that we'll do anyway for JS. But then, where should those generic icons land? ;|

[fredck]

Core?

[oleq]

I've just noticed that you added object-left/center/right icons to the image feature, which means that e.g. a media feature will not be able to use them

That's why I wrote in the PR that it awaits resolve of this discussion.

[Reinmar]

I'm fine with putting them in the core. So that PR could be moved there.