feat(theme): enable generalized ECharts theme overrides for array properties#37965
Conversation
Code Review Agent Run #24379bActionable Suggestions - 0Additional Suggestions - 1
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 |
Sequence DiagramShows how the Echart component now uses a custom merge function to apply theme overrides — including applying an object override to every item of array-based ECharts options — and then sets the merged options on the chart. This is the core behavior introduced in the PR. sequenceDiagram
participant EchartComponent
participant ThemeMergeUtil as mergeEchartsThemeOverrides
participant ECharts as ChartInstance
EchartComponent->>ThemeMergeUtil: merge(baseTheme, echartOptions, globalOverrides, chartOverrides)
ThemeMergeUtil-->>EchartComponent: mergedOptions (object-to-array merge: object applied to each array item; arrays in source replace dest)
EchartComponent->>ECharts: setOption(mergedOptions, true)
ECharts-->>EchartComponent: render updated chart
Generated by CodeAnt AI |
cfc9e0a to
c8c4167
Compare
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #a4713bActionable 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 |
|
Yes, it's possible to define more precise types for the echartsOptionsOverrides and echartsOptionsOverridesByChartType values using ECharts' type definitions from the echarts library, such as EChartsOption or specific chart option interfaces, instead of 'any' for better type safety and IntelliSense support. |
Vitor-Avila
left a comment
There was a problem hiding this comment.
Worth addressing @SBIN2010's comment, but overall lgtm!
c8c4167 to
c2005e5
Compare
|
Should be all good if CI passes. |
Sequence DiagramThis PR enables theme authors to apply default styles to all array items (series, axes) using object syntax. The new merge function detects object-to-array merges and applies the object to each array item. sequenceDiagram
participant ThemeConfig
participant SupersetTheme
participant EchartComponent
participant mergeEchartsThemeOverrides
ThemeConfig->>SupersetTheme: Pass echartsOptionsOverrides (config fix)
EchartComponent->>mergeEchartsThemeOverrides: Merge baseTheme + chartOptions + overrides
mergeEchartsThemeOverrides->>mergeEchartsThemeOverrides: Detect object-to-array<br/>(e.g., series: {...} → series: [{...}, {...}])
mergeEchartsThemeOverrides->>mergeEchartsThemeOverrides: Apply object props to EACH array item
mergeEchartsThemeOverrides-->>EchartComponent: Themed ECharts options
Generated by CodeAnt AI |
…perties
Adds a custom merge function `mergeEchartsThemeOverrides` that handles
ECharts theme overrides with special array-to-object merge behavior:
1. Arrays in source values replace destination arrays entirely (backward compat)
2. When source is a plain object and destination is an array, the object
is merged into each array item (allowing default styles for all items)
This enables theme authors to write intuitive overrides like:
```js
echartsOptionsOverridesByChartType: {
echarts_bar: {
series: { itemStyle: { borderRadius: 4 } }, // Applied to ALL series
yAxis: { axisLabel: { rotate: 45 } } // Applied to ALL y-axes
}
}
```
Works for any array-based ECharts option: series, xAxis, yAxis, dataZoom,
visualMap, etc. - no need to handle each property specially.
Also fixes a bug where `echartsOptionsOverrides` and
`echartsOptionsOverridesByChartType` were not being passed through from
the theme config to the SupersetTheme object.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
c2005e5 to
8437101
Compare
Code Review Agent Run #04e0dbActionable 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 |
- theming.mdx: document ECharts array property overrides (PR #37965) — array values like color palettes are fully supported and replaced entirely (not merged); adds Array Property Overrides section with color palette example - configuring-superset.mdx: document PKCE support for database OAuth2 (PR #37067) — add PKCE section under Custom OAuth2 with code_challenge_method config and when to use it - cache.mdx: document ETag support for thumbnail/screenshot endpoints (PR #37663) — conditional GET with If-None-Match to avoid downloading unchanged images - exploring-data.mdx: document SQL Lab UX improvements (PRs #37298, #37694, #37756) — treeview table browser, Ctrl+F find widget, resizable panels; also adds time range natural language expressions section (PR #37098) - creating-your-first-dashboard.mdx: document Table chart features — boolean and categorical conditional formatting (PRs #36338, #37756), gradient toggle (PR #36280), HTML cell rendering with security note (PR #37685), column header tooltips from dataset descriptions (PR #37179), Display Controls modal in dashboard view (PR #36062) - databases.json: update StarRocks supports_catalog and supports_dynamic_catalog to true — the engine spec (PR #37026) already implemented full catalog support with get_catalog_names(), get_default_catalog(), and SHOW CATALOGS; the committed JSON was stale and did not reflect this Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…perties (apache#37965) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This PR adds a custom merge function for ECharts theme overrides that enables theme authors to apply default styles to all items in array-based ECharts options (like
series,xAxis,yAxis,dataZoom, etc.) using a simple object syntax.The Problem
Previously,
mergeReplaceArrayswould replace arrays entirely. This made it impossible to set default styles for all series items via theme overrides:The only workaround was to replace the entire series array, which would break the chart since you'd lose all the actual data.
The Solution
The new
mergeEchartsThemeOverridesfunction detects when you're merging an object into an array and applies the object to each array item:What's Now Possible
series: { itemStyle: { borderRadius: 4 } }series: { label: { show: true, fontSize: 12 } }xAxis: { axisLabel: { rotate: 45 } }yAxis: { splitLine: { show: false } }dataZoom: { filterMode: 'filter' }Backward Compatibility
Array overrides still replace entirely (existing behavior preserved):
Test plan
🤖 Generated with Claude Code