fix: update dark theme colors to match Figma design system (QA clone of #5169)#5171
fix: update dark theme colors to match Figma design system (QA clone of #5169)#5171bekossy wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughDark theme color tokens were updated in ThemeContextProvider, introducing brand-yellow link colors and new background/text/border overrides plus a colorPrimaryBg token. Corresponding CSS variables, Tailwind config, and Antd theme config were updated, and layout/sidebar/banner components were adjusted to use the new tokens. ChangesDark theme color token update
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d384bd698a
ℹ️ 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".
| "subMenuItemBorderRadius": 6, | ||
| "popupBg": "#ffffff", | ||
| "itemSelectedColor": "#1c2c3d", | ||
| "itemSelectedColor": "${colorText}", |
There was a problem hiding this comment.
Resolve the Menu selected color before passing it
This JSON is loaded directly into the Ant Design theme, so "${colorText}" remains a literal string rather than being interpolated to the current text token. When a selected Menu item relies on itemSelectedColor (for example the sidebar/settings menus), Ant Design will emit an invalid CSS color and the selected item falls back to the inherited/unselected color instead of the intended selected-text contrast; use an actual color or CSS variable such as var(--ant-color-text) here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/oss/src/components/SidebarBanners/SidebarBanner.tsx (1)
32-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse theme-aware foreground colors on the new banner background.
bg-colorPrimaryBgresolves to a dark token here (ThemeContextProvider.tsxsetscolorPrimaryBgto#303012), but the banner still hardcodestext-gray-900/text-[var(--ag-c-586673)]and the dismiss control’s hover treatment. That will make the banner hard to read in dark mode. Please switch the foreground styles to semantic theme tokens or keep the lighter background for this component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d6419c9-eef7-4598-9b36-9d4e733f214c
📒 Files selected for processing (9)
web/oss/src/components/Layout/ThemeContextProvider.tsxweb/oss/src/components/Layout/assets/styles.tsweb/oss/src/components/Sidebar/Sidebar.tsxweb/oss/src/components/SidebarBanners/SidebarBanner.tsxweb/oss/src/components/pages/app-management/components/HelpAndSupportSection.tsxweb/oss/src/components/pages/app-management/components/WelcomeCardsSection/assets/components/WelcomeCard.tsxweb/oss/src/styles/theme-variables.cssweb/oss/src/styles/tokens/antd-themeConfig.jsonweb/oss/tailwind.config.ts
| // Links: light uses the navy primary, but in dark the seed is stripped and | ||
| // darkAlgorithm falls back to a harsh default blue. Pin a softer, dark-tuned | ||
| // link blue (GitHub-style) so every antd link (Typography.Link, type="link", | ||
| // anchors) reads as a clickable affordance without the jarring tone. | ||
| colorLink: "#58a6ff", | ||
| colorLinkHover: "#79b8ff", | ||
| colorLinkActive: "#3b8eea", | ||
| colorLink: "#F2F25C", | ||
| colorLinkHover: "#C2C24A", | ||
| colorLinkActive: "#919137", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update stale comment about link colors.
The comment on lines 86-89 describes "a softer, dark-tuned link blue (GitHub-style)" but the actual colorLink/colorLinkHover/colorLinkActive values are brand-yellow (#F2F25C, #C2C24A, #919137), not blue. This comment appears to be leftover from a previous version and will confuse future developers.
📝 Proposed comment update
// Links: light uses the navy primary, but in dark the seed is stripped and
// darkAlgorithm falls back to a harsh default blue. Pin a softer, dark-tuned
- // link blue (GitHub-style) so every antd link (Typography.Link, type="link",
- // anchors) reads as a clickable affordance without the jarring tone.
+ // brand-yellow link so every antd link (Typography.Link, type="link",
+ // anchors) reads as a clickable affordance consistent with the brand.
colorLink: "`#F2F25C`",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Links: light uses the navy primary, but in dark the seed is stripped and | |
| // darkAlgorithm falls back to a harsh default blue. Pin a softer, dark-tuned | |
| // link blue (GitHub-style) so every antd link (Typography.Link, type="link", | |
| // anchors) reads as a clickable affordance without the jarring tone. | |
| colorLink: "#58a6ff", | |
| colorLinkHover: "#79b8ff", | |
| colorLinkActive: "#3b8eea", | |
| colorLink: "#F2F25C", | |
| colorLinkHover: "#C2C24A", | |
| colorLinkActive: "#919137", | |
| // Links: light uses the navy primary, but in dark the seed is stripped and | |
| // darkAlgorithm falls back to a harsh default blue. Pin a softer, dark-tuned | |
| // brand-yellow link so every antd link (Typography.Link, type="link", | |
| // anchors) reads as a clickable affordance consistent with the brand. | |
| colorLink: "`#F2F25C`", | |
| colorLinkHover: "`#C2C24A`", | |
| colorLinkActive: "`#919137`", |
Railway Preview Environment
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d384bd698a
ℹ️ 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".
| // Surfaces / text / border come from darkAlgorithm by default. Uncomment to tune: | ||
| // colorBgContainer: "#141414", | ||
| colorBgBase: "#000000", | ||
| colorBgContainer: "#11100F", |
There was a problem hiding this comment.
Keep dark Drawer surface in sync with container
When dark mode opens any AntD Drawer, the component override below still pins Drawer.colorBgElevated to the old #141414 value. AntD Drawers render their body/content with colorBgElevated, so after this line moves the app container surface to #11100F, drawer panels no longer match the container-colored content surfaces this override was meant to align with; update the Drawer override to the new container token/value as part of the palette change.
Useful? React with 👍 / 👎.
antd-themeConfig.json shipped "${colorText}" as a literal string instead of
a resolved value — nothing in the codebase interpolates that placeholder,
so it rendered as an invalid CSS color in light mode. Restore the literal
"#1c2c3d", matching the other two itemSelectedColor entries in the file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Clone of #5169 for preview-environment QA in dark mode (fork PRs don't trigger preview deploys). See #5169 for full context and review discussion — this PR exists solely to get a preview build.
Test plan