feat: add dark theme support and UI polish#308
Conversation
There was a problem hiding this comment.
Pull request overview
Adds app-wide light/dark theming support (persisted) and updates many core screens/components to use theme token-based styling for improved dark-mode contrast and consistency.
Changes:
- Introduces a
ThemeProvider+ headerThemeToggle, and wires provider into the root layout. - Defines dark theme CSS variables (
html.dark) and replaces hard-coded slate/gray styles across core UI. - Polishes table empty/loading states, menus, forms, dashboards, and panels to be theme-aware.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/ui/theme-toggle.tsx | Adds a header button to toggle light/dark theme. |
| frontend/src/components/ui/table-state.tsx | Updates loading/empty table row styling to theme tokens. |
| frontend/src/components/ui/searchable-select.tsx | Converts select trigger/content/item base styles to theme tokens. |
| frontend/src/components/ui/popover.tsx | Adjusts popover surface/border/text styles to theme tokens. |
| frontend/src/components/ui/dropdown-select.tsx | Updates dropdown select trigger/list styling for theme support. |
| frontend/src/components/ui/command.tsx | Converts command palette UI (input/list/items) to theme tokens. |
| frontend/src/components/templates/DashboardShell.tsx | Adds theme toggle in header; updates shell surfaces/borders for theming. |
| frontend/src/components/templates/DashboardPageLayout.tsx | Updates page layout surfaces, header borders, and typography tokens. |
| frontend/src/components/tags/TagForm.tsx | Converts Tag form container/labels/errors to theme-aware styling. |
| frontend/src/components/tables/DataTable.tsx | Updates table divider/header/empty text styling to theme tokens. |
| frontend/src/components/providers/ThemeProvider.tsx | New provider implementing persisted theme + DOM application logic. |
| frontend/src/components/organization/MembersInvitesTable.tsx | Updates organization members table colors for dark mode. |
| frontend/src/components/organization/BoardAccessTable.tsx | Updates board access table subtitles/row/header styles for theming. |
| frontend/src/components/organisms/UserMenu.tsx | Converts user menu trigger/popover/menu items to theme tokens. |
| frontend/src/components/organisms/TaskBoard.tsx | Adjusts task board column hover/text/badge styles for dark mode. |
| frontend/src/components/organisms/OrgSwitcher.tsx | Updates org switcher select styles and error color tokens. |
| frontend/src/components/organisms/LocalAuthLogin.tsx | Updates error styling to token-based danger colors. |
| frontend/src/components/organisms/DashboardSidebar.tsx | Converts sidebar surfaces/nav items/status styling to theme tokens. |
| frontend/src/components/molecules/TaskCard.tsx | Updates card surfaces/badges/status highlights for dark mode. |
| frontend/src/components/gateways/GatewayForm.tsx | Converts gateway form styling and validation colors to theme tokens. |
| frontend/src/components/custom-fields/CustomFieldForm.tsx | Updates custom field form surfaces/labels/errors for dark mode. |
| frontend/src/components/boards/BoardsTable.tsx | Updates table hover/empty/icon styles to theme tokens. |
| frontend/src/components/activity/ActivityFeed.tsx | Converts feed loading/empty/error states to theme tokens. |
| frontend/src/components/BoardApprovalsPanel.tsx | Updates approvals panel surfaces/typography and badge tones for dark mode. |
| frontend/src/app/tags/page.tsx | Updates tags page container and error styling for dark mode. |
| frontend/src/app/skills/packs/page.tsx | Updates containers and error styling to token-based danger colors. |
| frontend/src/app/skills/marketplace/page.tsx | Converts marketplace surfaces/typography to theme tokens. |
| frontend/src/app/sign-in/[[...rest]]/page.tsx | Updates sign-in page background to app theme background. |
| frontend/src/app/settings/page.tsx | Converts settings page sections and form copy to theme tokens. |
| frontend/src/app/organization/page.tsx | Updates organization page surfaces, headers, and empty states for theming. |
| frontend/src/app/layout.tsx | Wires ThemeProvider into the app and suppresses hydration warning on <html>. |
| frontend/src/app/globals.css | Adds html.dark theme variables and token-backed utility classes. |
| frontend/src/app/gateways/page.tsx | Updates gateways list container and error styling for dark mode. |
| frontend/src/app/gateways/[gatewayId]/page.tsx | Updates gateway detail surfaces/labels/errors to theme tokens. |
| frontend/src/app/dashboard/page.tsx | Updates dashboard cards/panels/empty states for dark mode contrast. |
| frontend/src/app/custom-fields/page.tsx | Updates container and error styling for dark mode. |
| frontend/src/app/boards/page.tsx | Updates boards page container and error styling for dark mode. |
| frontend/src/app/boards/new/page.tsx | Updates new board form surfaces, required markers, and helper styles. |
| frontend/src/app/boards/[boardId]/webhooks/[webhookId]/payloads/page.tsx | Updates webhook payloads page surfaces/typography for theming. |
| frontend/src/app/board-groups/page.tsx | Updates board groups container and error styling for dark mode. |
| frontend/src/app/board-groups/new/page.tsx | Updates new board group form surfaces, lists, and helper text. |
| frontend/src/app/approvals/page.tsx | Updates approvals page background to theme background. |
| frontend/src/app/agents/page.tsx | Updates agents page container and error styling for dark mode. |
| frontend/src/app/agents/new/page.tsx | Updates new agent form styling and help/error tokens for dark mode. |
| frontend/src/app/activity/page.tsx | Updates activity feed card styling and some pill/text tokens for theming. |
|
|
||
| <section className="rounded-xl border border-rose-200 bg-rose-50/70 p-6 shadow-sm"> | ||
| <h2 className="text-base font-semibold text-rose-900"> | ||
| <section className="rounded-xl border border-[color:var(--danger)]/30 bg-[color:var(--danger)]/10/70 p-6 shadow-sm"> |
There was a problem hiding this comment.
bg-[color:var(--danger)]/10/70 is not a valid Tailwind class (double opacity segment). This will be ignored at runtime, so the Delete account section background likely won’t render as intended. Use a single opacity modifier (e.g. /10) or combine with a separate opacity-* utility instead.
| <section className="rounded-xl border border-[color:var(--danger)]/30 bg-[color:var(--danger)]/10/70 p-6 shadow-sm"> | |
| <section className="rounded-xl border border-[color:var(--danger)]/30 bg-[color:var(--danger)]/10 p-6 shadow-sm"> |
| itemClassName="px-4 py-3 text-sm text-slate-700 data-[selected=true]:bg-slate-50 data-[selected=true]:text-slate-900" | ||
| triggerClassName="w-full h-11 rounded-xl border border-[color:var(--border-strong)] bg-[color:var(--surface)] px-3 py-2 text-sm font-medium text-strong shadow-sm focus:border-[color:var(--accent)] focus:ring-2 focus:ring-[color:var(--accent-soft)]" | ||
| contentClassName="rounded-xl border border-[color:var(--border)] shadow-lg" | ||
| itemClassName="px-4 py-3 text-sm text-muted data-[selected=true]:bg-slate-50 data-[selected=true]:text-slate-900" |
There was a problem hiding this comment.
The SearchableSelect itemClassName here overrides the component’s new theme-aware selected styles with bg-slate-50/text-slate-900, which will look incorrect in dark mode (and conflicts with the base token styling). Consider removing the override or switching it to the token-based data-[selected=true]:bg-[color:var(--surface-muted)] + text-strong styling used elsewhere.
| itemClassName="px-4 py-3 text-sm text-muted data-[selected=true]:bg-slate-50 data-[selected=true]:text-slate-900" | |
| itemClassName="px-4 py-3 text-sm text-muted data-[selected=true]:bg-[color:var(--surface-muted)] data-[selected=true]:text-strong" |
| export function ThemeProvider({ children }: { children: ReactNode }) { | ||
| const [theme, setThemeState] = useState<Theme>("light"); | ||
| const [mounted, setMounted] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const stored = window.localStorage.getItem(STORAGE_KEY) as Theme | null; | ||
| const initialTheme = stored === "light" || stored === "dark" ? stored : getSystemTheme(); | ||
| setThemeState(initialTheme); | ||
| applyTheme(initialTheme); | ||
| setMounted(true); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (!mounted) return; | ||
| applyTheme(theme); | ||
| window.localStorage.setItem(STORAGE_KEY, theme); | ||
| }, [mounted, theme]); | ||
|
|
||
| useEffect(() => { | ||
| if (typeof window === "undefined") return; | ||
| const media = window.matchMedia("(prefers-color-scheme: dark)"); | ||
| const onChange = () => { | ||
| const stored = window.localStorage.getItem(STORAGE_KEY); | ||
| if (stored === "light" || stored === "dark") return; | ||
| const nextTheme = media.matches ? "dark" : "light"; | ||
| setThemeState(nextTheme); | ||
| applyTheme(nextTheme); | ||
| }; | ||
| media.addEventListener("change", onChange); | ||
| return () => media.removeEventListener("change", onChange); | ||
| }, []); |
There was a problem hiding this comment.
Consider adding unit tests for the new theme persistence behavior (initial theme resolution, toggling, and applying the dark class / colorScheme). The frontend already has vitest coverage for similar components, and this provider has several edge cases (localStorage + system preference changes) that are easy to regress.
| <div className="p-4 md:p-8"> | ||
| {metricsQuery.error ? ( | ||
| <div className="mb-4 rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-700"> | ||
| <div className="mb-4 rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-300"> |
There was a problem hiding this comment.
This error banner uses bg-rose-50 with text-rose-300. In light theme, rose-300 is likely too low-contrast against rose-50, and in dark theme the rose-50 background will be extremely bright. Consider switching both background/text to the theme danger tokens used elsewhere (e.g. bg-[color:var(--danger)]/10 + text-[color:var(--danger)] + matching border).
| <div className="mb-4 rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-300"> | |
| <div className="mb-4 rounded-lg border border-[color:var(--danger)] bg-[color:var(--danger)]/10 p-3 text-sm text-[color:var(--danger)]"> |
| </> | ||
| ) : gatewayUnavailableCount === gatewayTargets.length ? ( | ||
| <div className="rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-700"> | ||
| <div className="rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-300"> |
There was a problem hiding this comment.
Same as above: bg-rose-50 combined with text-rose-300 is likely to have poor contrast in light mode and will be visually jarring in dark mode. Prefer the theme danger tokens for both background and text.
| <div className="rounded-lg border border-rose-300 bg-rose-50 p-3 text-sm text-rose-300"> | |
| <div className="rounded-lg border border-[color:var(--danger-soft-border)] bg-[color:var(--danger-soft)] p-3 text-sm text-[color:var(--danger-strong)]"> |
| sideOffset={sideOffset} | ||
| className={cn( | ||
| "z-50 w-72 rounded-md border border-popover bg-popover p-4 text-popover-foreground shadow-md outline-none data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", | ||
| "z-50 w-72 rounded-md border border border-[color:var(--border)] bg-[color:var(--surface)] p-4 text-strong shadow-lg outline-none data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", |
There was a problem hiding this comment.
The Popover content class string contains a duplicated border token ("border border border-[...]"). This is harmless but likely accidental and makes the class list harder to read/maintain; remove the extra border.
| bodyClassName = "divide-y divide-[color:var(--border)]", | ||
| rowClassName = "hover:bg-slate-50", | ||
| cellClassName = "px-3 py-3 md:px-6 md:py-4", |
There was a problem hiding this comment.
rowClassName default still uses hover:bg-slate-50, which will render as a very light hover state in dark mode. Since this component is being made theme-aware, consider switching the default hover to a token-based surface (e.g. hover:bg-[color:var(--surface-muted)]).
| } | ||
| if (eventType === "agent.offline") { | ||
| return "border-slate-300 bg-slate-100 text-slate-700"; | ||
| return "border-slate-300 bg-slate-100 text-strong"; |
There was a problem hiding this comment.
This pill style still hard-codes bg-slate-100 (and border-slate-300 above), which will appear very bright in dark mode now that the surrounding feed card uses token-based dark surfaces. Consider converting these remaining slate backgrounds/borders to theme tokens (or adding dark-mode variants) for consistency.
| return "border-slate-300 bg-slate-100 text-strong"; | |
| return "border-slate-300 bg-slate-100 text-strong dark:border-slate-700 dark:bg-slate-800 dark:text-slate-100"; |
| className={`inline-flex h-6 w-11 shrink-0 items-center rounded-full border transition ${ | ||
| disableDevicePairing | ||
| ? "border-emerald-600 bg-emerald-600" | ||
| : "border-slate-300 bg-slate-200" | ||
| : "border-[color:var(--border-strong)] bg-slate-200" | ||
| } ${isLoading ? "cursor-not-allowed opacity-60" : "cursor-pointer"}`} |
There was a problem hiding this comment.
The switch “off” track uses bg-slate-200, which will be extremely bright in dark mode and inconsistent with the token-based surfaces used elsewhere in this form. Consider using a surface token (e.g. bg-[color:var(--surface-strong)]/bg-[color:var(--surface-muted)]) for the off state.
| allowInsecureTls | ||
| ? "border-emerald-600 bg-emerald-600" | ||
| : "border-slate-300 bg-slate-200" | ||
| : "border-[color:var(--border-strong)] bg-slate-200" |
There was a problem hiding this comment.
Same issue for the TLS toggle: the off-state bg-slate-200 will be too bright in dark mode. Prefer a token-based background for the off track.
| : "border-[color:var(--border-strong)] bg-slate-200" | |
| : "border-[color:var(--border-strong)] bg-[color:var(--surface-weak)]" |
Summary
Notes