fix(admin): make CodeMirror respect dark mode in custom scripts#38599
fix(admin): make CodeMirror respect dark mode in custom scripts#38599Pradeep-10x wants to merge 3 commits into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
WalkthroughThe CodeMirror component was refactored to support dark mode by detecting theme via a hook, capture the initial editor theme, load a material-darker CSS theme, synchronize prop-driven value changes to internal state and the editor, and move editor teardown to an unmount effect without changing public APIs. 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. 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx`:
- Around line 56-58: The callback ref for the CodeMirror textarea fails to clean
up a previous editor instance when React invokes the old ref with null; update
the ref callback used in CodeMirror.tsx to check for the `node === null` case
and call the existing editor cleanup (`editorRef.current?.toTextArea()` or
equivalent) before returning, and when creating a new instance use
`editorRef.current = CodeMirror.fromTextArea(...)`; ensure `editorRef` is the
single source of truth so the old instance is properly destroyed on ref
re-invocation.
- Line 111: The textAreaRef callback currently includes isDark in its dependency
array which causes the CodeMirror editor to be destroyed/recreated on every
theme toggle; remove isDark from the dependency list of the textAreaRef callback
so the editor initialization only runs on mount/element change, and keep theme
updates handled by the separate effect that calls setOption('theme', ...) (the
effect around lines 130–140). Also change the initial theme capture used during
editor creation (the logic at ~line 86) to use a static initial value or read
isDark once into a ref so the mount-time initialization does not capture a stale
isDark.
🧹 Nitpick comments (2)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx (2)
115-115: Remove inline code comments.Lines 115 and 130 contain implementation comments (
// Sync external value → editorand// React to dark mode changes). As per coding guidelines, "Avoid code comments in the implementation".Also applies to: 130-130
116-128: Consider collapsing the two value-sync effects into one.The intermediate
setValue(valueProp)on line 117 triggers a re-render, which then triggers the second effect to push into the editor. You could sync directly fromvaluePropto the editor in a single effect and keepsetValueonly for theonChangepath:Proposed simplification
- // Sync external value → editor - useEffect(() => { - setValue(valueProp); - }, [valueProp]); - useEffect(() => { if (!editorRef.current) { return; } - if (value !== editorRef.current.getValue()) { - editorRef.current.setValue(value ?? ''); + if (valueProp !== editorRef.current.getValue()) { + editorRef.current.setValue(valueProp ?? ''); } - }, [value]); + }, [valueProp]);This avoids an extra render cycle when the parent passes a new value.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (2)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx (2)
130-140: Dark mode effect looks good.Clean reactive approach using
setOptionto toggle the CodeMirror theme without re-initialization.
142-150: Rendered textarea element looks correct.The hidden textarea with
readOnlyand callback ref is the standard pattern forCodeMirror.fromTextArea. No issues here.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx`:
- Around line 51-55: The theme can become stale because initialThemeRef is
captured at mount; change the editor init to read isDark directly at creation
time and immediately set the editor's theme option after creating the editor
(replace use of initialThemeRef with a local isDark read inside the async init
callback used to import and call fromTextArea), and ensure the theme-sync effect
(which checks editorRef.current) still updates on subsequent isDark changes;
this closes the race where isDark flips while the async import/fromTextArea is
in flight.
- Around line 120-127: The effect that syncs external value into the editor
(useEffect that calls editorRef.current.setValue) causes CodeMirror to emit a
change event which triggers the existing change handler (handleChange/onChange)
and echoes the same value back to the parent; fix this by introducing a
temporary guard flag (e.g., isSettingValueRef) that you set true immediately
before calling editorRef.current.setValue(value ?? '') in the useEffect and set
back to false immediately after, and update the change listener (the handler
referenced around lines 90–94) to early-return when isSettingValueRef.current is
true so external setValue does not invoke onChange.
🧹 Nitpick comments (1)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx (1)
52-55: Remove inline code comments.The coding guidelines for this repository require avoiding code comments in implementation files. Lines 52, 72, 87, 115, 120, and 129 all contain comments (
// Capture initial theme once,// added,// Sync external value → state, etc.) that should be removed.As per coding guidelines:
**/*.{ts,tsx,js}: "Avoid code comments in the implementation".Also applies to: 72-72, 87-87, 115-115, 120-120, 129-129
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-19T18:08:13.423Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38169
File: packages/ui-client/src/hooks/useGoToDirectMessage.ts:20-27
Timestamp: 2026-01-19T18:08:13.423Z
Learning: In React hooks, including custom hooks like useUserSubscriptionByName, must always be called unconditionally and in the same order on every render, per the Rules of Hooks. Passing empty strings or fallback values to hooks is the correct approach when dealing with potentially undefined values, rather than conditionally calling the hook based on whether the value exists.
Applied to files:
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (2)
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx (2)
140-147: Good: dedicated unmount effect for editor teardown.Separating the cleanup into its own
useEffect(…, [])is clearer than embedding it in the callback ref. ThetoTextArea()call restores the original textarea and the ref is properly nulled.
149-157: Rendering looks correct.The hidden textarea with the controlled
valueand the forwardedrefis the standard CodeMirror integration pattern.readOnlyon the textarea prevents user edits on the raw element before CodeMirror takes over.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Proposed changes (screenshots)
This PR fixes an issue where the CodeMirror editor used in Administration → Layout → Custom Scripts did not adapt to Rocket.Chat’s dark mode.
Previously, the surrounding admin UI correctly switched themes, but the CodeMirror editor always rendered using a light theme, causing visual inconsistency and reduced readability in dark mode.
With this change, the editor now:
This ensures a consistent and comfortable editing experience across light and dark themes.
Issue #38562
Steps to test or reproduce
Further comments
Screenshot :
Summary by CodeRabbit
New Features
Bug Fixes