fix: CodeMirror and code surfaces follow light/dark theme#29
fix: CodeMirror and code surfaces follow light/dark theme#29
Conversation
- Set :root code surface tokens to light bg/dark text; keep .dark overrides - Add CSS variables for gutter, active line, and code chrome borders - Wire CodeMirror theme and code-renderer toolbar to those tokens - Theme .artifact-action.is-code and markdown comment hljs for light code bg Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Deploying agent-render with
|
| Latest commit: |
7dd69c3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4224fa1a.agent-render.pages.dev |
| Branch Preview URL: | https://cursor-codemirror-light-mode.agent-render.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces hardcoded code-surface colors with new light/dark CSS tokens (including "code chrome", Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Pull request overview
Aligns CodeMirror/code UI surfaces with the app’s light/dark theme tokens so light mode no longer renders code areas with dark editor styling, while preserving the prior dark appearance.
Changes:
- Updates
:rootcode-surface tokens to be light by default and adds “chrome” variables for borders/gutters/active line highlights, with dark overrides under.dark. - Applies the new variables to CodeMirror gutters and active-line styling, and to the code renderer shell/toolbar transitions.
- Makes code toolbar pills and
.artifact-action.is-codetheme-aware (with an explicit.darkoverride to keep prior visuals).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/components/renderers/code-renderer.tsx |
Uses new CSS variables for CodeMirror gutter/active-line colors and replaces hard-coded pill colors with theme-driven CSS vars. |
src/app/globals.css |
Switches default code surfaces to light, introduces additional code “chrome” tokens, and updates code shell/toolbar/HLJS comment styling to follow theme. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Define --rb-0..--rb-5 in :root (darker) and .dark (original pastels); editor theme uses var(--rb-n) so contrast follows code surface. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
There was a problem hiding this comment.
Code Review Summary
Status: Issues Fixed | Recommendation: Merge
The PR correctly addresses the rainbow bracket readability issue that was flagged in the existing Devin comment (line 1). The solution uses CSS variables (--rb-0 through --rb-5) defined in globals.css with:
- Darker saturated hues for light mode (
:root) - Original bright pastels for dark mode (
.dark)
These variables are then used in code-renderer.tsx via var(--rb-${index}) instead of hardcoded hex values.
Files Reviewed (2 files)
src/app/globals.css- Light/dark code surfaces, chrome variables, and rainbow bracket color tokens properly implementedsrc/components/renderers/code-renderer.tsx- CodeMirror theme and toolbar now use CSS variables for theming
Existing Comment Resolution
The existing Devin comment at src/components/renderers/code-renderer.tsx:1 about rainbow bracket colors being unreadable on light code surface has been properly addressed by this PR through the CSS variable approach.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/globals.css (1)
916-926: Consider tokenizing the dark.artifact-action.is-codeoverride for consistency.This works as-is, but keeping dark values in
color-mix(...)form (or dedicated vars) will make future palette tuning easier and reduce one-off RGBA constants.♻️ Optional refactor
.dark .artifact-action.is-code { - border-color: rgba(239, 243, 247, 0.12); - background: rgba(255, 255, 255, 0.04); - color: rgba(239, 243, 247, 0.86); + border-color: color-mix(in srgb, var(--surface-code-text) 12%, transparent); + background: color-mix(in srgb, var(--surface-code-text) 4%, transparent); + color: color-mix(in srgb, var(--surface-code-text) 86%, transparent); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 916 - 926, The dark-theme override for .artifact-action.is-code uses hard-coded rgba(...) colors; refactor it to use the same tokenized approach as the light rule by either converting those rgba values into dedicated CSS variables (e.g., --surface-code-chrome-border-dark, --surface-code-bg-dark, --surface-code-text-dark) or expressing them with color-mix(...) like the light rule so palette tuning is consistent; update the .dark .artifact-action.is-code selector to reference those vars or color-mix(...) expressions and ensure the newly introduced vars are defined alongside the other surface/code variables.src/components/renderers/code-renderer.tsx (1)
251-256: Optional: extract the long arbitrary-value class strings into semantic CSS classes.The current classes work, but moving them to named classes would improve readability and reduce inline styling complexity.
♻️ Optional refactor sketch
- <span className="mono-pill !border-[color:var(--surface-code-chrome-border)] !bg-[color-mix(in_srgb,var(--surface-code-text)_6%,transparent)] !text-[color:var(--surface-code-text)]"> + <span className="mono-pill code-renderer-language-pill"> {language} </span> - <span className="section-kicker !text-[color:color-mix(in_srgb,var(--surface-code-text)_56%,transparent)]"> + <span className="section-kicker code-renderer-readonly-label"> read-only codemirror </span>/* in src/app/globals.css */ .code-renderer-language-pill { border-color: var(--surface-code-chrome-border) !important; background: color-mix(in srgb, var(--surface-code-text) 6%, transparent) !important; color: var(--surface-code-text) !important; } .code-renderer-readonly-label { color: color-mix(in srgb, var(--surface-code-text) 56%, transparent) !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/renderers/code-renderer.tsx` around lines 251 - 256, Extract the long inline arbitrary-value class strings on the two spans in CodeRenderer (the span currently with "mono-pill !border-[color:var(--surface-code-chrome-border)] !bg-[color-mix(in_srgb,var(--surface-code-text)_6%,transparent)] !text-[color:var(--surface-code-text)]" and the span with "section-kicker !text-[color:color-mix(in_srgb,var(--surface-code-text)_56%,transparent)]") into semantic CSS classes (e.g., .code-renderer-language-pill and .code-renderer-readonly-label) in your global stylesheet (src/app/globals.css), preserve the !important rules and color-mix values exactly, then replace the long inline parts with those new class names while keeping the existing utility classes (mono-pill, section-kicker) on the respective elements so CodeRenderer continues to render identically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/globals.css`:
- Around line 916-926: The dark-theme override for .artifact-action.is-code uses
hard-coded rgba(...) colors; refactor it to use the same tokenized approach as
the light rule by either converting those rgba values into dedicated CSS
variables (e.g., --surface-code-chrome-border-dark, --surface-code-bg-dark,
--surface-code-text-dark) or expressing them with color-mix(...) like the light
rule so palette tuning is consistent; update the .dark .artifact-action.is-code
selector to reference those vars or color-mix(...) expressions and ensure the
newly introduced vars are defined alongside the other surface/code variables.
In `@src/components/renderers/code-renderer.tsx`:
- Around line 251-256: Extract the long inline arbitrary-value class strings on
the two spans in CodeRenderer (the span currently with "mono-pill
!border-[color:var(--surface-code-chrome-border)]
!bg-[color-mix(in_srgb,var(--surface-code-text)_6%,transparent)]
!text-[color:var(--surface-code-text)]" and the span with "section-kicker
!text-[color:color-mix(in_srgb,var(--surface-code-text)_56%,transparent)]") into
semantic CSS classes (e.g., .code-renderer-language-pill and
.code-renderer-readonly-label) in your global stylesheet (src/app/globals.css),
preserve the !important rules and color-mix values exactly, then replace the
long inline parts with those new class names while keeping the existing utility
classes (mono-pill, section-kicker) on the respective elements so CodeRenderer
continues to render identically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bc57d4d-83fa-43b6-9473-f251b2409aeb
📒 Files selected for processing (2)
src/app/globals.csssrc/components/renderers/code-renderer.tsx
- Add --surface-code-is-code-* vars in :root and .dark; single .artifact-action.is-code rule - Replace Tailwind arbitrary colors with .code-renderer-language-pill and .code-renderer-readonly-label Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Light --surface-code is #f8f6fa; document --surface-code-text for both themes. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/renderers/code-renderer.tsx (1)
161-168:⚠️ Potential issue | 🟡 MinorAdd
.catch()handler for unhandled promise rejections from dynamic imports.At line 164,
loadLanguageSupport(language)uses dynamic imports internally without error wrapping. If an import fails, the promise rejects with no catch handler, causing an unhandled rejection. Add a.catch(...)to gracefully handle import failures (or suppress warnings by logging):Example fix
void loadLanguageSupport(language).then((extension) => { if (!cancelled) { setLanguageExtension(extension); } }).catch(() => { // Import failed; proceed with null extension (plain text fallback) if (!cancelled) { setLanguageExtension(null); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/renderers/code-renderer.tsx` around lines 161 - 168, The useEffect call invoking loadLanguageSupport(language) can produce unhandled promise rejections from dynamic imports; update the promise chain on loadLanguageSupport(...) (inside the useEffect that sets languageExtension) to add a .catch handler that handles import failures by, when not cancelled, calling setLanguageExtension(null) or logging the error, and avoid leaving rejections unhandled; ensure the cancelled flag is still respected in the .catch branch so you don't update state after unmount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/renderers/code-renderer.tsx`:
- Around line 161-168: The useEffect call invoking loadLanguageSupport(language)
can produce unhandled promise rejections from dynamic imports; update the
promise chain on loadLanguageSupport(...) (inside the useEffect that sets
languageExtension) to add a .catch handler that handles import failures by, when
not cancelled, calling setLanguageExtension(null) or logging the error, and
avoid leaving rejections unhandled; ensure the cancelled flag is still respected
in the .catch branch so you don't update state after unmount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dea79914-c304-4d3d-9b60-bef9b7b28146
📒 Files selected for processing (3)
.impeccable.mdsrc/app/globals.csssrc/components/renderers/code-renderer.tsx
✅ Files skipped from review due to trivial changes (1)
- .impeccable.md
Avoid unhandled rejections from dynamic language imports; reset extension to null when load fails and skip state updates after unmount. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Summary
CodeMirror and related code UI used dark
--surface-code*values on:root, so light mode still looked like a dark editor. This aligns default tokens with the light shell and adds small chrome variables so gutters, active-line highlights, borders, and toolbar styling track the theme.Rainbow bracket highlights use
--rb-0..--rb-5: darker saturated hues on the light code surface, original bright pastels under.dark.Code toolbar polish:
--surface-code-is-code-*tokens drive.artifact-action.is-codein both themes; language pill and readonly label use semantic classes instead of long Tailwind arbitrary values.Design context:
.impeccable.mdtoken table updated so the design reference matches shippedglobals.css.Robustness:
loadLanguageSupportinCodeRenderernow uses.catchso dynamic import failures do not surface as unhandled rejections; on failure (when not cancelled) extension resets tonullfor baseline highlighting.Changes
globals.css,code-renderer.tsx,.impeccable.md: (as above)Testing
npm run lintpasses.Notes
Theme toggle updates
htmlclass; CodeMirror readsvar(...)at paint time.Summary by CodeRabbit