feat(webapp): light theme#4206
Conversation
|
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview Deployment
|
- semantic token overrides for light, trigger.light editor/code palettes - Interface theme dropdown on /account, gated by hasThemeSwitcher flag - unify light surfaces: buttons, inputs, radios, checkboxes, tabs, clipboard fields - fix checkbox :read-only override and useThemeColor hydration mismatch - resolve Firefox panel animation check from request UA
6525961 to
3ccd96f
Compare
7d9ea5b to
bad78e6
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7a0e2672-eab8-4ac6-909d-b71af6fcad18
📒 Files selected for processing (39)
.server-changes/light-theme.mdapps/webapp/app/components/AskAI.tsxapps/webapp/app/components/admin/debugTooltip.tsxapps/webapp/app/components/billing/AnimatedOrgBannerBar.tsxapps/webapp/app/components/billing/BillingLimitRecoveryPanel.tsxapps/webapp/app/components/billing/UsageBar.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/components/primitives/Avatar.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/components/primitives/Checkbox.tsxapps/webapp/app/components/primitives/ClientTabs.tsxapps/webapp/app/components/primitives/ClipboardField.tsxapps/webapp/app/components/primitives/Input.tsxapps/webapp/app/components/primitives/RadioButton.tsxapps/webapp/app/components/primitives/Resizable.tsxapps/webapp/app/components/primitives/SegmentedControl.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/Switch.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/RunTag.tsxapps/webapp/app/components/runs/v3/SpanEvents.tsxapps/webapp/app/components/runs/v3/agent/AgentMessageView.tsxapps/webapp/app/components/runs/v3/ai/AIChatMessages.tsxapps/webapp/app/components/runs/v3/ai/AIModelSummary.tsxapps/webapp/app/components/runs/v3/ai/SpanMetricRow.tsxapps/webapp/app/hooks/useThemeColor.tsapps/webapp/app/root.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsxapps/webapp/app/routes/account._index/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.vercel.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsxapps/webapp/app/services/dashboardPreferences.server.tsapps/webapp/app/tailwind.cssapps/webapp/app/v3/featureFlags.ts
| <div className="whitespace-pre-wrap text-xs italic text-amber-600/70 dark:text-amber-200/70"> | ||
| {p.text ?? ""} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,240p' apps/webapp/app/components/runs/v3/agent/AgentMessageView.tsxRepository: triggerdotdev/trigger.dev
Length of output: 8648
🏁 Script executed:
sed -n '1,260p' apps/webapp/app/components/runs/v3/ai/AIChatMessages.tsxRepository: triggerdotdev/trigger.dev
Length of output: 9023
🏁 Script executed:
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!coverage' "background-hover|text-amber-600|text-amber-200|grid-bright|text-text-dimmed" apps/webapp packages/coreRepository: triggerdotdev/trigger.dev
Length of output: 50382
🏁 Script executed:
rg -n "background-hover|amber-600|amber-200|text-amber-700|text-amber-600|background-bright|grid-bright" apps/webapp -g '!**/*.map' -g '!**/node_modules/**'Repository: triggerdotdev/trigger.dev
Length of output: 50381
🏁 Script executed:
python3 - <<'PY'
from math import pow
def hex_to_rgb(h):
h=h.lstrip('#')
return tuple(int(h[i:i+2],16)/255 for i in (0,2,4))
def srgb_to_lin(c):
return c/12.92 if c <= 0.04045 else ((c+0.055)/1.055) ** 2.4
def rel_lum(rgb):
r,g,b = map(srgb_to_lin, rgb)
return 0.2126*r + 0.7152*g + 0.0722*b
def contrast(c1, c2):
l1, l2 = rel_lum(c1), rel_lum(c2)
hi, lo = max(l1,l2), min(l1,l2)
return (hi + 0.05) / (lo + 0.05)
def blend(fg, bg, alpha):
return tuple(alpha*f + (1-alpha)*b for f,b in zip(fg,bg))
bg = hex_to_rgb("`#f2f3f5`")
amber600 = hex_to_rgb("`#d97706`")
amber700 = hex_to_rgb("`#b45309`")
amber200 = hex_to_rgb("`#fde68a`")
for name, c in [("amber600", amber600), ("amber700", amber700), ("amber200", amber200)]:
for alpha in [1.0, 0.7]:
eff = blend(c, bg, alpha)
print(name, alpha, "contrast=", round(contrast(eff, bg), 2))
PYRepository: triggerdotdev/trigger.dev
Length of output: 331
🏁 Script executed:
sed -n '140,175p' apps/webapp/app/tailwind.css && printf '\n---\n' && sed -n '624,655p' apps/webapp/app/tailwind.cssRepository: triggerdotdev/trigger.dev
Length of output: 2788
Increase the light-mode amber contrast. text-amber-600/70 on the light chat surface is too faint for the reasoning text; switch the light variant to an opaque darker amber token like text-amber-700 (or a semantic warning-text token).
| @@ -1,4 +1,4 @@ | |||
| import { useState } from "react"; | |||
| import { useEffect, useState } from "react"; | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all useThemeColor call sites and check whether any are in persistent layout components
rg -n -B3 -A3 'useThemeColor\(' apps/webapp/app --type=tsx --type=tsRepository: triggerdotdev/trigger.dev
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the hook implementation and all call sites.
rg -n -B3 -A3 'useThemeColor\(' apps/webapp/app
# Inspect the hook file.
wc -l apps/webapp/app/hooks/useThemeColor.ts
cat -n apps/webapp/app/hooks/useThemeColor.ts
# List route/layout files that may keep components mounted across navigations.
fd -H -t f 'route.tsx|layout.tsx|_index.tsx' apps/webapp/appRepository: triggerdotdev/trigger.dev
Length of output: 16922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the hook consumers and layout/route structure.
for f in \
apps/webapp/app/components/billing/FreePlanUsage.tsx \
apps/webapp/app/components/navigation/DashboardDialogs.tsx \
apps/webapp/app/components/layout/AppLayout.tsx \
apps/webapp/app/routes/_app/route.tsx
do
echo "===== $f ====="
wc -l "$f"
cat -n "$f" | sed -n '1,260p'
doneRepository: triggerdotdev/trigger.dev
Length of output: 18677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where the hook consumers are mounted.
rg -n -B3 -A5 'FreePlanUsage|DashboardDialogs' apps/webapp/app --type-add 'ts:*.ts' --type-add 'tsx:*.tsx'Repository: triggerdotdev/trigger.dev
Length of output: 9119
Re-run the color resolution when the theme changes. FreePlanUsage sits in the persistent SideMenu, so it can keep the color captured at mount time after data-theme flips on <html>. Add data-theme (or equivalent theme state) as a trigger so long-lived UI updates with the active theme.
| export async function updateThemePreference({ | ||
| user, | ||
| theme, | ||
| }: { | ||
| user: UserFromSession; | ||
| theme: "dark" | "light"; | ||
| }) { | ||
| if (user.isImpersonating) { | ||
| return; | ||
| } | ||
|
|
||
| if (user.dashboardPreferences.theme === theme) { | ||
| return; | ||
| } | ||
|
|
||
| const updatedPreferences: DashboardPreferences = { | ||
| ...user.dashboardPreferences, | ||
| theme, | ||
| }; | ||
|
|
||
| return prisma.user.update({ | ||
| where: { id: user.id }, | ||
| data: { dashboardPreferences: updatedPreferences }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other preference-update functions use the same spread-based read-modify-write pattern
rg -n -B3 -A10 'prisma\.user\.update' apps/webapp/app/services/dashboardPreferences.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 1584
🏁 Script executed:
#!/bin/bash
sed -n '1,280p' apps/webapp/app/services/dashboardPreferences.server.ts | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 8204
🏁 Script executed:
#!/bin/bash
rg -n 'dashboardPreferences|prisma\.user\.update|updateThemePreference|clearCurrentProject|updateSideMenuPreferences|updateListOrder' apps/webapp/app/services/dashboardPreferences.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 1502
🏁 Script executed:
#!/bin/bash
sed -n '1,320p' apps/webapp/app/services/dashboardPreferences.server.ts | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 8204
🏁 Script executed:
#!/bin/bash
rg -n -A6 -B3 'dashboardPreferences' apps/webapp prisma packages | head -n 200Repository: triggerdotdev/trigger.dev
Length of output: 17592
Avoid full-blob writes for dashboardPreferences.
updateThemePreference merges against the user.dashboardPreferences snapshot and writes the entire object back, so a concurrent preference change from another request can drop unrelated fields. The other setters in this service use the same pattern; if these writes can race, switch to a narrower update path or re-read before saving.
| useEffect(() => { | ||
| const value = getComputedStyle(document.documentElement).getPropertyValue(variable).trim(); | ||
| return value ? toRgb(value) : fallback; | ||
| }); | ||
| if (value) setColor(toRgb(value)); | ||
| }, [variable]); |
There was a problem hiding this comment.
🟡 Theme-dependent color animations show wrong colors after switching themes without page reload
The resolved animated color is never refreshed when the theme changes (useEffect at useThemeColor.ts:28 depends only on [variable], not on the active theme), so framer-motion progress bars keep using the old theme's RGB values until the component remounts.
Impact: After switching between dark and light themes, animated progress bars (e.g., usage meters in the sidebar) display colors from the previous theme until the user navigates away and back.
Mechanism: useEffect dependency omits the active theme
The hook resolves the CSS variable to a concrete RGB value once on mount via getComputedStyle (useThemeColor.ts:29). The effect's dependency array is [variable] — the variable name (e.g. "--color-success"), which never changes. When the user switches themes on the account page, the root component re-renders with a new data-theme attribute (root.tsx:146), causing CSS variables to resolve to different values. But mounted components using useThemeColor (e.g. FreePlanUsage.tsx:11-13, DashboardDialogs.tsx:179-180) keep the stale resolved color in state.
A fix would be to add the current theme to the dependency array, or to listen for data-theme attribute changes.
Prompt for agents
The useThemeColor hook resolves a CSS variable to an RGB value once on mount, but its useEffect dependency array is [variable] — only the variable name. When the user switches themes (data-theme attribute changes on <html>), the CSS variables resolve to different values, but the effect doesn't re-run.
To fix this, the hook needs to re-resolve when the theme changes. Options:
1. Read the current data-theme from document.documentElement.dataset.theme and include it in the dependency array.
2. Accept a theme prop/context value and include it in deps.
3. Use a MutationObserver on the html element's data-theme attribute to trigger re-resolution.
The simplest approach is option 1: read the data-theme attribute inside the effect and add it as a dependency. But since this is a React hook, you'd need to track the theme in state (e.g., via a context or by reading it from the root loader data via useTypedMatchesData).
Was this helpful? React with 👍 or 👎 to provide feedback.
| [data-theme="light"] { | ||
| --color-secondary: #ffffff; | ||
| --color-input-bg: #ffffff; | ||
|
|
||
| /* shadcn vars consumed by charts/streamdown (tooltip cursor fill etc.) */ | ||
| --background: #ffffff; | ||
| --foreground: #1a1b1f; | ||
| --muted: #eceef1; | ||
| --muted-foreground: #5f6570; | ||
| --border: #e2e4e9; | ||
| --sidebar: #f6f7f8; | ||
| --primary-foreground: #ffffff; | ||
|
|
||
| /* Text */ | ||
| --color-primary: var(--color-apple-600); | ||
| --color-tertiary: #eef0f3; | ||
| --color-text-link: var(--color-lavender-600); | ||
| --color-text-faint: var(--color-charcoal-400); | ||
| --color-text-dimmed: var(--color-charcoal-500); | ||
| --color-text-bright: var(--color-charcoal-800); | ||
|
|
||
| /* Surfaces */ | ||
| --color-background-deep: #f1f2f4; | ||
| --color-background-dimmed: #fbfbfc; | ||
| --color-background-bright: #ffffff; | ||
| --color-background-hover: #f2f3f5; | ||
| --color-background-raised: #e9eaee; | ||
| --color-surface-control: #dcdee3; | ||
| --color-surface-control-hover: #cfd2d9; | ||
| --color-surface-control-active: #b8bcc6; | ||
|
|
||
| /* Borders */ | ||
| --color-grid-dimmed: #eceef1; | ||
| --color-grid-bright: #e2e4e9; | ||
| --color-border-bright: #d2d5db; | ||
| --color-border-brighter: #b9bdc7; | ||
| --color-border-brightest: #9ba1ad; | ||
|
|
||
| /* Status - darker steps for contrast on white */ | ||
| --color-success: var(--color-mint-600); | ||
| --color-warning: var(--color-amber-600); | ||
| --color-dev: var(--color-pink-600); | ||
| --color-prod: var(--color-mint-600); | ||
| --color-staging: var(--color-orange-600); | ||
| --color-preview: var(--color-yellow-700); | ||
|
|
||
| /* Neutral run statuses: soft light grays for sparkbars and charts */ | ||
| --color-run-pending: #ccd0d6; | ||
| --color-run-delayed: #d3d6db; | ||
| --color-run-waiting-to-resume: #c5c9d0; | ||
| --color-run-canceled: #d8dbdf; | ||
| --color-run-expired: #dee0e4; | ||
|
|
||
| /* Icons that fail contrast on white */ | ||
| --color-schedules: var(--color-yellow-600); | ||
| --color-previewBranches: var(--color-yellow-600); | ||
| --color-customDashboards: var(--color-charcoal-500); | ||
|
|
||
| /* Callouts - deep tints instead of the dark theme's pastels */ | ||
| --color-callout-warning-text: var(--color-yellow-800); | ||
| --color-callout-error-text: var(--color-rose-700); | ||
| --color-callout-success-text: var(--color-green-800); | ||
| --color-callout-docs-text: var(--color-blue-800); | ||
| --color-callout-pending-bg: var(--color-blue-100); | ||
| --color-callout-pending-text: var(--color-blue-800); | ||
| --color-callout-pricing-bg: var(--color-indigo-100); | ||
| --color-callout-pricing-text: var(--color-indigo-800); | ||
|
|
||
| /* Code syntax - trigger.light */ | ||
| --color-code-background: #ffffff; | ||
| --color-code-foreground: #333333; | ||
| --color-code-line-number: #a8a8ad; | ||
| --color-code-plain: #2e2e4b; | ||
| --color-code-muted: #333333; | ||
| --color-code-comment: #767a81; | ||
| --color-code-keyword: #b114d3; | ||
| --color-code-storage: #b114d3; | ||
| --color-code-type: #b114d3; | ||
| --color-code-function: #6532f5; | ||
| --color-code-variable: #404040; | ||
| --color-code-constant: #1e1e1e; | ||
| --color-code-language: #b114d3; | ||
| --color-code-object-key: #222222; | ||
| --color-code-string: #262626; | ||
| --color-code-template-punctuation: #0879e2; | ||
| --color-code-number: #262626; | ||
| --color-code-builtin: #3080e0; | ||
| --color-code-attribute: #222222; | ||
| --color-code-escape: #222222; | ||
| --color-code-regexp: #dc3545; | ||
| --color-code-regexp-constant: #5f6570; | ||
| --color-code-invalid: #dc3545; | ||
| --color-code-deleted: #dc3545; | ||
| --color-code-jsx-text: #2e2e4b; | ||
|
|
||
| /* CodeMirror - trigger.light */ | ||
| --color-editor-background: #ffffff; | ||
| --color-editor-foreground: #2e2e4b; | ||
| --color-editor-keyword: #b114d3; | ||
| --color-editor-name: #197c3c; | ||
| --color-editor-function: #6532f5; | ||
| --color-editor-constant: #3080e0; | ||
| --color-editor-type: #2980b9; | ||
| --color-editor-operator: #333333; | ||
| --color-editor-comment: #767a81; | ||
| --color-editor-heading: #2c3e50; | ||
| --color-editor-string: #262626; | ||
| --color-editor-invalid: #dc3545; | ||
| --color-editor-panel-background: #f4f4f6; | ||
| --color-editor-highlight-background: #f8f8fa; | ||
| --color-editor-tooltip-background: #ffffff; | ||
| --color-editor-selection: #d9dce3; | ||
| --color-editor-cursor: #2e2e4b; | ||
| --color-editor-search-match: #0879e226; | ||
| --color-editor-search-match-outline: #0879e2; | ||
| --color-editor-search-match-selected: #0879e240; | ||
| --color-editor-selection-match: #e8e8ed; | ||
| --color-editor-matching-bracket: rgba(240, 241, 244, 0.9); | ||
| --color-editor-matching-bracket-outline: rgba(160, 166, 180, 0.5); | ||
| --color-editor-fold-placeholder: #555555; | ||
| --color-editor-scrollbar-track-active: #e8e9ec; | ||
| --color-editor-scrollbar-thumb: #c9ccd4; | ||
| --color-editor-scrollbar-thumb-active: #aeb3be; | ||
| } | ||
|
|
||
| /* Streamdown's muted surface has no semantic token (charcoal-775); theme it here */ | ||
| [data-theme="light"] .streamdown-container { | ||
| --muted: #eceef1; | ||
| } | ||
|
|
||
| /* The timeline label shadow is a dark-theme legibility aid; drop it on light */ | ||
| [data-theme="light"] .text-shadow-custom { | ||
| text-shadow: none; | ||
| } | ||
|
|
||
| /* Neutral timeline points: invert to a light dot with a gray ring */ | ||
| [data-theme="light"] .timeline-point.bg-surface-control-active { | ||
| border-color: var(--color-surface-control-active); | ||
| background-color: var(--color-background-bright); | ||
| } | ||
|
|
||
| /* Run timeline bars: no fade gradient on light */ | ||
| [data-theme="light"] .timeline-span { | ||
| background-image: none; | ||
| } | ||
| /* On saturated bars the duration label keeps the dark-theme treatment, | ||
| but only when the bar is wide enough to contain the label — on narrow | ||
| bars the sticky label overflows onto the page background, where the | ||
| default dark-on-light text is correct. */ | ||
| [data-theme="light"] .timeline-span.bg-success, | ||
| [data-theme="light"] .timeline-span.bg-error { | ||
| container-type: inline-size; | ||
| } | ||
| @container (min-width: 3.5rem) { | ||
| [data-theme="light"] .timeline-span.bg-success .text-shadow-custom, | ||
| [data-theme="light"] .timeline-span.bg-error .text-shadow-custom { | ||
| color: #ffffff; | ||
| text-shadow: 1px 1px 1px rgba(0, 0, 0, 0.5); | ||
| } |
There was a problem hiding this comment.
🔍 Tailwind dark: variant may not match the data-theme attribute selector
The PR uses dark: Tailwind variant in ~15 places across components (e.g., dark:text-rose-300 at AskAI.tsx:392, dark:bg-surface-control at ClientTabs.tsx:125, dark:text-amber-200 at AnimatedOrgBannerBar.tsx:48). In Tailwind v4, the dark: variant defaults to @media (prefers-color-scheme: dark) unless explicitly reconfigured. The theme system in this PR uses [data-theme="dark"] / [data-theme="light"] attributes on <html> (root.tsx:146). If dark: isn't configured to match [data-theme="dark"] (e.g., via @custom-variant dark), these classes would respond to the OS dark-mode preference rather than the app's theme setting, causing visual mismatches when the two disagree (e.g., OS in light mode but app set to dark theme). This needs verification against the actual Tailwind v4 config — I could not find a @custom-variant dark declaration in tailwind.css.
Was this helpful? React with 👍 or 👎 to provide feedback.
Adds an opt-in light theme, gated by the
hasThemeSwitcherfeature flag (off by default — dark stays the default theme for everyone).data-theme