You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/05/13 17:28:38 UTC
(superset) 02/02: feat(dashboard): per-component theme provider — Phase 1 (Chart PoC)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch feat/granular-theming-v2
in repository https://gitbox.apache.org/repos/asf/superset.git
commit a1a3454b5c5723b67526cec758117c8386773f46
Author: Claude <cl...@anthropic.com>
AuthorDate: Wed May 13 10:28:03 2026 -0700
feat(dashboard): per-component theme provider — Phase 1 (Chart PoC)
Adds the skeleton for granular (per-component) theming on dashboard grid
components, with the inheritance chain:
Instance theme -> Dashboard theme -> Tab theme
-> Row/Col theme -> Chart/Markdown theme
This commit lands Phase 1 from the SIP (`SIP.md` at repo root): the
storage shape and the resolver, wired into `ChartHolder` as the
proof-of-concept call site. No UI yet — `themeId` must be set via Redux
devtools / position_json hand-edit to verify visually. Phase 2 will
introduce the `ComponentHeaderControls` menu and Phase 3 the
`ThemeSelectorModal` that drives this from a real UI.
Surface:
- `LayoutItemMeta.themeId?: number | null` — optional CRUD theme id
stored per-component in `position_json` meta (no schema migration; the
meta map is already open-ended). `null` and `undefined` both mean
"inherit from parent".
- `pickEffectiveThemeId(layoutId, layout)` — pure resolver. Walks
`parents` up the layout map from the given node until it finds a
numeric `themeId` or hits `DASHBOARD_ROOT_ID`. Hop-capped at 32 to
defend against malformed parents chains.
- `useEffectiveThemeId(layoutId)` — Redux hook variant.
- `<ComponentThemeProvider layoutId={...}>` — wraps children in the
resolved theme's `SupersetThemeProvider`. Lazy-fetches via the
existing `ThemeController.createDashboardThemeProvider`, which already
caches by id so N components sharing one theme = 1 fetch. Pass-through
when no ancestor sets a `themeId`.
- `ChartHolder.tsx` — wraps the existing `<AntdThemeProvider>` (which is
a popup-container shim for fullscreen mode, not a token provider) so
per-component tokens are set before antd's ConfigProvider for popup
targeting.
Tests: 8 unit cases for `pickEffectiveThemeId` covering own / inherited /
null-skip / no-ancestor / root-stop / malformed-parents / other-meta /
missing-id.
Closes the spirit of the closed PR #36749 (which became unrebasable
after .jsx -> .tsx + React 18 + theme controller churn).
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
SIP.md | 44 +++++++-
.../components/ComponentThemeProvider/index.tsx | 84 ++++++++++++++
.../useEffectiveThemeId.test.ts | 121 +++++++++++++++++++++
.../ComponentThemeProvider/useEffectiveThemeId.ts | 69 ++++++++++++
.../gridComponents/ChartHolder/ChartHolder.tsx | 103 +++++++++---------
superset-frontend/src/dashboard/types.ts | 6 +
6 files changed, 371 insertions(+), 56 deletions(-)
diff --git a/SIP.md b/SIP.md
index ee08de1c724..fe079ed2b2c 100644
--- a/SIP.md
+++ b/SIP.md
@@ -147,15 +147,47 @@ Each phase brings its own tests; the cumulative bar:
## Implementation log
-- _(Phase 1)_ — in progress. See section below for status notes.
+- _(Phase 1)_ — ✅ landed locally. `LayoutItemMeta.themeId`,
+ `ComponentThemeProvider` + `useEffectiveThemeId` hook, wired into
+ `ChartHolder`. 8 passing unit tests. No UI yet — `themeId` has to be
+ set via Redux devtools or position_json hand-edit to verify visually.
- _(Phase 2)_ — pending.
- _(Phase 3)_ — pending.
- _(Phase 4)_ — pending.
### Phase 1 status
-- [ ] Add optional `themeId` field to `LayoutItemMeta`.
-- [ ] Build `ComponentThemeProvider` (Redux selector + parents walk + AntD theme injection).
-- [ ] Wire into `ChartHolder` as the proof of concept (Chart is the leaf with the most existing theme integration, so it's the riskiest first target).
-- [ ] Add unit tests for the resolution hook.
-- [ ] Update this SIP with any architecture surprises uncovered during the wiring.
+- [x] Add optional `themeId` field to `LayoutItemMeta`. (`src/dashboard/types.ts`)
+- [x] Build `ComponentThemeProvider` — `pickEffectiveThemeId` resolver (pure
+ function, walks `parents` up the layout map until it finds a non-null
+ `themeId` or hits `DASHBOARD_ROOT_ID`) + `useEffectiveThemeId` Redux hook
+ + `ComponentThemeProvider` that lazy-fetches the resolved theme via the
+ existing `ThemeController.createDashboardThemeProvider` (which caches by
+ id, so N components referencing the same theme = 1 fetch). Renders as a
+ pass-through when no ancestor sets a `themeId`.
+- [x] Wire into `ChartHolder` — wrapped around the existing
+ `<AntdThemeProvider>` so per-component theme tokens apply to the chart
+ body while the existing popup-container `ConfigProvider` continues to
+ work in fullscreen mode.
+- [x] Add unit tests — 8 cases for `pickEffectiveThemeId` covering own-id /
+ inheritance / null-skip / no-ancestor / root-stop / malformed-parents /
+ other-meta-keys / missing-id.
+- [x] Update SIP with surprises uncovered during wiring (none significant —
+ the existing `createDashboardThemeProvider` did exactly what we needed,
+ including caching by id; the only structural decision was treating the
+ ChartHolder's `<AntdThemeProvider>` as a popup-container shim rather than
+ a token provider, and nesting our provider outside it).
+
+#### Phase 1 surprises / notes
+
+- `ThemeController.createDashboardThemeProvider` already does Theme.fromConfig
+ with the right dark/light base + font loading + caching. We did not need
+ to duplicate any of that logic in the component-level provider.
+- The provider is `useState` + `useEffect` rather than `useMemo` because the
+ fetch is async. That means there's a one-frame flash of the parent theme
+ before the component theme kicks in. Probably acceptable; if not, we can
+ Suspense-ify in Phase 4.
+- `useEffectiveThemeId` re-runs on every Redux state change because the
+ selector returns a primitive `number | null` — that's fine for now, but
+ if dashboards get bigger we may want a memoized selector via reselect
+ keyed on `(layoutId, layout)` — file in the open questions section.
diff --git a/superset-frontend/src/dashboard/components/ComponentThemeProvider/index.tsx b/superset-frontend/src/dashboard/components/ComponentThemeProvider/index.tsx
new file mode 100644
index 00000000000..0822beaabd5
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/ComponentThemeProvider/index.tsx
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { type ReactNode, useEffect, useState } from 'react';
+import type { Theme } from '@apache-superset/core/theme';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import { useEffectiveThemeId } from './useEffectiveThemeId';
+
+interface ComponentThemeProviderProps {
+ /**
+ * Layout item id (the key into `dashboardLayout.present`). The provider
+ * walks up the parents tree from this node to compute the effective
+ * theme override.
+ */
+ layoutId: string | undefined;
+ children: ReactNode;
+}
+
+/**
+ * Per-component theme override wrapper. When the component (or any
+ * ancestor up to but not including the dashboard root) sets a `themeId`
+ * in its `LayoutItemMeta`, this provider loads that CRUD theme and
+ * applies it as a `SupersetThemeProvider` around the children, overriding
+ * the dashboard-level (and, transitively, the instance-level) theme for
+ * this subtree.
+ *
+ * When no ancestor sets a `themeId` — the default — the component renders
+ * as a pass-through. The outer `CrudThemeProvider` (mounted by
+ * `DashboardPage`) continues to provide the dashboard-level theme.
+ */
+export default function ComponentThemeProvider({
+ layoutId,
+ children,
+}: ComponentThemeProviderProps) {
+ const effectiveThemeId = useEffectiveThemeId(layoutId);
+ const { createDashboardThemeProvider } = useThemeContext();
+ const [componentTheme, setComponentTheme] = useState<Theme | null>(null);
+
+ useEffect(() => {
+ if (effectiveThemeId == null) {
+ setComponentTheme(null);
+ return undefined;
+ }
+ let cancelled = false;
+ // `createDashboardThemeProvider` caches by id internally, so per-component
+ // calls for the same theme are deduplicated to a single fetch.
+ createDashboardThemeProvider(String(effectiveThemeId)).then(theme => {
+ if (!cancelled) setComponentTheme(theme);
+ });
+ return () => {
+ cancelled = true;
+ };
+ }, [effectiveThemeId, createDashboardThemeProvider]);
+
+ if (!componentTheme) {
+ return <>{children}</>;
+ }
+
+ return (
+ <componentTheme.SupersetThemeProvider>
+ {children}
+ </componentTheme.SupersetThemeProvider>
+ );
+}
+
+export {
+ useEffectiveThemeId,
+ pickEffectiveThemeId,
+} from './useEffectiveThemeId';
diff --git a/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.test.ts b/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.test.ts
new file mode 100644
index 00000000000..1deb7d05745
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.test.ts
@@ -0,0 +1,121 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { DashboardLayout } from 'src/dashboard/types';
+import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
+import { pickEffectiveThemeId } from './useEffectiveThemeId';
+
+/**
+ * Helper to build a minimally-shaped `DashboardLayout` for these tests.
+ * The real reducer carries many more fields per node; only `parents` and
+ * `meta` are read by the resolver.
+ */
+const buildLayout = (
+ nodes: Record<
+ string,
+ {
+ parents?: string[];
+ themeId?: number | null;
+ extraMeta?: Record<string, unknown>;
+ }
+ >,
+): DashboardLayout =>
+ Object.fromEntries(
+ Object.entries(nodes).map(([id, { parents, themeId, extraMeta }]) => [
+ id,
+ {
+ id,
+ type: 'CHART',
+ children: [],
+ parents,
+ meta: {
+ ...(extraMeta ?? {}),
+ ...(themeId !== undefined ? { themeId } : {}),
+ },
+ },
+ ]),
+ ) as unknown as DashboardLayout;
+
+test('returns null for missing layoutId', () => {
+ expect(pickEffectiveThemeId(undefined, buildLayout({}))).toBeNull();
+ expect(pickEffectiveThemeId('CHART-1', buildLayout({}))).toBeNull();
+});
+
+test("returns the node's own themeId when set", () => {
+ const layout = buildLayout({
+ 'CHART-1': {
+ parents: [DASHBOARD_ROOT_ID, 'ROW-1'],
+ themeId: 42,
+ },
+ 'ROW-1': { parents: [DASHBOARD_ROOT_ID], themeId: 7 },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBe(42);
+});
+
+test("inherits the closest ancestor's themeId when own is unset", () => {
+ const layout = buildLayout({
+ 'CHART-1': { parents: [DASHBOARD_ROOT_ID, 'TAB-1', 'ROW-1'] },
+ 'ROW-1': { parents: [DASHBOARD_ROOT_ID, 'TAB-1'], themeId: 7 },
+ 'TAB-1': { parents: [DASHBOARD_ROOT_ID], themeId: 99 },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBe(7);
+});
+
+test('skips ancestors whose themeId is null and continues walking', () => {
+ // A literal `null` on a node means "I don't override" — keep walking.
+ const layout = buildLayout({
+ 'CHART-1': { parents: [DASHBOARD_ROOT_ID, 'TAB-1', 'ROW-1'] },
+ 'ROW-1': { parents: [DASHBOARD_ROOT_ID, 'TAB-1'], themeId: null },
+ 'TAB-1': { parents: [DASHBOARD_ROOT_ID], themeId: 99 },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBe(99);
+});
+
+test('returns null when no ancestor sets a themeId', () => {
+ const layout = buildLayout({
+ 'CHART-1': { parents: [DASHBOARD_ROOT_ID, 'ROW-1'] },
+ 'ROW-1': { parents: [DASHBOARD_ROOT_ID] },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBeNull();
+});
+
+test('stops at DASHBOARD_ROOT_ID — root-level theme is the dashboard CRUD theme, handled separately', () => {
+ const layout = buildLayout({
+ 'CHART-1': { parents: [DASHBOARD_ROOT_ID] },
+ [DASHBOARD_ROOT_ID]: { parents: [], themeId: 999 },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBeNull();
+});
+
+test('does not loop on a malformed parents chain', () => {
+ // Self-referential parent shouldn't hang the resolver.
+ const layout = buildLayout({
+ 'CHART-1': { parents: ['CHART-1'] },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBeNull();
+});
+
+test('ignores other meta keys', () => {
+ const layout = buildLayout({
+ 'CHART-1': {
+ parents: [DASHBOARD_ROOT_ID],
+ extraMeta: { sliceName: 'Foo', width: 4, background: 'gray' },
+ },
+ });
+ expect(pickEffectiveThemeId('CHART-1', layout)).toBeNull();
+});
diff --git a/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.ts b/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.ts
new file mode 100644
index 00000000000..be42aa2b05a
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/ComponentThemeProvider/useEffectiveThemeId.ts
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useSelector } from 'react-redux';
+import type { DashboardLayout, RootState } from 'src/dashboard/types';
+import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
+
+/**
+ * Walks up the dashboard layout tree from `layoutId` and returns the first
+ * `themeId` it finds, or `null` if no ancestor sets one.
+ *
+ * Inheritance order (closest wins):
+ * Chart/Markdown -> Row/Column -> Tab -> Dashboard root -> (null = inherit
+ * from dashboard CRUD theme or instance theme, applied by CrudThemeProvider
+ * higher in the tree).
+ *
+ * `themeId: null` on a node means "explicitly clear my override" — we treat
+ * the property as absent (and continue walking) iff it is undefined; a literal
+ * `null` is also treated as "no override" since the dashboard-level theme is
+ * applied by a different provider.
+ */
+export function pickEffectiveThemeId(
+ layoutId: string | undefined,
+ layout: DashboardLayout,
+): number | null {
+ if (!layoutId || !layout) return null;
+ let cursorId: string | undefined = layoutId;
+ // Defensive cap — dashboards shouldn't nest deeper than this, and the cap
+ // protects against malformed `parents` arrays causing infinite loops.
+ let hops = 0;
+ while (cursorId && cursorId !== DASHBOARD_ROOT_ID && hops < 32) {
+ const node = layout[cursorId];
+ if (!node) return null;
+ const themeId = (node.meta as { themeId?: number | null } | undefined)
+ ?.themeId;
+ if (typeof themeId === 'number') return themeId;
+ cursorId = node.parents?.[node.parents.length - 1];
+ hops += 1;
+ }
+ return null;
+}
+
+/**
+ * Redux hook variant of `pickEffectiveThemeId`. Memoizes on the layout
+ * reference; consumers that only care about the resolved id (not the layout
+ * map itself) won't re-render when sibling components change their meta.
+ */
+export function useEffectiveThemeId(
+ layoutId: string | undefined,
+): number | null {
+ return useSelector<RootState, number | null>(state =>
+ pickEffectiveThemeId(layoutId, state.dashboardLayout?.present),
+ );
+}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
index 7894796ed62..cc5b7fe9b77 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
@@ -33,6 +33,7 @@ import ResizableContainer from 'src/dashboard/components/resizable/ResizableCont
import getChartAndLabelComponentIdFromPath from 'src/dashboard/util/getChartAndLabelComponentIdFromPath';
import useFilterFocusHighlightStyles from 'src/dashboard/util/useFilterFocusHighlightStyles';
import { AntdThemeProvider } from '@superset-ui/core/components';
+import ComponentThemeProvider from 'src/dashboard/components/ComponentThemeProvider';
import { COLUMN_TYPE, ROW_TYPE } from 'src/dashboard/util/componentTypes';
import {
GRID_BASE_UNIT,
@@ -284,59 +285,61 @@ const ChartHolder = ({
outlinedComponentId ? 'fade-in' : 'fade-out',
)}
>
- <AntdThemeProvider
- getPopupContainer={(triggerNode: HTMLElement) =>
- document.fullscreenElement
- ? (triggerNode?.closest?.(
- '[data-test="dashboard-component-chart-holder"]',
- ) as HTMLElement) || document.body
- : document.body
- }
- >
- {!editMode && (
- <AnchorLink
- id={component.id}
- scrollIntoView={outlinedComponentId === component.id}
- />
- )}
- {!!outlinedComponentId && (
- <style>
- {`label[for=${outlinedColumnName}] + .Select .Select__control {
+ <ComponentThemeProvider layoutId={component.id}>
+ <AntdThemeProvider
+ getPopupContainer={(triggerNode: HTMLElement) =>
+ document.fullscreenElement
+ ? (triggerNode?.closest?.(
+ '[data-test="dashboard-component-chart-holder"]',
+ ) as HTMLElement) || document.body
+ : document.body
+ }
+ >
+ {!editMode && (
+ <AnchorLink
+ id={component.id}
+ scrollIntoView={outlinedComponentId === component.id}
+ />
+ )}
+ {!!outlinedComponentId && (
+ <style>
+ {`label[for=${outlinedColumnName}] + .Select .Select__control {
border-color: ${theme.colorPrimary};
transition: border-color 1s ease-in-out;
}`}
- </style>
- )}
- <Chart
- componentId={component.id}
- id={component.meta.chartId ?? 0}
- dashboardId={dashboardId}
- width={chartWidth}
- height={chartHeight}
- sliceName={
- component.meta.sliceNameOverride ||
- component.meta.sliceName ||
- ''
- }
- updateSliceName={(_sliceId: number, name: string) =>
- handleUpdateSliceName(name)
- }
- isComponentVisible={isComponentVisible}
- handleToggleFullSize={handleToggleFullSize}
- isFullSize={isFullSize}
- setControlValue={handleExtraControl}
- extraControls={extraControls}
- isInView={isInView}
- chartHolderRef={chartHolderRef}
- />
- {editMode && (
- <HoverMenu position="top">
- <div data-test="dashboard-delete-component-button">
- <DeleteComponentButton onDelete={handleDeleteComponent} />
- </div>
- </HoverMenu>
- )}
- </AntdThemeProvider>
+ </style>
+ )}
+ <Chart
+ componentId={component.id}
+ id={component.meta.chartId ?? 0}
+ dashboardId={dashboardId}
+ width={chartWidth}
+ height={chartHeight}
+ sliceName={
+ component.meta.sliceNameOverride ||
+ component.meta.sliceName ||
+ ''
+ }
+ updateSliceName={(_sliceId: number, name: string) =>
+ handleUpdateSliceName(name)
+ }
+ isComponentVisible={isComponentVisible}
+ handleToggleFullSize={handleToggleFullSize}
+ isFullSize={isFullSize}
+ setControlValue={handleExtraControl}
+ extraControls={extraControls}
+ isInView={isInView}
+ chartHolderRef={chartHolderRef}
+ />
+ {editMode && (
+ <HoverMenu position="top">
+ <div data-test="dashboard-delete-component-button">
+ <DeleteComponentButton onDelete={handleDeleteComponent} />
+ </div>
+ </HoverMenu>
+ )}
+ </AntdThemeProvider>
+ </ComponentThemeProvider>
</div>
</ResizableContainer>
),
diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts
index 2a7056bebab..74e8815179b 100644
--- a/superset-frontend/src/dashboard/types.ts
+++ b/superset-frontend/src/dashboard/types.ts
@@ -278,6 +278,12 @@ export type LayoutItemMeta = {
code?: string;
/** Background style value for columns and rows */
background?: string;
+ /**
+ * Optional per-component theme override (CRUD `theme` resource id).
+ * `null` means "explicitly inherit"; a missing key means the same thing
+ * semantically. See ComponentThemeProvider for the inheritance walk.
+ */
+ themeId?: number | null;
/** Allow additional meta properties used by different component types */
[key: string]: unknown;
};