Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Icons can't use fill color attributes #3420

Closed
michaeldjeffrey opened this issue Nov 1, 2018 · 3 comments · Fixed by ckeditor/ckeditor5-theme-lark#207
Closed

Custom Icons can't use fill color attributes #3420

michaeldjeffrey opened this issue Nov 1, 2018 · 3 comments · Fixed by ckeditor/ckeditor5-theme-lark#207
Assignees

Comments

@michaeldjeffrey
Copy link

User defined svg icons cannot use fill as an attribute to set their color.

<polygon id="path" fill="red" points="1 2 3 4"></polygon>

will never be red unless the color set by a parent is red.

@oleq
Copy link
Member

oleq commented Nov 2, 2018

Hi, I'm not sure I understand your issue but I assume you're using custom SVG icons in your project that define fills using inline attributes.

If I'm right, then yes, CKEditor uses CSS to control fills of the icons which allows an easy single-line-of-code customization (like in our theme customization guide). This is one of the assumptions of the system. And yes, this way of controlling the colors will probably override your all fill="..." declarations in SVG files.

You can override our CSS rules with your custom colors in CSS, e.g. by applying CSS classes to <path> elements.

michaeldjeffrey referenced this issue in michaeldjeffrey/ckeditor5-theme-lark Nov 2, 2018
Only override the fill for elements that don't declare one.
Fixes #206
@dkonopka
Copy link
Contributor

dkonopka commented Nov 5, 2018

@michaeldjeffrey If you are looking for other easy way to fix it, just use CSS Specificity power and add inline style in the <svg> file:

<polygon id="path" points="1 2 3 4" style="fill: red"></polygon>

@michaeldjeffrey
Copy link
Author

@dkonopka That's the fix I'm employing now.

The workflow I'm aiming for (if I can be so selfish) is to be able to take an icon from a designer and now have to fiddle with how their colors were applied.

It appears the reason fill attributes don't work as expected is something to do with Firefox, and that if there was someway to keep the default behavior for everywhere, that would be ideal.

I submitted a PR-207 that only declares the fill if the element does declare it itself.

Thanks for all the work you guys put into this project though, it's very much appreciated.

@oleq oleq self-assigned this Nov 19, 2018
Reinmar referenced this issue in ckeditor/ckeditor5-theme-lark Nov 22, 2018
Fix: Only override the `fill` for icons that don't declare one. Closes #206.

Thanks to @michaeldjeffrey!
Reinmar referenced this issue in ckeditor/ckeditor5-alignment Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-basic-styles Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-core Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-font Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-heading Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-highlight Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-link Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-list Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-media-embed Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-paragraph Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-table Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-ui Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-undo Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue in ckeditor/ckeditor5-widget Nov 22, 2018
Other: Improved SVG icons size. See ckeditor/ckeditor5-theme-lark#206.
Reinmar referenced this issue Nov 22, 2018
Internal: Created the `clean-up-svg-icons` npm task that optimizes SVG icons. Added SVGO to npm dependencies. Updated the Development Environment guide (see ckeditor/ckeditor5-theme-lark#206).
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-theme-lark Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants