feat(dashboard-v2): theme a building block's chart by msyavuz · Pull Request #43070 · apache/superset

msyavuz · 2026-08-11T17:29:09Z

SUMMARY

Targets dashboard-v2, on top of the containers work merged in #43065.

A Dashboard v2 block draws its ECharts option on a bare canvas, bypassing SuperChart/ChartPlugin — so nothing applied the theme to it. Three consequences, fixed here:

A chart ignored the theme. It rendered ECharts' stock near-black text and default blue palette whatever theme was active. getEchartsTheme moves out of Echart.tsx's inline closure into utils/echartsTheme.ts so something other than that one component can reach it, and ChartBlock merges it under the authored option — an explicit choice in the spec still wins. Axis styling applies only when the option declares an axis, so it no longer draws a cartesian grid onto a pie.
A block had no way to know what the theme means. The only affordance was getCategoricalColors(), so each contributed block re-derived the rest from raw antd tokens — every extension its own mapping, all destined to drift. dashboard.getChartTheme() states it once, semantically (background, text, axis, tooltip, accent, categorical + sequential colours); a renderer maps those few fields and merges its own spec over the result. It also exposes Superset's sequential schemes, which nothing was exposing at all, so any continuous colour fell back to the library's default blues.

getColor(label) colours a series by name through the scale that remembers what it gave a label — the same one v1 charts use. By position, a category is one colour in a chart that lists it second and another in a chart that lists it fifth, which is what makes a set of blocks read as unrelated charts rather than one dashboard.
A themed deployment didn't reach a canvas. The card used colorBgContainer/colorBorderSecondary/borderRadiusLG — close to v1's dashboard tile but not it. v1 reads three dashboardTile* override points, so a deployment that themes its tiles had no effect here at all. Same tokens now, same fallbacks.

Also adds per-series-type theming, which a global textStyle cannot reach: v1 is theme-correct because ~40 transformProps files each know which of their chart's elements are themeable, and a generic option has no viz_type to look that up by. It does carry series[].type, so the same thing is derived from the option — which is what stops an AI-authored pie drawing a white halo around every label (zrender's useDefaultFill stroke when no label colour is set).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — the visible change is charts matching the active theme rather than ECharts' defaults; no layout or chrome changes beyond the card's token source.

TESTING INSTRUCTIONS

Open a v2 dashboard and add an echarts block with a dataBinding and a bar or pie option that sets no colours.
Confirm text, axes, legend and tooltip follow the active theme, and that the series takes a colour from the active categorical scheme rather than ECharts' blue.
Add a second chart whose series has the same name; confirm both draw it in the same colour.
Switch light/dark and confirm both follow without a reload.
Set colorScheme on the root node; confirm charts pick up that palette.
With a theme defining dashboardTileBg/dashboardTileBorder/dashboardTileBorderRadius, confirm a block's card matches a v1 dashboard tile.
npm run test -- src/core/dashboard — chartTheme, echartsSeriesDefaults and resolveBindings suites.

ADDITIONAL INFORMATION

Has associated issue: No
Required feature flags: None
Changes UI — charts inside v2 building blocks follow the theme; the block card reads v1's dashboardTile* tokens
Includes DB Migration: No — frontend only
Introduces new feature or API — dashboard.getChartTheme() on the extension API; getEchartsTheme/mergeEchartsThemeOverrides newly exported from plugin-chart-echarts
Removes existing feature or API: No — getCategoricalColors() stays, though getChartTheme().categoricalColors supersedes it

A building block draws its ECharts option on a bare canvas, bypassing SuperChart/ChartPlugin entirely, so nothing was applying the theme to it: a chart rendered ECharts' stock near-black text and default blue palette whatever theme the deployment was running. `getEchartsTheme` comes out of `Echart.tsx`'s inline closure into `utils/echartsTheme.ts` so it is reachable by something other than that one component, and axis styling is applied only when the option actually declares an axis — otherwise it draws a cartesian grid onto a pie. `ChartBlock` merges it, the categorical palette and the theme's own ECharts overrides *under* the authored option, so anything the spec sets explicitly still wins.

Chrome consistency is the shallow half of the problem. The deeper one is that a block's *contents* do not follow the theme: every charting library ships its own palette and its own near-black text, so a block looks like its library rather than like Superset, and two blocks on one dashboard disagree about what "the first series" is. The only affordance a contributed block had was `getCategoricalColors()`, so each one re-derived the rest from raw tokens — every extension a different mapping of what a theme *means*, all destined to drift. Theme compatibility should be what a block starts from, not something its author remembers to implement. `dashboard.getChartTheme()` states it once, semantically: background, text, axis, tooltip, accent, categorical and sequential colours. A renderer maps those few fields onto its own config and merges its own spec *over* the result, so a block that genuinely wants different colours still says so while consistency is the default. Includes Superset's sequential schemes, which nothing was exposing at all — any continuous colour a block drew fell back to its library's default blues. `getColor(label)` is how a series should get its colour: by name, through the scale that remembers what it gave a label, which is also how the v1 charts beside a canvas resolve theirs. By position, a category is one colour in a chart that lists it second and another in a chart that lists it fifth. A `theme` bind resolves against this vocabulary rather than the raw token bag, falling back to token names so options already authored against them keep rendering.

Two things a canvas could not inherit from the deployment it runs in. The card is drawn from `colorBgContainer`/`colorBorderSecondary`/ `borderRadiusLG`, which is close to v1's dashboard tile but not it. v1 reads three `dashboardTile*` theme override points, and a deployment that themes its dashboard tiles was having no effect whatsoever on a canvas — so a v2 dashboard in a customised deployment looked like a different product. Reading the same tokens means v2 inherits that customisation for free. Per-series-type theming is what a global `textStyle` cannot reach. v1 charts are theme-correct because each of ~40 `transformProps` files knows which of *its* chart's elements are themeable; a generic option has no `viz_type` to look that up by, which is why an AI-authored pie draws a white halo around every label — with no explicit label colour, zrender falls back to its own contrasting stroke. The option already says what it is (`series[].type`), so the same thing is derived from the option itself. Applied after the theme merge and only where the author said nothing, since a source array replaces the destination's in a merge and would drop whatever the theme layer contributed. That pass is also where a series takes its colour from its name.

bito-code-review · 2026-08-11T17:29:20Z

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

netlify · 2026-08-11T17:31:15Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`1d1af64`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a7b5be974dc030008ffc1bc
😎 Deploy Preview	https://deploy-preview-43070--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

codeant-ai-for-open-source · 2026-08-11T17:33:47Z

+  const scale = CategoricalColorNamespace.getScale(scheme);
+  return {
+    background: 'transparent',
+    text: {
+      color: theme.colorText,
+      mutedColor: theme.colorTextSecondary,
+      disabledColor: theme.colorTextDisabled,
+      fontFamily: theme.fontFamily,
+      fontSize: theme.fontSize,
+    },
+    axis: {
+      lineColor: theme.colorSplit,
+      labelColor: theme.colorTextSecondary,
+      gridColor: theme.colorSplit,
+      minorGridColor: theme.colorBorderSecondary,
+    },
+    tooltip: {
+      background: theme.colorBgContainer,
+      color: theme.colorText,
+    },
+    accent: theme.colorPrimary,
+    categoricalColors: scale.colors,
+    getColor: (label: string) => scale.getColor(label),
+    sequentialColors: getSequentialColors(),


Suggestion: The scale is created without a slice identifier, so calls to scale.getColor(label) record the mapping only in that scale's private chartLabelsColorMap; CategoricalColorScale adds labels to the shared dashboard map only when a truthy sliceId is supplied. Because getChartTheme creates a fresh scale for every block, the same label can receive different colors depending on each block's series order, contradicting the documented cross-block consistency. Use the shared namespace color API or otherwise provide a stable dashboard-level mapping rather than creating independent unregistered scales. [cache]

Severity Level: Major ⚠️

- ⚠️ Repeated categories receive inconsistent colors across dashboard blocks. - ⚠️ ECharts series colors vary with authored series order. - ⚠️ Cross-block visual comparison becomes misleading for shared labels.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/src/core/dashboard/chartTheme.ts **Line:** 125:148 **Comment:** *Cache: The scale is created without a slice identifier, so calls to `scale.getColor(label)` record the mapping only in that scale's private `chartLabelsColorMap`; `CategoricalColorScale` adds labels to the shared dashboard map only when a truthy `sliceId` is supplied. Because `getChartTheme` creates a fresh scale for every block, the same label can receive different colors depending on each block's series order, contradicting the documented cross-block consistency. Use the shared namespace color API or otherwise provide a stable dashboard-level mapping rather than creating independent unregistered scales. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

codeant-ai-for-open-source · 2026-08-11T17:34:03Z

+  // module is imported.
+  getCategoricalColors: () =>
+    CategoricalColorNamespace.getScale(canvasColorScheme()).colors,
+  getChartTheme: () => getChartTheme(themeObject.theme, canvasColorScheme()),


Suggestion: The public API derives its theme from the module-level themeObject, while mounted dashboard blocks derive it from React's useTheme() context. When the application uses a different ThemeController or provider theme, dashboard.getChartTheme() and the rendered block can return different colors and typography after a theme change. Resolve the API through the same active theme source used by the provider, or ensure the provider updates this exact themeObject. [api mismatch]

Severity Level: Major ⚠️

- ⚠️ Dashboard-specific themes can disagree with block API values. - ⚠️ Extensions may render inconsistent chart colors and typography. - ⚠️ Theme changes can produce mismatched canvas and block styling.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/src/core/dashboard/index.ts **Line:** 77:77 **Comment:** *Api Mismatch: The public API derives its theme from the module-level `themeObject`, while mounted dashboard blocks derive it from React's `useTheme()` context. When the application uses a different `ThemeController` or provider theme, `dashboard.getChartTheme()` and the rendered block can return different colors and typography after a theme change. Resolve the API through the same active theme source used by the provider, or ensure the provider updates this exact `themeObject`. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

codeant-ai-for-open-source · 2026-08-11T17:34:05Z

+     * `accent`, `text.mutedColor`, `axis.gridColor`.
+     */
    token?: string;
    /**


Suggestion: The legacy fallback reads the raw theme through normal property access, so tokens such as constructor or toString resolve inherited Object.prototype members instead of throwing the documented “not a chart theme field” error. This can inject functions or prototype objects into authored chart options. Restrict both legacy and dotted-path lookups to own properties. [api mismatch]

Severity Level: Minor 🧹

- ⚠️ Invalid theme binds bypass the documented validation error. - ⚠️ ECharts options can receive inherited functions or objects. - ⚠️ Malformed authored charts may fail during rendering.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/src/core/dashboard/resolveBindings.ts **Line:** 51:54 **Comment:** *Api Mismatch: The legacy fallback reads the raw theme through normal property access, so tokens such as `constructor` or `toString` resolve inherited `Object.prototype` members instead of throwing the documented “not a chart theme field” error. This can inject functions or prototype objects into authored chart options. Restrict both legacy and dotted-path lookups to own properties. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

bito-code-review · 2026-08-11T17:34:08Z

The flagged issue is correct. In superset-frontend/src/core/dashboard/chartTheme.ts, getChartTheme creates a new CategoricalColorScale instance without a sliceId for every block. Because CategoricalColorScale only registers labels in the shared dashboard-level map when a sliceId is provided, these independent scales do not share color mappings, leading to inconsistent colors for the same labels across different dashboard blocks.

To resolve this, you should ensure that the scale is associated with a stable dashboard-level identifier. Since getChartTheme is intended to be used across the dashboard, you can pass a consistent identifier (such as a dashboard-level namespace or a shared sliceId if applicable) to CategoricalColorNamespace.getScale(scheme) to ensure the scale instance is shared or correctly scoped.

Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?

superset-frontend/src/core/dashboard/chartTheme.ts

export function getChartTheme(theme: Theme, scheme?: string): ChartTheme {
  // Use a consistent dashboard-level identifier instead of an unregistered scale
  const scale = CategoricalColorNamespace.getScale(scheme, 'dashboard-shared-namespace');
  return {
    // ... rest of the implementation
    getColor: (label: string) => scale.getColor(label),
    // ...
  };
}

codeant-ai-for-open-source · 2026-08-11T17:34:16Z

+  if (options?.xAxis) {
+    echartsTheme.xAxis = axisTheme;
+  }
+  if (options?.yAxis) {
+    echartsTheme.yAxis = axisTheme;
+  }


Suggestion: When xAxis or yAxis is authored in ECharts' valid array form, this helper still returns a single object axis override. mergeEchartsThemeOverrides replaces that object with the authored array, so none of the theme's axis line, label, or grid styling reaches any array axis. Convert the theme override to an array-compatible per-axis default before merging. [logic error]

Severity Level: Major ⚠️

- ⚠️ Multi-axis building blocks lose themed axis styling. - ⚠️ Axis labels and grid lines use ECharts defaults. - ⚠️ Dashboard charts can look inconsistent across axis forms.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/echartsTheme.ts **Line:** 74:79 **Comment:** *Logic Error: When `xAxis` or `yAxis` is authored in ECharts' valid array form, this helper still returns a single object axis override. `mergeEchartsThemeOverrides` replaces that object with the authored array, so none of the theme's axis line, label, or grid styling reaches any array axis. Convert the theme override to an array-compatible per-axis default before merging. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

codeant-ai-for-open-source · 2026-08-11T17:34:16Z

+      getChartTheme(
+        theme,
+        typeof colorScheme === 'string' ? colorScheme : undefined,


Suggestion: An empty string is passed as a color-scheme name instead of being normalized to undefined. The shared dashboard implementation explicitly treats an empty root colorScheme as unset and uses the deployment default, whereas CategoricalColorNamespace looks up the empty key and returns an empty palette. A saved or edited block with colorScheme: '' can therefore lose its categorical colors and produce undefined color assignments. [logic error]

Severity Level: Major ⚠️

- ⚠️ Saved blocks can lose categorical chart colors. - ⚠️ Named series receive undefined item colors. - ⚠️ Empty schemes bypass the deployment default palette.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/src/core/dashboard/blocks/ChartBlock.tsx **Line:** 235:237 **Comment:** *Logic Error: An empty string is passed as a color-scheme name instead of being normalized to `undefined`. The shared dashboard implementation explicitly treats an empty root `colorScheme` as unset and uses the deployment default, whereas `CategoricalColorNamespace` looks up the empty key and returns an empty palette. A saved or edited block with `colorScheme: ''` can therefore lose its categorical colors and produce undefined color assignments. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

codeant-ai-for-open-source · 2026-08-11T17:34:16Z

+      withoutTitle,
+      theme.echartsOptionsOverrides ?? {},


Suggestion: The v2 renderer applies only global ECharts overrides and never selects echartsOptionsOverridesByChartType, unlike Echart.tsx. Deployments that configure chart-specific overrides will therefore render regular charts and building-block charts differently, even when both use the same theme configuration. The renderer needs a chart-type mapping or an explicit documented policy for resolving the option's series type to the configured override. [api mismatch]

Severity Level: Major ⚠️

- ⚠️ Chart-type overrides apply only to v1 charts. - ⚠️ Dashboard v2 blocks ignore configured series styling. - ⚠️ Themed deployments render equivalent charts inconsistently.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/src/core/dashboard/blocks/ChartBlock.tsx **Line:** 268:269 **Comment:** *Api Mismatch: The v2 renderer applies only global ECharts overrides and never selects `echartsOptionsOverridesByChartType`, unlike `Echart.tsx`. Deployments that configure chart-specific overrides will therefore render regular charts and building-block charts differently, even when both use the same theme configuration. The renderer needs a chart-type mapping or an explicit documented policy for resolving the option's series type to the configured override. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

michael-s-molina

Thanks for the PR @msyavuz. Overall, there are things we need to move to the theme package in core and other things seem specific to ECharts and we might need to move them to the ChartBlock (ECharts) component. Maybe create a folder for it. There's probably a dependency with @villebro's work that we can discuss on Thursday related to the chart theme schema.

michael-s-molina · 2026-08-11T17:46:26Z

+ * palette (ECharts' blues, Vega-Lite's `category10`), so two blocks on one
+ * dashboard disagree about what "the first series" looks like.
+ */
+export declare function getCategoricalColors(): string[];


These are part of the theme package like @rusackas's #39226. We can address the comments in that PR and merge it or just copy the relevant parts for the prototype into the theme folder.

michael-s-molina · 2026-08-11T17:49:48Z

+import {
+  getEchartsTheme,
+  mergeEchartsThemeOverrides,
+} from '@superset-ui/plugin-chart-echarts';


Our plan is to delete/replace the plugin-chart-echarts with a generic ECharts component (ChartBlock is the name for now) so these should actually be exposed here. I think this will depend on @villebro's effort as an ECharts block will need to expose its "schema" which contains theme properties.

michael-s-molina · 2026-08-11T17:51:54Z

+
+type Theme = ReturnType<typeof useTheme>;
+
+export interface ChartTheme {


Doesn't the ChartTheme structure depends on the chart library used? In other words, should this be inside ChartBlock?

michael-s-molina · 2026-08-11T17:52:32Z

+ * have: whatever the author set explicitly is left alone.
+ */
+
+import type { ChartTheme } from './chartTheme';


Same here. This looks specific to ChartBlock (ECharts version).

- An axis authored as an array kept none of the theme's styling: a single object merged over an authored array is replaced wholesale by it, so a chart with two y-axes drew ECharts' own axis lines, labels and grid. The default is matched to the shape of what was authored, one per axis. - An empty `colorScheme` on the root node was passed through as a scheme named "", which the registry looks up and answers with no palette at all. Empty is unset, which is how the dashboard API already read the same prop. - A `theme` bind naming `constructor` or `toString` resolved the inherited `Object.prototype` member through plain property access, splicing a function into the option instead of raising the documented error. Own properties only, and never a function.

msyavuz added 3 commits August 11, 2026 20:28

msyavuz requested review from eschutho, geido, kgabryje, michael-s-molina, rusackas and villebro as code owners August 11, 2026 17:29

pull-request-size Bot added the size/XXL label Aug 11, 2026

dosubot Bot added change:frontend Requires changing the frontend dashboard Namespace | Anything related to the Dashboard global:theming Related to theming Superset viz:charts:echarts Related to Echarts labels Aug 11, 2026

github-actions Bot added plugins packages labels Aug 11, 2026

codeant-ai-for-open-source Bot reviewed Aug 11, 2026

View reviewed changes

michael-s-molina requested changes Aug 11, 2026

View reviewed changes


		type Theme = ReturnType<typeof useTheme>;

		export interface ChartTheme {

Uh oh!

Conversation

msyavuz commented Aug 11, 2026

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Aug 11, 2026

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

bito-code-review Bot commented Aug 11, 2026

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

michael-s-molina left a comment

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bito-code-review Bot commented Aug 11, 2026 •

edited

Loading