feat(plugin-chart-echarts): add Candlestick Chart - #43863
Conversation
This reverts commit 35aaeb6.
Code Review Agent Run #fe22c4Actionable Suggestions - 0Additional Suggestions - 4
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #43863 +/- ##
==========================================
+ Coverage 79.48% 79.50% +0.02%
==========================================
Files 2896 2903 +7
Lines 168323 168577 +254
Branches 39059 39154 +95
==========================================
+ Hits 133790 134028 +238
- Misses 32034 32050 +16
Partials 2499 2499
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Moving averages are shifted by one observation, and null placeholder collisions can silently merge or discard chart data.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an ECharts candlestick visualization with OHLC series, moving averages, formatting, legends, and zoom controls.
Changes:
- Registers the new
Candlestickvisualization. - Implements query building, controls, rendering, and transformation.
- Adds unit tests and chart imagery.
File summaries
| File | Description |
|---|---|
superset-frontend/src/visualizations/presets/MainPreset.ts |
Registers the chart plugin. |
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts |
Adds the visualization type. |
superset-frontend/plugins/plugin-chart-echarts/src/index.ts |
Exports the plugin and transformer. |
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx |
Registers ECharts candlestick support. |
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.test.tsx |
Updates the ECharts module mock. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/index.ts |
Defines plugin metadata and loading. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/EchartsCandlestick.tsx |
Renders the chart and handles legends. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/buildQuery.ts |
Builds OHLC queries. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/constants.ts |
Defines defaults and labels. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/controlPanel.tsx |
Defines chart controls. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/transformProps.ts |
Produces ECharts options and series. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/types.ts |
Defines chart types. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/utils.ts |
Implements moving-average utilities. |
superset-frontend/plugins/plugin-chart-echarts/test/Candlestick/buildQuery.test.ts |
Tests query construction. |
superset-frontend/plugins/plugin-chart-echarts/test/Candlestick/controlPanel.test.ts |
Tests control migration. |
superset-frontend/plugins/plugin-chart-echarts/test/Candlestick/transformProps.test.ts |
Tests chart transformation. |
superset-frontend/plugins/plugin-chart-echarts/test/Candlestick/utils.test.ts |
Tests moving-average utilities. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/images/thumbnail.png |
Adds light thumbnail. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/images/thumbnail-dark.png |
Adds dark thumbnail. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/images/example1.png |
Adds light example image. |
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/images/example1-dark.png |
Adds dark example image. |
Review details
Suppressed comments (1)
superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/transformProps.ts:297
- This similarly merges a null series value with an actual
"<NULL>"series value, causing their records to be combined or overwritten instead of rendered as distinct series. Use a distinct internal key for null and derive the legend display name separately.
data.map(datum =>
datum[seriesName] == null ? NULL_STRING : String(datum[seriesName]),
),
- Files reviewed: 17/21 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Review Agent Run #42c18cActionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 18 out of 22 changed files in this pull request and generated 7 comments.
| formData: { echartOptions: customEchartOptionsInput, ...rawFormData }, | ||
| hooks, | ||
| queriesData, | ||
| inContextMenu, | ||
| theme, | ||
| legendState = {}, | ||
| } = chartProps; | ||
| const formData = { | ||
| ...DEFAULT_FORM_DATA, | ||
| ...rawFormData, |
| const { | ||
| xAxis, | ||
| open, | ||
| close, | ||
| high, | ||
| low, | ||
| series: seriesControl, | ||
| increaseColor = DEFAULT_INCREASE_COLOR, | ||
| decreaseColor = DEFAULT_DECREASE_COLOR, | ||
| increaseLabel, | ||
| decreaseLabel, | ||
| showXAxis, | ||
| showYAxis, | ||
| xAxisTimeFormat, | ||
| xAxisTitle, | ||
| xAxisTitleMargin, | ||
| xAxisLabelRotation, | ||
| xAxisLabelInterval, |
| xAxis, | ||
| open, | ||
| close, | ||
| high, | ||
| low, | ||
| series: seriesControl, | ||
| increaseColor = DEFAULT_INCREASE_COLOR, | ||
| decreaseColor = DEFAULT_DECREASE_COLOR, | ||
| increaseLabel, | ||
| decreaseLabel, | ||
| showXAxis, | ||
| showYAxis, | ||
| xAxisTimeFormat, | ||
| xAxisTitle, | ||
| xAxisTitleMargin, | ||
| xAxisLabelRotation, | ||
| xAxisLabelInterval, | ||
| yAxisTitle, | ||
| yAxisTitleMargin, | ||
| yAxisTitlePosition, | ||
| yAxisFormat, | ||
| currencyFormat, | ||
| tooltipTimeFormat, | ||
| tooltipValuesFormat, | ||
| showLegend, | ||
| legendMargin, | ||
| legendOrientation = LegendOrientation.Top, | ||
| legendType = LegendType.Scroll, | ||
| legendSort, | ||
| zoomable, | ||
| movingAverages, |
| xAxis: QueryFormColumn; | ||
| open: QueryFormMetric; | ||
| close: QueryFormMetric; | ||
| high: QueryFormMetric; | ||
| low: QueryFormMetric; | ||
| series?: QueryFormColumn | QueryFormColumn[]; | ||
| increaseColor: RgbaColor; | ||
| decreaseColor: RgbaColor; | ||
| increaseLabel?: string; | ||
| decreaseLabel?: string; | ||
| showXAxis: boolean; | ||
| showYAxis: boolean; | ||
| xAxisTimeFormat?: string; | ||
| xAxisLabelRotation: number; | ||
| xAxisLabelInterval: string; | ||
| yAxisFormat: string; | ||
| tooltipTimeFormat?: string; | ||
| tooltipValuesFormat?: string; | ||
| zoomable: boolean; | ||
| movingAverages?: (number | string)[]; | ||
| echartOptions?: string; |
| width={width} | ||
| echartOptions={echartOptions} | ||
| eventHandlers={eventHandlers} | ||
| vizType={formData.vizType} |
| } | ||
|
|
||
| function toCategoryKey(value: unknown): string { | ||
| return value == null ? NULL_STRING : String(value); |
There was a problem hiding this comment.
This uses the display placeholder as the lookup key, so a database NULL and a literal "" category or series value collide. The Set/Map then discards one candle rather than rendering both. Could the internal key use a distinct null sentinel and only format the placeholder for display?
There was a problem hiding this comment.
In that case, I will separate the search key and the signature.
Code Review Agent Run #fbc0c9Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
SUMMARY
A new visualization type, the Candlestick Chart
implements the idea #20254 and #37653
The chart maps each data row to a candlestick in the format:
[open, close, low, high]. An optional Series parameter allows the data to be split into multiple candlestick series. Optional moving averages (MA5, MA10, MA15, etc., including custom periods) are displayed as smoothed lines based on closing prices (following the official ECharts candlestick chart example).Settings added for this chart include: titles, a legend, time and number formatting in tooltips, data zooming, X and Y axis visibility and formatting, colors for upward and downward movement, and
echart_options.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
run test
npm run test superset-frontend/plugins/plugin-chart-echarts/test/Candlestick
ADDITIONAL INFORMATION