feat(charts): export chart as SVG - #720
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughAdds ECharts SVG rendering and chart→SVG serialization, a triggerSvgDownload helper, and a conditional "Export SVG" widget action in DashboardContainer with corresponding tests and test-mock updates. ChangesSVG Chart Export
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardContainer
participant exportWidgetSvg
participant exportChartToSvg
participant triggerSvgDownload
User->>DashboardContainer: click Export SVG
DashboardContainer->>exportWidgetSvg: widget
exportWidgetSvg->>exportChartToSvg: chart container
exportChartToSvg->>exportWidgetSvg: svg string
exportWidgetSvg->>triggerSvgDownload: svg string, filename
triggerSvgDownload->>User: starts download
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@component/src/charts/base-chart.tsx`:
- Around line 290-327: In exportChartToSvg, ensure the SVG-mode ECharts instance
(svgInstance) is always disposed to avoid leaks: move creation of svgInstance
and any calls that can throw (echarts.init and svgInstance.setOption) into a try
block and call svgInstance.dispose() in a finally block (or track svgInstance
and dispose it in the existing finally before removing the offscreen element);
reference svgInstance and the exportChartToSvg function to locate where to add
the guaranteed dispose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 640b1ea0-16e6-4493-aaae-a9728b296873
📒 Files selected for processing (8)
app/src/components/dashboard-container.tsxcomponent/src/charts/__tests__/echarts-mock.tscomponent/src/charts/base-chart.tsxcomponent/src/charts/index.tscomponent/src/lib/__tests__/export-utils.test.tscomponent/src/lib/export-utils.tscomponent/src/utils/index.tscomponent/vitest.setup.ts
| function exportChartToSvg(container: HTMLElement): string | null { | ||
| const instance = echarts.getInstanceByDom(container); | ||
| if (!instance) return null; | ||
|
|
||
| const options = instance.getOption(); | ||
| const { width, height } = instance.getDom().getBoundingClientRect(); | ||
|
|
||
| // Create an offscreen container for the SVG renderer | ||
| const offscreen = document.createElement("div"); | ||
| offscreen.style.width = `${width}px`; | ||
| offscreen.style.height = `${height}px`; | ||
| offscreen.style.position = "absolute"; | ||
| offscreen.style.left = "-9999px"; | ||
| document.body.appendChild(offscreen); | ||
|
|
||
| try { | ||
| const themeName = isDarkMode() ? THEME_DARK : THEME_LIGHT; | ||
| const svgInstance = echarts.init(offscreen, themeName, { | ||
| renderer: "svg", | ||
| width, | ||
| height, | ||
| }); | ||
| svgInstance.setOption(options); | ||
|
|
||
| // Extract the rendered SVG from the offscreen container | ||
| const svgEl = offscreen.querySelector("svg"); | ||
| if (!svgEl) { | ||
| svgInstance.dispose(); | ||
| return null; | ||
| } | ||
|
|
||
| const svgString = new XMLSerializer().serializeToString(svgEl); | ||
| svgInstance.dispose(); | ||
| return svgString; | ||
| } finally { | ||
| document.body.removeChild(offscreen); | ||
| } | ||
| } |
There was a problem hiding this comment.
Dispose the ECharts instance in all code paths to prevent memory leaks.
If svgInstance.setOption(options) (line 312) or any subsequent operation throws an exception, svgInstance.dispose() won't be called, leaving the ECharts instance alive in memory. Each failed export attempt will leak an instance.
🔧 Proposed fix to ensure disposal in all paths
try {
const themeName = isDarkMode() ? THEME_DARK : THEME_LIGHT;
const svgInstance = echarts.init(offscreen, themeName, {
renderer: "svg",
width,
height,
});
- svgInstance.setOption(options);
-
- // Extract the rendered SVG from the offscreen container
- const svgEl = offscreen.querySelector("svg");
- if (!svgEl) {
+ try {
+ svgInstance.setOption(options);
+
+ // Extract the rendered SVG from the offscreen container
+ const svgEl = offscreen.querySelector("svg");
+ if (!svgEl) {
+ return null;
+ }
+
+ return new XMLSerializer().serializeToString(svgEl);
+ } finally {
svgInstance.dispose();
- return null;
}
-
- const svgString = new XMLSerializer().serializeToString(svgEl);
- svgInstance.dispose();
- return svgString;
} finally {
document.body.removeChild(offscreen);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@component/src/charts/base-chart.tsx` around lines 290 - 327, In
exportChartToSvg, ensure the SVG-mode ECharts instance (svgInstance) is always
disposed to avoid leaks: move creation of svgInstance and any calls that can
throw (echarts.init and svgInstance.setOption) into a try block and call
svgInstance.dispose() in a finally block (or track svgInstance and dispose it in
the existing finally before removing the offscreen element); reference
svgInstance and the exportChartToSvg function to locate where to add the
guaranteed dispose.
defa723 to
8e4b989
Compare
- Add "Export SVG" option to widget actions menu for ECharts-based charts - Register SVG renderer alongside Canvas in base-chart - exportChartToSvg() creates offscreen instance with SVG renderer, copies current chart options, serializes SVG, triggers download - Only appears for widgets with isECharts capability (bar, line, pie, gauge, radar, sankey, treemap, sunburst, single-value) - Add triggerSvgDownload() helper to export-utils Closes #706 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8e4b989 to
83c9b03
Compare
|


Summary
Add "Export SVG" option to the widget actions dropdown for ECharts-based charts. Creates a vector SVG file download — resolution-independent, perfect for docs, presentations, and embedding.
How it works
isECharts: truecapabilitySupported chart types
Bar, Line, Pie, Gauge, Radar, Sankey, Treemap, Sunburst, Single Value
Test plan
Closes #706
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests