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 96880a5e8a783674fb5560f0ec74ee881e5243e4 Author: Claude <[email protected]> AuthorDate: Wed May 13 11:03:44 2026 -0700 feat(dashboard): ThemeSelectorModal + setComponentThemeId action — Phase 3 End-to-end mechanism for applying a CRUD theme to a single dashboard grid component. Two pieces: 1. `setComponentThemeId(componentId, themeId | null)` — thin Redux action that merges \`themeId\` into the target component's \`meta\` via the existing \`updateComponents\` thunk, preserving every other meta field. Explicit \`null\` clears the override and falls back to the inherited theme; the resolver in Phase 1 treats null and undefined identically. No-ops when the component id isn't in the layout. 2. \`ThemeSelectorModal\` — parent-owned modal that fetches non-system themes (same query as the dashboard Properties modal: \`is_system:false\` filter on \`/api/v1/theme/\`), preselects the currently-resolved override via the Phase-1 \`useEffectiveThemeId\` hook, and exposes Apply / Cancel / Clear-override-(inherit) actions. Each call site provides \`layoutId\` + the \`show\`/\`onHide\` toggle. No call site for the modal yet — Phase 4 wires the "Apply theme" menu item into each component's \`ComponentHeaderControls\` to open it. 3 passing tests on the action: merge preserves other meta keys, clear stores explicit null (not undefined), no-op for missing component. SIP.md updated with the Phase 3 implementation notes and the deferred- to-Phase-4 wiring detail. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- SIP.md | 17 ++- .../dashboard/actions/setComponentThemeId.test.ts | 98 ++++++++++++++ .../src/dashboard/actions/setComponentThemeId.ts | 52 +++++++ .../components/ThemeSelectorModal/index.tsx | 149 +++++++++++++++++++++ 4 files changed, 315 insertions(+), 1 deletion(-) diff --git a/SIP.md b/SIP.md index bdb0a82cd93..8be321d732c 100644 --- a/SIP.md +++ b/SIP.md @@ -180,7 +180,22 @@ Each phase brings its own tests; the cumulative bar: its toggle-style Edit/Preview switcher and gains a dots menu), so we do them per-component alongside the theme wiring so each can be reviewed in isolation. -- _(Phase 3)_ — pending. +- _(Phase 3)_ — ✅ landed locally. `ThemeSelectorModal` (fetches non-system + themes via the same `/api/v1/theme/?q=...` query that the dashboard + Properties modal uses; preselects the currently-resolved override; + "Apply" / "Cancel" / "Clear override (inherit)" buttons) and the + thin `setComponentThemeId(componentId, themeId | null)` action that + merges into `meta.themeId` via the existing `updateComponents` thunk. + + No call site for the modal yet — Phase 4's per-component PRs add the + "Apply theme" item to each component's menu that opens this modal. + The modal is parent-controlled (`show`/`onHide`), parent-owned, so + there's no wiring needed beyond `<ThemeSelectorModal layoutId={id} + show={open} onHide={...} />` in each call site. + + 3 passing tests on `setComponentThemeId`: preserves other meta keys + + sets numeric `themeId`; stores explicit `null` for the clear path; + no-op when the component id isn't in the layout. - _(Phase 4)_ — pending. ### Phase 1 status diff --git a/superset-frontend/src/dashboard/actions/setComponentThemeId.test.ts b/superset-frontend/src/dashboard/actions/setComponentThemeId.test.ts new file mode 100644 index 00000000000..26ac622d1b6 --- /dev/null +++ b/superset-frontend/src/dashboard/actions/setComponentThemeId.test.ts @@ -0,0 +1,98 @@ +/** + * 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 { setComponentThemeId } from './setComponentThemeId'; +import { UPDATE_COMPONENTS } from './dashboardLayout'; + +const componentFixture = { + id: 'CHART-abc', + type: 'CHART', + children: [], + parents: ['ROOT_ID', 'ROW-1'], + meta: { + chartId: 42, + sliceName: 'My Chart', + width: 4, + height: 30, + }, +}; + +const buildState = () => + ({ + dashboardLayout: { + present: { + 'CHART-abc': componentFixture, + }, + }, + // The thunk wrapper (`setUnsavedChangesAfterAction`) reads this. + dashboardState: { hasUnsavedChanges: false }, + }) as unknown as ReturnType< + Parameters<ReturnType<typeof setComponentThemeId>>[1] + >; + +// `updateComponents` is wrapped by `setUnsavedChangesAfterAction`, which +// returns a thunk. The outer dispatch receives the thunk; we recursively +// execute it to capture the actual UPDATE_COMPONENTS action object. +const dispatchedActions = ( + outer: (dispatch: any, getState: any) => void, + getState: any, +): any[] => { + const actions: any[] = []; + const dispatch = (action: any) => { + if (typeof action === 'function') { + action(dispatch, getState); + } else { + actions.push(action); + } + }; + outer(dispatch, getState); + return actions; +}; + +test('dispatches an UPDATE_COMPONENTS that preserves existing meta and sets themeId', () => { + const actions = dispatchedActions( + setComponentThemeId('CHART-abc', 7), + () => buildState(), + ); + const action = actions.find(a => a.type === UPDATE_COMPONENTS); + expect(action).toBeDefined(); + expect(action.payload.nextComponents['CHART-abc'].meta).toEqual({ + chartId: 42, + sliceName: 'My Chart', + width: 4, + height: 30, + themeId: 7, + }); +}); + +test('clearing the override stores explicit null (not undefined)', () => { + const actions = dispatchedActions( + setComponentThemeId('CHART-abc', null), + () => buildState(), + ); + const action = actions.find(a => a.type === UPDATE_COMPONENTS); + expect(action.payload.nextComponents['CHART-abc'].meta.themeId).toBeNull(); +}); + +test('no-op when the component is missing from layout', () => { + const actions = dispatchedActions( + setComponentThemeId('CHART-missing', 7), + () => buildState(), + ); + expect(actions).toEqual([]); +}); diff --git a/superset-frontend/src/dashboard/actions/setComponentThemeId.ts b/superset-frontend/src/dashboard/actions/setComponentThemeId.ts new file mode 100644 index 00000000000..41770c1b0b8 --- /dev/null +++ b/superset-frontend/src/dashboard/actions/setComponentThemeId.ts @@ -0,0 +1,52 @@ +/** + * 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 { AppDispatch, GetState } from 'src/dashboard/types'; +import { updateComponents } from './dashboardLayout'; + +/** + * Sets (or clears) the per-component theme override on a dashboard + * grid component. `themeId === null` clears the override and falls back + * to the inherited theme. + * + * Thin wrapper around `updateComponents` that touches only the `themeId` + * key on the component's `meta`, preserving every other meta field. Used + * by `ThemeSelectorModal` (and any future call site) so the meta-merge + * logic lives in one place. + */ +export function setComponentThemeId( + componentId: string, + themeId: number | null, +) { + return (dispatch: AppDispatch, getState: GetState) => { + const { dashboardLayout } = getState(); + const component = dashboardLayout.present[componentId]; + if (!component) return; + dispatch( + updateComponents({ + [componentId]: { + ...component, + meta: { + ...component.meta, + themeId, + }, + }, + }), + ); + }; +} diff --git a/superset-frontend/src/dashboard/components/ThemeSelectorModal/index.tsx b/superset-frontend/src/dashboard/components/ThemeSelectorModal/index.tsx new file mode 100644 index 00000000000..19ec62b2411 --- /dev/null +++ b/superset-frontend/src/dashboard/components/ThemeSelectorModal/index.tsx @@ -0,0 +1,149 @@ +/** + * 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 { useEffect, useMemo, useState } from 'react'; +import rison from 'rison'; +import { useDispatch } from 'react-redux'; +import { SupersetClient } from '@superset-ui/core'; +import { t } from '@apache-superset/core/translation'; +import { Button, Modal, Select } from '@superset-ui/core/components'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import { useEffectiveThemeId } from 'src/dashboard/components/ComponentThemeProvider'; +import { setComponentThemeId } from 'src/dashboard/actions/setComponentThemeId'; + +interface ThemeOption { + id: number; + theme_name: string; +} + +interface ThemeSelectorModalProps { + /** The layout component receiving the theme override. */ + layoutId: string; + /** Controls visibility. Parent owns this — toggled via menu click. */ + show: boolean; + onHide: () => void; +} + +/** + * Modal for picking a CRUD theme to apply to a single dashboard component + * (or clearing the existing override). On save, dispatches + * `setComponentThemeId`, which updates `component.meta.themeId` and marks + * the dashboard dirty. The actual visual application is handled by + * `ComponentThemeProvider`, which reads the meta change via its Redux + * selector and re-renders the component with the new theme tokens. + */ +export default function ThemeSelectorModal({ + layoutId, + show, + onHide, +}: ThemeSelectorModalProps) { + const dispatch = useDispatch(); + const { addDangerToast } = useToasts(); + const currentThemeId = useEffectiveThemeId(layoutId); + + // Modal-local draft of the selection. Synced from the resolved id when + // the modal opens; only committed to Redux on save. + const [selectedId, setSelectedId] = useState<number | null>(currentThemeId); + const [themes, setThemes] = useState<ThemeOption[]>([]); + const [loading, setLoading] = useState(false); + + // Keep the draft in sync if the resolved id changes while the modal is + // open (e.g. another tab updated the dashboard). Cheap because the + // selector returns a primitive. + useEffect(() => { + if (show) setSelectedId(currentThemeId); + }, [show, currentThemeId]); + + useEffect(() => { + if (!show) return; + setLoading(true); + // Same query the dashboard-properties modal uses — non-system themes only. + const q = rison.encode({ + columns: ['id', 'theme_name'], + filters: [{ col: 'is_system', opr: 'eq', value: false }], + }); + SupersetClient.get({ endpoint: `/api/v1/theme/?q=${q}` }) + .then(({ json }) => { + setThemes((json.result as ThemeOption[]) ?? []); + }) + .catch(() => { + addDangerToast(t('An error occurred while fetching available themes')); + }) + .finally(() => setLoading(false)); + }, [show, addDangerToast]); + + const options = useMemo( + () => themes.map(t => ({ value: t.id, label: t.theme_name })), + [themes], + ); + + const handleSave = () => { + dispatch(setComponentThemeId(layoutId, selectedId)); + onHide(); + }; + + const handleClear = () => { + // Clearing the override means "inherit from parent" — store explicit + // null so the resolver knows it was intentional (vs absent / never set). + dispatch(setComponentThemeId(layoutId, null)); + onHide(); + }; + + return ( + <Modal + show={show} + onHide={onHide} + title={t('Apply theme')} + footer={ + <> + {currentThemeId !== null && ( + <Button + data-test="component-theme-clear" + buttonStyle="secondary" + onClick={handleClear} + > + {t('Clear override (inherit)')} + </Button> + )} + <Button data-test="component-theme-cancel" onClick={onHide}> + {t('Cancel')} + </Button> + <Button + data-test="component-theme-apply" + buttonStyle="primary" + onClick={handleSave} + disabled={selectedId === null} + > + {t('Apply')} + </Button> + </> + } + > + <Select + ariaLabel={t('Theme')} + loading={loading} + options={options} + value={selectedId ?? undefined} + onChange={value => setSelectedId(value as number)} + placeholder={t('Select a theme')} + allowClear + onClear={() => setSelectedId(null)} + /> + </Modal> + ); +}
