Fix dark mode alert styling - #3586
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAlert dark-mode tokens now use status-color mixes with the dark background. Alert text, icons, and content now use the theme foreground. Tests cover generated tokens and server-rendered error alerts. ChangesAlert color modes
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0935bb925b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "--alert-warning-bg": "color-mix(in oklch,var(--status-warning) 18%,var(--background))", | ||
| "--alert-error-bg": "color-mix(in oklch,var(--status-error) 18%,var(--background))", | ||
| "--alert-success-bg": "color-mix(in oklch,var(--status-success) 18%,var(--background))", |
There was a problem hiding this comment.
Keep status badges legible with the new dark fills
In dark mode, these shared tokens also provide the backgrounds for the success, warning, and destructive Badge variants in badge.tsx:19-21, while their text remains var(--status-*). Mixing only 18% of that same status color into #282828 makes the destructive and success combinations approximately 2.3:1 and 2.7:1 contrast, respectively, so badges become difficult to read even though alerts now use the light theme foreground. Use alert-specific dark surface tokens or update the badge foregrounds when applying these fills.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What changed
Why
Alert surfaces reused the light pastel fills in dark mode. Their child content then mixed dark-mode background text with independently styled foreground actions, producing the washed-out, inconsistent error banner shown in chat.
The light-mode values remain unchanged. Dark mode now uses muted status-tinted surfaces with a consistent light foreground.
Visual verification
Matched 1200×700 browser renders were compared with the same alert text and layout:
rgb(236, 211, 209)with dark message text and a light Retry actionoklch(0.326059 0.0396107 25.0399)with message and Retry both usingrgb(240, 239, 233)The repository Storybook currently fails on
mainbefore rendering this story because Vite externalizesnode:async_hooks; the comparison harness imported the real generated token CSS and used the alert's exact token bindings.Validation
verify:quickreached the existing API reference check, which reports the checked-in API docs are stale across all 42 module groups onmainSummary by CodeRabbit
Bug Fixes
Tests