rusackas opened a new pull request, #40133: URL: https://github.com/apache/superset/pull/40133
**Draft / RFC PR — opening for early architectural review.** This branch implements component-level theming for dashboard grid components, with the inheritance chain `Instance → Dashboard → Tab → Row/Col → Chart/Markdown`. See `SIP.md` at the repo root for the full design doc (motivation, non-goals, storage decision, architecture, open questions, test plan). This supersedes the closed PR #36749 (which became unrebasable after the .jsx → .tsx conversion in #fc5506e466, the React 18 upgrade in #38563, and the heavy theme-controller churn since 2025-12). ## Phases (each is a separate commit on this branch) | Commit | Phase | What | |---|---|---| | `9959465017` | SIP | `SIP.md` design doc — kept current as the implementation evolved | | `96e8ddc95c` | **1** | Storage shape (`LayoutItemMeta.themeId`), `pickEffectiveThemeId` resolver, `useEffectiveThemeId` hook, `ComponentThemeProvider` wired into `ChartHolder` as PoC. **8 unit tests** for the parents-walk resolver. | | `5d9f0780fc` | **2** | Shared `ComponentHeaderControls` (vertical-dots menu, generic `items` API). **4 unit tests**. | | `96880a5e8a` | **3** | `ThemeSelectorModal` (fetches non-system themes, Apply/Cancel/Clear-override buttons) + `setComponentThemeId` action that merges into `meta.themeId`. **3 unit tests**. | | `1f3d2cc305` | **4a** | Chart end-to-end demo: \"Apply theme\" item in `SliceHeaderControls` opens the modal. | | `1be84f1769` | **4b/c/d/e** | Same recipe (provider wrap + modal mount + menu item) applied to Tabs, Row, Column, Markdown. | ## What works end-to-end after this branch A user in dashboard edit mode can: 1. Open the dots/options menu on any grid component (Chart, Markdown, Row, Column, Tabs). 2. Click **Apply theme** → modal opens. 3. Pick a CRUD theme → component re-renders with the theme's tokens. 4. Save the dashboard → persisted in `position_json`'s `meta.themeId` (no schema migration — the meta map was already open-ended). 5. Reload → theme reapplied via the existing `ThemeController.createDashboardThemeProvider` cache (one fetch per unique theme id, regardless of how many components reference it). 6. The full inheritance chain works — set a theme on a Row, all chart/markdown children inherit unless they set their own override. ## What's intentionally deferred Documented in detail in `SIP.md` under \"Open questions / shortcomings\": - **Menu-pattern unification.** This branch *adds* the dots menu *alongside* existing component menus (Markdown's Edit/Preview toggle, Row/Column's gear icon → BackgroundStyleDropdown, Tabs' nothing-before). Converging those onto the shared dots menu is a separate user-visible UX displacement that deserves its own SIP/PR. - **Live theme preview** in `ThemeSelectorModal` (initial scope is just save/cancel). - **Embedded SDK verification** — embedded skips `ThemeController.setCrudTheme`, need to confirm component-level themes still apply. - **Mobile** — hover-to-reveal `HoverMenu` pattern needs a tap-equivalent. - **Permission gate** — should `theme_write` be required to assign a theme? Currently any dashboard editor can. - **Screenshot regression test** for theme application via the existing screenshot service. - **Stale-theme cleanup** — what happens if a `themeId` references a deleted CRUD theme? Currently falls back silently to parent; might want explicit cleanup. ## Test plan ### Unit (passing) - `pickEffectiveThemeId` (8 cases): own / inherited / null-skip / no-ancestor / root-stop / malformed-parents-loop-safe / other-meta-keys-ignored / missing-id. - `ComponentHeaderControls` (4 cases): empty items renders nothing, trigger renders, onClick fires, disabled items don't fire. - `setComponentThemeId` (3 cases): merge preserves other meta, clear stores explicit null, no-op for missing component. ### Integration / E2E (TODO before non-draft) - RTL: assign theme to a chart → chart re-renders. Assign to a Row → child chart inherits. Override on chart → chart wins. Clear override → falls back. Reload from `position_json` → preserved. - Playwright: one happy-path scenario per component type. - Permission scenario: editor can apply, viewer can't see the menu. ### Manual / screenshot - Light dashboard with one dark-themed chart and one default-themed chart — visual diff in CI. - Embedded dashboard with a component theme — verify no host-CSS bleed. ## ADDITIONAL INFORMATION - [ ] Has associated issue: (will open a SIP discussion thread separately, gating on this draft review) - [ ] Required feature flags: none — additive feature, backwards-compatible for dashboards without `meta.themeId`. - [x] Changes UI - [ ] Includes DB Migration - [x] Introduces new feature or API (per-component theming) - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
