|
| 1 | +--- |
| 2 | +description: Theming external libraries (Perses, etc.) with PatternFly tokens in ODH Dashboard |
| 3 | +globs: "packages/observability/**,packages/*/src/**/*theme*,packages/*/src/**/*Theme*" |
| 4 | +alwaysApply: false |
| 5 | +--- |
| 6 | + |
| 7 | +# Third-Party Library Theming — ODH Dashboard |
| 8 | + |
| 9 | +Reference implementation: `packages/observability/src/perses/theme.ts` |
| 10 | + |
| 11 | +For the MLflow federated UI theming approach (Emotion token translation + SCSS shell overrides), see the [MLflow fork](https://github.com/opendatahub-io/mlflow/tree/master/mlflow/server/js/src/common/styles/patternfly) — that pattern lives in the fork, not here. |
| 12 | + |
| 13 | +--- |
| 14 | + |
| 15 | +## Invariant 1 — ECharts/canvas: use `.value`, not CSS vars |
| 16 | + |
| 17 | +Canvas-based renderers cannot resolve CSS custom properties at paint time. Passing a `var(--pf-t--...)` string to an ECharts option or canvas `fillStyle` silently produces the wrong color. |
| 18 | + |
| 19 | +```ts |
| 20 | +import { t_color_white, t_color_gray_95 } from '@patternfly/react-tokens'; |
| 21 | + |
| 22 | +// Correct — resolved hex value |
| 23 | +const textColor = isDark ? t_color_white.value : t_color_gray_95.value; |
| 24 | + |
| 25 | +// These overrides are passed to generateChartsTheme(), not as top-level MUI theme properties |
| 26 | +const chartsTheme = generateChartsTheme(muiTheme, { |
| 27 | + echartsTheme: { |
| 28 | + color: palette, // hex array, never CSS var strings |
| 29 | + tooltip: { |
| 30 | + // CSS vars ARE fine for non-canvas properties (e.g. tooltip is DOM-rendered) |
| 31 | + backgroundColor: 'var(--pf-t--global--background--color--inverse--default)', |
| 32 | + }, |
| 33 | + }, |
| 34 | + thresholds: { |
| 35 | + defaultColor: textColor, // .value, not .var |
| 36 | + }, |
| 37 | +}); |
| 38 | +``` |
| 39 | + |
| 40 | +Use `@patternfly/react-tokens` `.value` anywhere the value is consumed by a canvas renderer. Use `var(--pf-t--*)` strings everywhere else. |
| 41 | + |
| 42 | +--- |
| 43 | + |
| 44 | +## Invariant 2 — Dark mode source: `useThemeContext()` only |
| 45 | + |
| 46 | +Never infer dark mode from `prefers-color-scheme`, a local prop, or any source other than the dashboard theme context. |
| 47 | + |
| 48 | +```ts |
| 49 | +import { useThemeContext } from '@odh-dashboard/internal/app/ThemeContext'; |
| 50 | + |
| 51 | +const { theme: contextTheme } = useThemeContext(); |
| 52 | +const theme: PatternFlyTheme = contextTheme === 'dark' ? 'dark' : 'light'; |
| 53 | +``` |
| 54 | + |
| 55 | +The full theme object must recompute when context changes: |
| 56 | + |
| 57 | +```ts |
| 58 | +return React.useMemo(() => { |
| 59 | + const muiTheme = getTheme(theme, { ...mapPatternFlyThemeToMUI(theme) }); |
| 60 | + // ... build chartsTheme ... |
| 61 | + return { muiTheme, chartsTheme }; |
| 62 | +}, [theme]); |
| 63 | +``` |
| 64 | + |
| 65 | +--- |
| 66 | + |
| 67 | +## Invariant 3 — `ThemeProvider` at the library boundary, not the app root |
| 68 | + |
| 69 | +Placing an MUI `ThemeProvider` at the app root leaks MUI styles into PF-only components. Scope it as close to the library's render boundary as possible. |
| 70 | + |
| 71 | +```tsx |
| 72 | +// Correct — scoped to the Perses render boundary |
| 73 | +// packages/observability/src/perses/PersesWrapper.tsx |
| 74 | +const { muiTheme, chartsTheme } = usePatternFlyTheme(); |
| 75 | +return ( |
| 76 | + <ThemeProvider theme={muiTheme}> |
| 77 | + <ChartsProvider chartsTheme={chartsTheme}> |
| 78 | + {children} |
| 79 | + </ChartsProvider> |
| 80 | + </ThemeProvider> |
| 81 | +); |
| 82 | + |
| 83 | +// Wrong — app root wrapping bleeds MUI styles into unrelated PF components |
| 84 | +// frontend/src/app/App.tsx |
| 85 | +return <ThemeProvider theme={muiTheme}><App /></ThemeProvider>; |
| 86 | +``` |
| 87 | + |
| 88 | +One wrapper component per external library. Never scatter `ThemeProvider` across individual pages. |
0 commit comments