fix(dashboard): comprehensive dark mode fixes across all affected components#1110
fix(dashboard): comprehensive dark mode fixes across all affected components#1110
Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — Well-organized dark mode fix across ~25 files with a sound architectural approach for the theme-aware chart color system.
Notes
Theme-aware chart colors: The dual-palette system (CHART_PALETTE_LIGHT / CHART_PALETTE_DARK) with the useChartColors() hook is a clean solution to Recharts' limitation of requiring hex values rather than CSS variables. The backward-compatible getAgentColor function with optional palette parameter preserves existing call sites and test compatibility. The pure functions (buildDurationSegments, buildDurationChartData) correctly accept optional colorFn to remain testable without React context.
Sonner CSS variable fix: The var(--popover) → var(--color-popover) change is correct — the CSS defines --color-popover, matching Tailwind v4's naming convention. The old variable names would have resolved to nothing.
Destructive button pattern: Replacing inline className="bg-destructive text-white hover:bg-destructive/90" with variant="destructive" is the right call — AlertDialogAction already supports the variant prop and delegates to the Button component which handles dark mode correctly through the design system.
Select dark mode fix: The dark:bg-card dark:text-foreground [&_option]:bg-card pattern is consistently applied across all native <select> elements. The [&_option]:bg-card selector targets <option> children which inherit the OS-default white background in dark mode — correct fix for the "white flash" issue.
Variable shadowing in work-item-duration-bar.tsx: The const getAgentColor = useChartColors() at component level intentionally shadows the module-level getAgentColor import. The import is still needed as the default fallback in the buildDurationSegments pure function. This is fine — the shadowing is scoped to the component function body.
All CI checks pass. Tests cover the dual-palette system adequately.
🕵️ claude-code · claude-opus-4-6 · run details
Summary
Fixes 7 categories of dark mode issues across ~25 files in
web/src/, as identified in the investigation phase.<select>dark backgrounds — Addeddark:bg-card dark:text-foreground [&_option]:bg-cardto all native select elements inrun-filters.tsx,stats-filters.tsx,webhook-logs.tsx,agent-definition-sections.tsx,user-form-dialog.tsx, andwizard-shared.tsxCHART_PALETTE_DARKhex approximations of dark-mode oklch CSS vars; newuseChartColors()hook usinguseTheme()fromnext-themesto select the correct palette; updatedgetAgentColor()to accept optional palette parameter for backward compatibilitywork-item-cost-chart.tsx,work-item-duration-chart.tsx,project-work-duration-chart.tsx,project-work-table.tsx,work-item-duration-bar.tsxnow useuseChartColors()hook;CartesianGridusescurrentColorstroke,XAxis/YAxistick fill usescurrentColor, Legend text usescolor: 'currentColor'var(--popover)tovar(--color-popover)to match Tailwind v4 naming conventionbg-whitetobg-white dark:bg-primary-foregroundon toggle switch knob inagent-definition-shared.tsxdark:variants — Added dark variants for hardcoded colors:text-blue-600 dark:text-blue-400in PR runs page,text-red-500 dark:text-red-400andring-red-500/60 dark:ring-red-400/60in duration bar,text-green-500/600 dark:text-green-400in pm-wizard stepsbg-muted dark:bg-black/90for softer light mode while preserving intentional dark terminal look;text-neutral-700 dark:text-neutral-300for readable text in both themesclassName="bg-destructive text-white hover:bg-destructive/90"withvariant="destructive"inprojects-table.tsx,project-general-form.tsx, andusers-table.tsxTest plan
web/project)tests/unit/web/chart-colors.test.tsvalidates dual-palette systemKey decisions
getAgentColor— The exported function still works without args (uses light palette by default), so all existing call sites and tests continue to work without changesbuildDurationSegments/buildDurationChartData— These functions remain pure (no hooks), accepting an optionalcolorFnparameter. Tests mock the function without needing React contextbg-black/90dark mode look is intentional. We added abg-mutedlight-mode fallback for softer appearance without changing the terminal aestheticTrello card: https://trello.com/c/69df4d553feba59e3fb7dba2
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details