feat: add Highcharts pattern-fill support to vaadin-chart

[DiegoCardoso]

Highcharts pattern fills let series use SVG patterns instead of solid colors, which helps color-blind users tell series apart. The pattern-fill module ships with Highcharts (~8 KB) but was not enabled in <vaadin-chart>, and there was no way to use it.

• Register the Highcharts pattern-fill module.
• <vaadin-chart> runs Highcharts in styled mode, where color options (including patterns) are not applied directly — fills come from CSS classes. A new PatternFillBridge (vaadin-chart-pattern-fill.js) turns color: { pattern: {...} } / { patternIndex: N } into a rendered pattern: it creates the <pattern> def and injects one shadow-scoped CSS rule per color index, so the point, its legend symbol and its tooltip swatch all get the pattern. A point whose pattern differs from its series uses a fill attribute.
• In non-styled mode Highcharts renders patterns itself, so the bridge does nothing.
• Theme color rules in the base styles now skip elements that already have a pattern url() fill, so they don't override it.

Known limitation (follow-up): server-side exportChart builds a fresh chart the bridge does not run on, so server-rendered SVGs don't include these patterns yet.

Part of #9990

🤖 Generated with Claude Code

[vaadin-review-bot]

Reviewed the changes — left 8 comments.

	Finding
⚠️	Adding :not([fill^='url('] to the color rules raises their specificity and makes point markers lose their background-colored outline.
⚠️	A theme re-injection wipes the bridge's pattern stylesheet from adoptedStyleSheets, so pattern fills vanish until the next redraw.
⚠️	Removing all patterns strands a point's fill="url(#id)" attribute pointing at a destroyed def, so the point renders with no fill.
⚠️	colorIndexRules is keyed only by color index, so two series sharing a color index render the same pattern.
⚠️	The bridge's stylesheet is never removed on teardown, leaking across reconnects and leaving dangling url() fills.
⚠️	hash.toString(16).replace('-', '1') maps a negative and a positive hash to the same id, silently sharing one pattern.
👀	Tests assert on chart.__patternSheet, a property that is never set, so the assertions pass vacuously.
⚡	apply() re-scans every series and point and reparses the CSS on every render with no dirty check.

[vaadin-review-bot]

Around line 481 — could not attach to the exact diff line.

⚠️ Adding :not([fill^='url('] to the color rules raises their specificity and makes point markers lose their background-colored outline.

The 10 .highcharts-color-N rules went from specificity (0,1,0) to (0,2,0) (the [fill^='url('] attribute selector inside :not() adds a class-level weight). The later .highcharts-markers { stroke: var(--_bg) } rule at line 481 is only (0,1,0) and used to win by source order, giving markers a background-colored border. It now loses to the color rule, so the markers group's stroke becomes the series color and marker outlines disappear.

This hits every styled-mode line, scatter, or spline chart with markers — not just charts using patterns — so it is the widest-reaching regression here. The author already hit the same mechanism for .highcharts-data-label-connector and added the :not([fill^='url('] guard there (see the comment at line 517); the .highcharts-markers rule needs an equivalent specificity bump to keep winning.

_{vaadin-chart-base-styles.js:481 · correctness · confirmed}

[DiegoCardoso]

Fixed in dc0af88. The per-point fallback now sets an inline style.fill (which wins over the theme rule regardless of specificity), so the :not([fill^='url(']) guards were removed from all 10 .highcharts-color-N rules and the connector. Markers/connector win on source order again; base/lumo/aura visual suites pass with no diffs.

[vaadin-review-bot]

⚠️ A theme re-injection wipes the bridge's pattern stylesheet from adoptedStyleSheets, so pattern fills vanish until the next redraw.

The bridge appends #patternSheet to shadowRoot.adoptedStyleSheets. <vaadin-chart> mixes in LumoInjectionMixin, whose applyInstanceStyles calls Lit's adoptStyles(shadowRoot, getEffectiveStyles(...)), reassigning adoptedStyleSheets to a freshly computed list of base/lumo/theme styles that does not include #patternSheet. This fires at runtime whenever Lumo styles are added or removed from the document (theme switch, dynamic registerStyles), driven by --_lumo-vaadin-chart-inject — independent of any chart redraw.

After such an injection with no following redraw, the .highcharts-color-N { fill: url(...) } rules are gone and all patterned series fall back to solid theme color. The sheet is only re-added inside apply(), so it stays lost until the next render. The bridge and the theme injector both write adoptedStyleSheets with no coordination; the pattern sheet needs to be re-asserted after theme injection (or added through the same mechanism).

_{vaadin-chart-pattern-fill.js:213 · correctness · confirmed}

[DiegoCardoso]

Fixed in dc0af88. Pattern rules moved from a constructable sheet in adoptedStyleSheets (which adoptStyles replaces wholesale on Lumo injection) into a <style data-vaadin-pattern-fill> element in the shadow root, which the injector never touches. prepareExport now also copies :scope > style so exported SVGs keep the patterns.

[vaadin-review-bot]

⚠️ Removing all patterns strands a point's fill="url(#id)" attribute pointing at a destroyed def, so the point renders with no fill.

Configure a per-point pattern (the point gets fill="url(#id)" set by #applyPointPatternFill), then call updateConfiguration to remove all patterns. hasPatterns is now false but #patternIds is non-empty, so the early return at line 75 does not fire. The if (hasPatterns) block at line 83 is skipped, so the stale-fill clearing branch (graphic.removeAttribute('fill'), the only such call in the package) never runs — yet #cleanupPatternDefs still destroys the def.

The point keeps fill="url(#id)" pointing at a now-deleted <pattern>, and because that attribute starts with url( the base rule .highcharts-color-N:not([fill^='url('] excludes it from the theme color too. The point renders with a broken/empty fill instead of returning to its series color. The point-fill cleanup must run even when hasPatterns is false.

_{vaadin-chart-pattern-fill.js:75 · correctness · confirmed}

[DiegoCardoso]

Fixed in dc0af88. apply() now clears a stale pattern fill on every point on every render, even when no patterns remain, so removing all patterns no longer strands a url() fill on a destroyed def. Added a regression test.

[vaadin-review-bot]

⚠️ colorIndexRules is keyed only by color index, so two series sharing a color index render the same pattern.

Highcharts recycles color indices 0–9 and wraps, and users can set colorIndex explicitly. When two patterned series share a color index (e.g. an 11th series wrapping to index 0, or two series both set to colorIndex: 0), colorIndexRules.set(colorIndex, seriesPatternId) overwrites the first entry. The single injected rule .highcharts-color-0 { fill: url(#lastId) } then applies to both series, and neither gets the per-point fill fallback (that only triggers when a point differs from its own series).

Both series render the last one's pattern instead of their own, defeating the color-blind differentiation the feature exists for. A class keyed on color index cannot distinguish two series that share it; the rule needs to key on something series-unique (while still covering the legend and tooltip swatch).

_{vaadin-chart-pattern-fill.js:92 · correctness · plausible}

[DiegoCardoso]

Documented as a known limitation in dc0af88 (code comment where the rules are built) rather than fixed: keying on .highcharts-color-N is what lets one rule also cover the legend symbol and tooltip swatch, and a collision needs >10 patterned series or an explicit duplicate colorIndex. Happy to revisit if you'd prefer a series-unique mechanism.

[vaadin-review-bot]

⚠️ The bridge's stylesheet is never removed on teardown, leaking across reconnects and leaving dangling url() fills.

Each __initChart (reached on lazy attach, updateConfiguration(..., true), and reconnect after disconnect) creates a new PatternFillBridge with a fresh #patternSheet and appends it to shadowRoot.adoptedStyleSheets. Nothing removes the old sheet — configuration.destroy() tears down the Highcharts SVG and its render listener but not the constructable sheet, so the comment at line 725 ("Teardown rides on configuration.destroy()") does not hold for the sheet.

Old sheets accumulate on adoptedStyleSheets (each keeps its dead bridge alive), and if a chart is reconnected with a plain, no-pattern config, the leftover rule .highcharts-color-0 { fill: url(#vaadin-pattern-OLD) } still matches while the def is gone — the new points resolve to a dangling url() and render with no fill. The sheet should be dropped when the chart is destroyed.

_{vaadin-chart-mixin.js:727 · correctness · plausible}

[DiegoCardoso]

Fixed in dc0af88. Added PatternFillBridge.destroy() (removes the <style> element and destroys its tracked defs); called from disconnectedCallback in the real-disconnect block, and the bridge reference is nulled.

[vaadin-review-bot]

⚠️ hash.toString(16).replace('-', '1') maps a negative and a positive hash to the same id, silently sharing one pattern.

hash &= hash yields a signed 32-bit int, so a negative value stringifies with a leading -. Replacing that - with '1' collides with a positive hash: e.g. -21 → "-15" → "115", while 277 (0x115) → "115". Two different pattern configs then both resolve to vaadin-pattern-115.

renderer.addPattern({...pattern, id}) is a no-op when the id already exists, so the second series or point silently renders the first pattern's shape and color. Compute the hex from the unsigned value (e.g. (hash >>> 0).toString(16)) so the sign never has to be rewritten.

_{vaadin-chart-pattern-fill.js:283 · correctness · plausible}

[DiegoCardoso]

Fixed in dc0af88: (hash >>> 0).toString(16) (unsigned), dropping the .replace('-','1'). Added a test that two distinct configs yield distinct ids.

[vaadin-review-bot]

👀 Tests assert on chart.__patternSheet, a property that is never set, so the assertions pass vacuously.

The injected stylesheet lives in the bridge's private #patternSheet field; nothing ever assigns chart.__patternSheet. The assertions expect(chart.__patternSheet).to.not.exist at lines 102 and 238 are therefore always true regardless of behavior.

The "non-styled mode" and "without patterns" tests intend to prove no stylesheet was injected, but a regression that wrongly injects one would still pass. The tests need to check the actual sheet — for example that shadowRoot.adoptedStyleSheets gained no rule referencing a vaadin-pattern- fill.

_{pattern-fill.test.js:102 · test-coverage · confirmed}

[DiegoCardoso]

Fixed in dc0af88. The field is private now; tests assert the actual injected <style data-vaadin-pattern-fill> rules instead of chart.__patternSheet — a positive assertion for the styled case and no vaadin-pattern- rule for the non-styled/no-pattern cases.

[vaadin-review-bot]

⚡ apply() re-scans every series and point and reparses the CSS on every render with no dirty check.

apply() runs on every Highcharts render — including hover-, zoom-, resize-, and animation-driven redraws. On each call it walks all series and points once for the hasPatterns check and again in the main forEach, re-runs addPattern + querySelector('path') + inline style writes for every patterned def, and calls replaceSync (a full CSS reparse) even when the pattern config is byte-identical to the previous render.

For a large chart redrawing on hover this is O(points) work plus repeated DOM writes many times per second for no change in output. Cache a cheap signature of the pattern-relevant inputs (color-index-to-id map plus per-point override ids) and return early when it matches the last render.

_{vaadin-chart-pattern-fill.js:55 · efficiency · plausible}

[DiegoCardoso]

Acknowledged — deferred as a follow-up. Prioritized the correctness findings in this round; a dirty-check (signature of colorIndex→id + per-point override ids) is a reasonable next optimization.

[DiegoCardoso]

Now addressed in ce0441d (had marked this deferred earlier). Two safe wins: #rebuildPatternSheet skips the textContent write (and its CSS re-parse) when the rule text is unchanged, and #ensurePatternDef styles each def's path only once (skips the querySelector + inline writes on later renders; the flag is cleared if the def is destroyed). I intentionally did NOT add a full walk-skipping early-return: per-point fallback fills are inline styles on point graphics that Highcharts can recreate on a data-only updateConfiguration (same pattern signature), so the walk + per-point re-stamp must keep running each render — only the redundant CSS re-parse and def re-styling are cached away. Added a regression test for a def re-added with the same id after removal.

[DiegoCardoso]

Highcharts doesn't support pattern fills through options when the chart is in styled mode. Instead, users are expected to define the SVG pattern structure and apply it via CSS (source). The purpose of this class is to allow charts in styled mode to define patterns through the options object by reading it and doing the following:

1. Creating the SVG <pattern> element described in the pattern object via the configuration.renderer.addPattern method, or retrieving the pattern object if a patternIndex is defined instead.
2. Applying the <pattern> ID to the point objects' styles using the CSS url() function.

This implementation deviates from how charts in styled mode are usually styled, as it selectively allows patterns to be defined through the options API. I decided to take this approach because vaadin-charts doesn't allow light DOM styling, and shadow DOM styling is no longer recommended for Vaadin applications—meaning the pattern fill feature would otherwise only be available in non-styled mode.

Another important aspect of this module is that it relies on private Highcharts methods, making it somewhat fragile to changes in their internal implementation.

[DiegoCardoso]

/code-review

[vaadin-review-bot]

Reviewed the changes — left 4 comments.

	Finding
⚡	apply() re-runs JSON.stringify and the hash for every patterned point on every render, then throws the result away.
⚠️	Two patterns that share an explicit pattern.id but differ in color silently both render the first color.
⚠️	The render listener calls this.__patternFillBridge.apply() with no optional chaining, unlike every other bridge access.
🧹	#stripInternalKeys is a 9-line helper for a one-line key filter used at a single call site.

[vaadin-review-bot]

⚡ apply() re-runs JSON.stringify and the hash for every patterned point on every render, then throws the result away.

A chart with per-point pattern colors fires render on every redraw — resize, zoom, and each shared-tooltip hover move. For each render, #ensurePatternDef runs #stripInternalKeys (a fresh object plus an Object.keys array), a { ...stripped, color } spread, JSON.stringify, and the char-by-char hash for every patterned entity, before the #patternIds.has(id) gate on line 169 discards all of it because the def already exists. With hundreds of per-point patterns this is steady per-frame allocation and CPU during interaction.

The id only depends on the pattern config, which does not change between renders. Memoize it on the color-options object (e.g. a WeakMap<colorOptions, id>) so the stringify and hash run once per distinct config instead of once per patterned point per render.

_{vaadin-chart-pattern-fill.js:163 · efficiency · confirmed}

[vaadin-review-bot]

⚠️ Two patterns that share an explicit pattern.id but differ in color silently both render the first color.

Two series use color: { pattern: { id: 'stripes', path, color: '#ff0000' } } and color: { pattern: { id: 'stripes', path, color: '#0000ff' } }. Both resolve id to "stripes". The first call creates and styles the def red; the second sees id already in #patternIds, so the if (!this.#patternIds.has(id)) gate skips both addPattern and the path re-styling. Both series' injected .highcharts-color-N rules point at the one red def, so the second series renders red instead of blue with no warning.

Explicit pattern.id is a supported option (the removing a series test relies on my-custom-pattern). If shared ids are meant to share one pattern, the divergent color should at least not be silently dropped; otherwise the gate must re-style when the resolved color changes.

_{vaadin-chart-pattern-fill.js:169 · correctness · plausible}

[DiegoCardoso]

This matches Highcharts' own behavior, so I documented the contract rather than changing it (c026e56). Native SVGRenderer#addPattern is first-wins for a shared explicit id: if (this.defIds.indexOf(id) > -1) { return; } — the second pattern with the same id is a no-op, and everything referencing that id uses the first def (styling only runs on creation, so non-styled mode is first-wins too). A single <pattern id> has one path/one color, so two series sharing an id physically cannot render two colors; re-styling would just make it last-wins with per-render churn. Without an explicit id the content hash (incl. color) gives distinct patterns distinct defs. Added a comment at the id gate noting an explicit pattern.id identifies one shared def — use distinct ids for distinct patterns.

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud