Repository navigation
fix: color utils get css color from theme mode - #331
Merged
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
bdjulbic
changed the title
bdjulbic
changed the title
ilyabo
reviewed
Jan 28, 2026
| * @param currentMatch - The last match found so far (used internally for recursion to accumulate results across nested rules) | ||
| * @returns The last matching CSS variable value, or undefined if not found | ||
| */ | ||
| function walkCssRulesForVariable( |
Collaborator
Author
There was a problem hiding this comment.
Most probably it will,
I updated some projects from tailwind 3 to tailwind 4 and it was never a smooth ride.
So ill wait for that PR to merge?
Collaborator
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
📝 WalkthroughWalkthroughRemoves stylesheet/element-based CSS variable helpers and centralizes runtime DOM reads for color resolution; simplifies signatures for Changes
Sequence Diagram(s)sequenceDiagram
participant App as React App
participant ME as MonacoEditor
participant DOM as document (CSS vars)
participant Monaco as monaco.editor
App->>ME: render(props: isDark, monacoTheme, language)
ME->>ME: beforeMount / onMount
ME->>DOM: getCssColor(...) — read CSS variables from document.documentElement
ME->>Monaco: defineMonacoThemes(monaco, isDark, monacoTheme)
Monaco-->>ME: themes registered (sqlrooms-*, json themes)
ME->>Monaco: setTheme(activeTheme) (guarded by lastDefinedForDarkMode)
App->>ME: props update (isDark / monacoTheme / language)
ME->>DOM: re-read CSS variables via getCssColor
ME->>Monaco: re-run defineMonacoThemes + setTheme (if needed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/monaco-editor/src/utils/color-utils.ts (1)
127-283: Reduce duplicate mappings ingetMenuColors.
The dark/light branches are identical now that values come from CSS vars; building the map once will avoid drift.♻️ Suggested refactor
- if (isDarkTheme) { - // Dark theme mappings - result['editorWidget.background'] = getCssColor( - '--popover', - defaults['editorWidget.background'], - ); - result['editorWidget.foreground'] = getCssColor( - '--popover-foreground', - defaults['editorWidget.foreground'], - ); - result['editorWidget.border'] = getCssColor( - '--border', - defaults['editorWidget.border'], - ); - result['editorSuggestWidget.background'] = getCssColor( - '--popover', - defaults['editorSuggestWidget.background'], - ); - result['list.hoverBackground'] = getCssColor( - '--accent', - defaults['list.hoverBackground'], - ); - result['list.highlightForeground'] = getCssColor( - '--primary', - defaults['list.highlightForeground'], - ); - result['menu.background'] = getCssColor( - '--popover', - defaults['menu.background'], - ); - result['menu.foreground'] = getCssColor( - '--popover-foreground', - defaults['menu.foreground'], - ); - result['menu.selectionBackground'] = getCssColor( - '--accent', - defaults['menu.selectionBackground'], - ); - result['menu.selectionForeground'] = getCssColor( - '--accent-foreground', - defaults['menu.selectionForeground'], - ); - result['quickInput.background'] = getCssColor( - '--popover', - defaults['quickInput.background'], - ); - result['quickInput.foreground'] = getCssColor( - '--popover-foreground', - defaults['quickInput.foreground'], - ); - result['dropdown.background'] = getCssColor( - '--popover', - defaults['dropdown.background'], - ); - result['dropdown.foreground'] = getCssColor( - '--popover-foreground', - defaults['dropdown.foreground'], - ); - } else { - // Light theme mappings - result['editorWidget.background'] = getCssColor( - '--popover', - defaults['editorWidget.background'], - ); - result['editorWidget.foreground'] = getCssColor( - '--popover-foreground', - defaults['editorWidget.foreground'], - ); - result['editorWidget.border'] = getCssColor( - '--border', - defaults['editorWidget.border'], - ); - result['editorSuggestWidget.background'] = getCssColor( - '--popover', - defaults['editorSuggestWidget.background'], - ); - result['list.hoverBackground'] = getCssColor( - '--accent', - defaults['list.hoverBackground'], - ); - result['list.highlightForeground'] = getCssColor( - '--primary', - defaults['list.highlightForeground'], - ); - result['menu.background'] = getCssColor( - '--popover', - defaults['menu.background'], - ); - result['menu.foreground'] = getCssColor( - '--popover-foreground', - defaults['menu.foreground'], - ); - result['menu.selectionBackground'] = getCssColor( - '--accent', - defaults['menu.selectionBackground'], - ); - result['menu.selectionForeground'] = getCssColor( - '--accent-foreground', - defaults['menu.selectionForeground'], - ); - result['quickInput.background'] = getCssColor( - '--popover', - defaults['quickInput.background'], - ); - result['quickInput.foreground'] = getCssColor( - '--popover-foreground', - defaults['quickInput.foreground'], - ); - result['dropdown.background'] = getCssColor( - '--popover', - defaults['dropdown.background'], - ); - result['dropdown.foreground'] = getCssColor( - '--popover-foreground', - defaults['dropdown.foreground'], - ); - } + Object.assign(result, { + 'editorWidget.background': getCssColor( + '--popover', + defaults['editorWidget.background'], + ), + 'editorWidget.foreground': getCssColor( + '--popover-foreground', + defaults['editorWidget.foreground'], + ), + 'editorWidget.border': getCssColor('--border', defaults['editorWidget.border']), + 'editorSuggestWidget.background': getCssColor( + '--popover', + defaults['editorSuggestWidget.background'], + ), + 'list.hoverBackground': getCssColor('--accent', defaults['list.hoverBackground']), + 'list.highlightForeground': getCssColor( + '--primary', + defaults['list.highlightForeground'], + ), + 'menu.background': getCssColor('--popover', defaults['menu.background']), + 'menu.foreground': getCssColor('--popover-foreground', defaults['menu.foreground']), + 'menu.selectionBackground': getCssColor( + '--accent', + defaults['menu.selectionBackground'], + ), + 'menu.selectionForeground': getCssColor( + '--accent-foreground', + defaults['menu.selectionForeground'], + ), + 'quickInput.background': getCssColor( + '--popover', + defaults['quickInput.background'], + ), + 'quickInput.foreground': getCssColor( + '--popover-foreground', + defaults['quickInput.foreground'], + ), + 'dropdown.background': getCssColor('--popover', defaults['dropdown.background']), + 'dropdown.foreground': getCssColor( + '--popover-foreground', + defaults['dropdown.foreground'], + ), + });
bdjulbic
commented
Feb 2, 2026
Comment on lines
78
to
79
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
This file was rollbacked to the version before I edited my edits for the Monaco editor flashing.
Collaborator
Author
|
@coderabbitai review |
Contributor
|
✅ Actions performedReview triggered.
|
bdjulbic
commented
Feb 2, 2026
|
|
||
| let themesDefined = false; | ||
| // Module-level singleton to track theme state | ||
| let lastDefinedForDarkMode: boolean | null = null; |
Collaborator
Author
There was a problem hiding this comment.
This guard was needed to prevent theme redefining every time a component was created.
Like a singleton value to keep track of property isDarkMode
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts Monaco editor theming to derive colors directly from the live DOM (via CSS variables) instead of walking stylesheets or manually simulating theme modes, aiming to fix incorrect background colors under Tailwind @layer base and keep the editor synced with the app theme.
Changes:
- Simplified
color-utilsby removing stylesheet rule inspection and introducinggetCssColorthat reads CSS variable values fromgetComputedStyle(document.documentElement). - Updated
getMenuColorsandgetJsonEditorThemeto use the newgetCssColorand rely on the current DOM theme instead of explicit theme-mode arguments. - Refactored
MonacoEditortheme setup to define all Monaco themes from current DOM CSS vars, track the last dark/light mode at module level, and re-define/apply themes when the app theme or editor configuration changes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/monaco-editor/src/utils/color-utils.ts |
Reworked color resolution helpers to read live CSS variables from the DOM and simplified menu/JSON theme generation APIs to depend on current theme state. |
packages/monaco-editor/src/components/MonacoEditor.tsx |
Refactored theme definition logic to use the new color utilities, cache last dark/light mode, and re-apply Monaco themes when relevant props or app theme change. |
Comments suppressed due to low confidence (1)
packages/monaco-editor/src/utils/color-utils.ts:103
- The PR description says a recursive search via
getRawCssVarFromStylesheets()was added to fix colors inside@layer base, but in this file that function is removed and the implementation now relies solely ongetComputedStyle(document.documentElement)viagetCssColor. Please either update the PR description to match this approach or reintroduce and use the recursive stylesheet walk if that behavior is still intended, so future readers aren’t misled about how theme colors are actually resolved.
export function getCssColor(
variableName: string,
fallbackColor: string,
): string {
if (typeof document === 'undefined') return fallbackColor;
try {
// Get CSS variable value from current DOM
const cssValue = getComputedStyle(document.documentElement)
.getPropertyValue(variableName)
.trim();
if (!cssValue) return fallbackColor;
return normalizeCssColorValue(cssValue, fallbackColor);
} catch (error) {
console.error(`Error getting CSS variable ${variableName}:`, error);
return fallbackColor;
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ilyabo
approved these changes
Feb 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After the previous PR merge regarding Monaco editor flashing,
an issue appeared where theme colors weren't getting resolved properly.
This PR rollbacks file color-utils.ts,
and instead makes sure the theme is once defined in MonacoEditor, instead of getting redefined every time MonacoEditor was created which caused the initial issue with flashing
Example of editor background, before the color was #09090b (very dark) and after update its #020817 (blueish black)
Summary by CodeRabbit
New Features
Refactor
Bug Fixes