-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[JEWEL-1072] Support CSS class references in SVG rendering #3335
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
[JEWEL-1072] Support CSS class references in SVG rendering #3335
Conversation
4253870 to
6aedad3
Compare
...src/main/kotlin/org/jetbrains/jewel/ui/painter/hints/EmbeddedToInlineCssStyleSvgPatchHint.kt
Outdated
Show resolved
Hide resolved
| selectors.forEach { selector -> | ||
| // Only process simple class selectors | ||
| if (selector.matches(CLASS_SELECTOR_PATTERN)) { | ||
| rules[selector] = CssRule(selector = selector, properties = properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Duplicate CSS selectors overwrite instead of merge properties
When the same CSS class selector appears multiple times (e.g., .st0 { fill: red; } .st0 { stroke: blue; }), the code completely replaces the rule instead of merging properties. At line 286 in parseCssBlock and line 413 in addStyleElement, the assignment rules[selector] = ... and cache[className] = rule overwrites existing entries. Per CSS cascade rules and the documented behavior ("Non-conflicting properties are merged"), properties from duplicate selectors should be merged, with later values overriding earlier ones for the same property. This could cause incorrect SVG rendering when design tools export duplicate class definitions.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point, but I'm ok with a follow-up issue/PR for this in 0.34
...test/kotlin/org/jetbrains/jewel/ui/painter/hints/EmbeddedToInlineCssStyleSvgPatchHintTest.kt
Show resolved
Hide resolved
6aedad3 to
032e284
Compare
...src/main/kotlin/org/jetbrains/jewel/ui/painter/hints/EmbeddedToInlineCssStyleSvgPatchHint.kt
Outdated
Show resolved
Hide resolved
| selectors.forEach { selector -> | ||
| // Only process simple class selectors | ||
| if (selector.matches(CLASS_SELECTOR_PATTERN)) { | ||
| rules[selector] = CssRule(selector = selector, properties = properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point, but I'm ok with a follow-up issue/PR for this in 0.34
...src/main/kotlin/org/jetbrains/jewel/ui/painter/hints/EmbeddedToInlineCssStyleSvgPatchHint.kt
Outdated
Show resolved
Hide resolved
| private fun Element.removeAllStyleElements() { | ||
| val styleElements = getElementsByTagName("style") | ||
| // Remove in reverse to avoid index shifting issues | ||
| for (i in styleElements.length - 1 downTo 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: assuming it exists for this type of collection
| for (i in styleElements.length - 1 downTo 0) { | |
| for (i in styleElements.lastIndex downTo 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it doesn't :(
public interface NodeList {
/**
* Returns the <code>index</code>th item in the collection. If
* <code>index</code> is greater than or equal to the number of nodes in
* the list, this returns <code>null</code>.
* @param index Index into the collection.
* @return The node at the <code>index</code>th position in the
* <code>NodeList</code>, or <code>null</code> if that is not a valid
* index.
*/
public Node item(int index);
/**
* The number of nodes in the list. The range of valid child node indices
* is 0 to <code>length-1</code> inclusive.
*/
public int getLength();
}But I can add an extension property for it if you think it's necessary.
...src/main/kotlin/org/jetbrains/jewel/ui/painter/hints/EmbeddedToInlineCssStyleSvgPatchHint.kt
Outdated
Show resolved
Hide resolved
|
Great job, just a couple small things to adjust. |
faogustavo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with a minor comment
| expectedOutput = | ||
| """ | ||
| <svg xmlns="http://www.w3.org/2000/svg"> | ||
| <rect style="fill:orange;stroke:black;stroke-width:2" x="120" y="120" width="60" height="60"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, if you define color multiple times in the class, the last one should be the one that's applied, yeah. But maybe I'm misunderstanding what you're asking, as that's a separate potential issue.
In this case:
- The class defined fill: blue
- The style defined fill: orange
So fill: orange is the correct expected output, no?
@nebojsa-vuksic we need to cover this scenario too:
<style type="text/css">
.blue-rect { fill: blue; stroke: black; fill: red; stroke-width: 2; }
</style>I expect a fill: red output. Same in this case:
<rect style="fill: orange; fill: red;" x="120" y="120" width="60" height="60"/>(and permutations)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rock3r @faogustavo The current implementation has non-intuitive behavior that I'd like to clarify and improve.
Current Behavior:
When processing this example:
<style>.blue-rect { fill: blue; stroke: black; stroke-width: 2; }</style>
<rect class="blue-rect" style="fill: orange" />The implementation:
- Collects class properties into a map:
"fill" -> "blue"
"stroke" -> "black"
"stroke-width" -> "2"- Collects inline style properties:
"fill" -> "orange"- Merges them by replacing class property values with inline values when keys match
This produces:
style="fill: orange; stroke: black; stroke-width: 2"because value of a fill attribute has been overwritten. If there was no occurences of the fill attribute in the class definition, the inline style property would just be appended to the end.
While this achieves the correct CSS cascade behavior (inline overrides class), the map-based replacement approach is not intuitive. It also ensures only one occurrence of each property, but the mechanism isn't obvious from reading the code.
I'll refactor this to use a simpler, more conventional approach: prepend class properties, then append inline properties. This makes the CSS cascade explicit:
style="fill: blue; stroke: black; stroke-width: 2; fill: orange"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated implementation to do following thing:
- resolve all class references and merge them together. This will result in a list of unique properties where the value of the property is equal to the one from the last occurence of the property definition.
- In don style attribute, prepend class style properties before already present style attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe some test on the scenario that Seb mentioned could be added?
smth like this for example
@Test
fun `parse duplicate properties in class selector`() {
doTestEmbeddedCssInlining(
input =
"""
<svg xmlns="http://www.w3.org/2000/svg">
<style type="text/css">
.blue-rect { fill: blue; stroke: black; fill: red; stroke-width: 2; }
</style>
<rect class="blue-rect" x="120" y="120" width="60" height="60"/>
</svg>
""",
expectedOutput =
"""
<svg xmlns="http://www.w3.org/2000/svg">
<rect style="fill:red;stroke:black;stroke-width:2" x="120" y="120" width="60" height="60"/>
</svg>
""",
)
}
Add EmbeddedToInlineCssStyleSvgPatchHint to convert embedded CSS class
selectors in SVG files to inline style attributes.
Many vector graphics editors (Adobe Illustrator, Inkscape, Figma) export SVGs
with embedded <style> blocks containing CSS rules instead of inline styles:
<style type="text/css">
.st0 { fill: red; opacity: 0.5; }
</style>
<circle class="st0" cx="10" cy="10" r="5"/>
This hint transforms class-based styles into inline attributes, enabling
proper rendering in Jewel painters:
<circle style="fill:red;opacity:0.5" cx="10" cy="10" r="5"/>
Implementation features:
- CSS parser supporting minified CSS, comments, and CDATA sections
- Multiple selector handling (.st0, .st1 { ... })
- Full CSS cascade: multiple classes with later override, inline precedence
- URL reference preservation for gradients and patterns
- Processes only class selectors; ignores ID, element, and attribute selectors
- Removes processed <style> blocks and class attributes after inlining
Signed-off-by: Nebojsa.Vuksic <[email protected]>
032e284 to
fcba5c1
Compare
|
Ready to merge |

Summary
This PR adds
EmbeddedToInlineCssStyleSvgPatchHint- a new painter hint that enables rendering of SVG files exported from vector graphics editors that use embedded CSS<style>blocks with class selector references instead of inline styles.Problem
Design tools like Adobe Illustrator, Inkscape, and Figma commonly export SVGs using CSS class references:
Many design tools export SVGs with CSS class references like
.st0,.st1. Since Skiko does not support CSS class selector references, these icons render incorrectly or not at all.Solution
This hint transforms class-based styles into inline
styleattributes during SVG loading. The hint acts as a preprocessing step:<style type="text/css">blocks.className { ... })<style>blocks andclassattributesBefore & After
Before change ('sun' icon was rendered without color)
After change
Transformation Example
Input SVG (from design tool):
Output (after hint processing):
Implementation Details
What Changed
EmbeddedToInlineCssStyleSvgPatchHintimplementingPainterSvgPatchHint<style type="text/css">blocks, supporting minified CSS, comments, and CDATA<style>blocks andclassattributes after inliningCSS Feature Support
✅ Supported Features
.classNameselectors.st0 { fill: red; }.st0, .st1 { fill: red; }<circle class="base override"><circle class="st0" style="fill: blue">base override.st0{fill:red;opacity:0.5;}/* */and inline comments/* comment */ .st0 { fill: red; }<![CDATA[ .st0 { ... } ]]>url()for gradients, patterns, filtersfill: url(#gradient1);<style>elements in one SVG<style>blocksfill,stroke,opacity,clip-rule, etc.❌ Not Supported (Intentionally)
#id)circle,rect)[attr="value"]):hover,:focus).parent .child,.parent > .child)@media,@keyframes)The hint focuses on static rendering scenarios with class selectors only, which covers the vast majority of SVG exports from design tools.
CSS Cascade & Conflict Resolution
When the same CSS property is defined in multiple places, the hint resolves conflicts according to standard CSS cascade rules:
styleattributeHow It Works
Given:
<circle class="A B C" style="x: 1">For any CSS property:
style→ if defined, use it ✅C→ if defined and no inline, use it ✅B→ if defined and not in C or inline, use it ✅A→ if defined and not in B, C, or inline, use it ✅Example
.base { fill: green; opacity: 0.5; stroke: purple; } .override { opacity: 0.9; } <circle class="base override" style="stroke: black">Property resolution:
fill: ✅.base(no conflict, only source)opacity: ✅.override(conflicts with.base, later class wins)stroke: ✅ inline style (conflicts with.base, inline wins)Result:
style="fill:green;opacity:0.9;stroke:black"Usage
Showcase Demo
ShowcaseIcons.sunnySVG with embedded CSS stylesRelease notes
New features
EmbeddedToInlineCssStyleSvgPatchHintpainter hint to support rendering SVG files with embedded CSS class selectors exported from vector graphics editors<style>blocks with.classNameselectors to inlinestyleattributes during SVG loadingNote
Adds
EmbeddedToInlineCssStyleSvgPatchHintto inline CSS class styles in SVGs, plus a showcase toggle and newsunnyicon, with comprehensive tests.EmbeddedToInlineCssStyleSvgPatchHintimplementingPainterSvgPatchHintto convert embedded CSS<style>rules (class selectors) into inlinestyleattributes and remove class/style blocks.platform/jewel/ui/api-dump-experimental.txt.EmbeddedToInlineCssStyleSvgPatchHintTestwith extensive cases covering parsing, cascade, multiple blocks, and no-op scenarios.ShowcaseIcons.sunnyand SVG assetplatform/jewel/samples/showcase/src/main/resources/icons/sunny.svg.components/Icons.ktto include a checkbox to enable the hint and render thesunnyicon withEmbeddedToInlineCssStyleSvgPatchHint.getSunny().Written by Cursor Bugbot for commit fcba5c1. This will update automatically on new commits. Configure here.