Code Editor [WIP} #589
Code Editor [WIP} #589
Conversation
…t unnecessary code comments
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces Monaco Editor integration as a replacement for the existing Blocks component in the Code tab. Changes include new dependencies ( Changes
Sequence DiagramsequenceDiagram
actor User
participant GUI
participant MonacoBlocks
participant Editor as Monaco Editor
participant Redux
participant ExtLib as Extension Library
participant Prompt
User->>GUI: Open Code Tab
GUI->>MonacoBlocks: Render with isVisible=true
MonacoBlocks->>Editor: Mount & Initialize
Editor->>MonacoBlocks: onMount callback
MonacoBlocks->>Editor: Configure Diagnostics & Layout
User->>Editor: Edit Code
Editor->>MonacoBlocks: onChange event
MonacoBlocks->>Redux: Dispatch onCodeChange
Redux->>MonacoBlocks: Update code prop
alt Extension Library Flow
User->>ExtLib: Select Category
ExtLib->>MonacoBlocks: onCategorySelected
MonacoBlocks->>Redux: Dispatch onOpenConnectionModal
Redux->>Prompt: Show Modal Dialog
end
alt Prompt Interaction
MonacoBlocks->>Prompt: handlePromptStart(message)
User->>Prompt: Provide Input
Prompt->>MonacoBlocks: handlePromptCallback(input)
MonacoBlocks->>Prompt: Close Dialog
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 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 |
🎉 MEGA PR Test & Analysis Complete!Hi @supervoidcoder! Your PR has been fully tested and analyzed! 🚀✨ 🌍 Test DeploymentLive Preview: https://OmniBlocks.github.io/scratch-gui/supervoidcoder/589/ 🧪 Test ResultsESLint: ❌ Found issues
To fix, click the checkbox below or type
Unit Tests: ❌
Integration Tests: ❌
📄 Full test outputs available in artifacts 🔦 Performance Audit (Lighthouse)✅ Audit completed successfully! 📊 Lighthouse Scores:
⚡ Core Web Vitals:
🟢 90-100 (Good) | 🟠 50-89 (Needs Improvement) | 🔴 0-49 (Poor) 📄 Full report in artifacts 📦 Bundle Size AnalysisTotal Build Size: 185M 📦 Top 5 Largest JavaScript Files:
🎨 Top 5 Largest CSS Files:
♿ Accessibility Testing✅ Accessibility scan completed!
📊 Build Details
💡 Pro Tip: Only one build was needed for all these checks - saving CI/CD minutes! 🎯 |
🎬 ULTRA Visual Regression Test ResultsHi @supervoidcoder! Tested across 9 combinations (3 platforms × 3 browsers). 8/9 completed. 📊 Test Matrix Summary
|
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.55% (60344 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.60% (60344 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.55% (60344 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (308 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (308 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.17% (56819 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.15% (56698 pixels)
Ubuntu + WebKit
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.45% (237869 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.43% (237885 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.45% (237869 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1011 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1011 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 1.51% (55612 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.04% (222525 pixels)
Windows + Chrome
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.41% (59082 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.36% (59082 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.41% (59082 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (256 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (256 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 41.20% (379730 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.03% (55555 pixels)
Windows + Firefox
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.40% (59006 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.34% (59006 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.40% (59006 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.02% (229 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.02% (229 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.16% (1471 pixels)
Player View
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 2.89% (48208 pixels)
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.00% (55321 pixels)
Windows + WebKit
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.50% (239463 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.51% (239480 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.50% (239463 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1076 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1076 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.29% (10578 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.07% (223890 pixels)
Apple + Chrome
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.49% (59844 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.51% (59844 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.49% (59844 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (281 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (281 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 26.12% (240740 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.09% (56120 pixels)
Apple + Firefox
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.48% (59761 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.49% (59761 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.48% (59761 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (270 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (270 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.30% (2726 pixels)
Player View
✅ No pixel difference detected.
Extension Library
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.28% (2584 pixels)
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.08% (56022 pixels)
Apple + WebKit
Editor Initial
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.46% (238110 pixels)
Blocks Workspace
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 11.45% (238122 pixels)
Stage Sprites
✅ No pixel difference detected.
Code Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.46% (238110 pixels)
Costumes Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1200 pixels)
Sounds Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.03% (1200 pixels)
Songs Tab
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 0.02% (834 pixels)
Player View
✅ No pixel difference detected.
Extension Library
✅ No pixel difference detected.
Advanced Settings
| Base | PR | Diff |
|---|---|---|
 |  |  |
Diff: 6.04% (222787 pixels)
📦 Resources
|
@coderabbitai can you gimme a good ol' review pls 😁 i've worked so hard on this lol |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR introduces Monaco Editor integration for OmniBlocks but has several critical issues that prevent it from being merge-ready:
🚨 Critical Issues
- Broken Redux Integration: The code editor component references non-existent Redux state properties (
state.scratchGui.codeContent,state.scratchGui.editorLanguage) and dispatches undefined action types (UPDATE_CODE_CONTENT) - Component Architecture Mismatch: Replacing
BlocksComponentwithCodeComponentbreaks the visual blocks editor functionality - Missing PropTypes: The
themeprop is used but not declared in PropTypes
🔧 Required Fixes
- Add proper Redux reducers and actions for code editor state management
- Fix PropTypes declarations to include all used props
- Remove unused imports to clean up the codebase
- Consider architectural approach - this appears to replace blocks editor rather than complement it
📝 Code Quality
- Excessive commentary in
sb3-downloader.jsxshould be condensed - Webpack configuration changes look appropriate for Monaco Editor integration
The Monaco Editor integration itself appears technically sound, but the Redux state management and component integration need significant work before this can be merged. Consider creating the necessary Redux infrastructure first, then integrating the editor as a separate tab rather than replacing the blocks editor.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| MonacoBlocks.propTypes = { | ||
| code: PropTypes.string, | ||
| extensionLibraryVisible: PropTypes.bool, | ||
| isVisible: PropTypes.bool, | ||
| language: PropTypes.string, | ||
| onCodeChange: PropTypes.func, | ||
| onOpenConnectionModal: PropTypes.func, | ||
| onRequestCloseExtensionLibrary: PropTypes.func | ||
| }; |
There was a problem hiding this comment.
🛑 Missing PropType: The theme prop is used in the component but not declared in PropTypes. This will cause runtime warnings and makes the component interface unclear.
| MonacoBlocks.propTypes = { | |
| code: PropTypes.string, | |
| extensionLibraryVisible: PropTypes.bool, | |
| isVisible: PropTypes.bool, | |
| language: PropTypes.string, | |
| onCodeChange: PropTypes.func, | |
| onOpenConnectionModal: PropTypes.func, | |
| onRequestCloseExtensionLibrary: PropTypes.func | |
| }; | |
| MonacoBlocks.propTypes = { | |
| code: PropTypes.string, | |
| extensionLibraryVisible: PropTypes.bool, | |
| isVisible: PropTypes.bool, | |
| language: PropTypes.string, | |
| onCodeChange: PropTypes.func, | |
| onOpenConnectionModal: PropTypes.func, | |
| onRequestCloseExtensionLibrary: PropTypes.func, | |
| theme: PropTypes.object.isRequired | |
| }; |
| code: state.scratchGui.codeContent || '', | ||
| language: state.scratchGui.editorLanguage || 'javascript' | ||
| }); |
There was a problem hiding this comment.
🛑 Redux State Access Error: The Redux state is accessing state.scratchGui.codeContent and state.scratchGui.editorLanguage but these state properties don't exist in the current Redux store structure. This will cause the component to always receive empty values.
| }, | ||
| onRequestCloseExtensionLibrary: () => dispatch(closeExtensionLibrary()), | ||
|
|
||
| onCodeChange: (value) => dispatch({type: 'UPDATE_CODE_CONTENT', content: value}) |
There was a problem hiding this comment.
🛑 Redux Action Type Error: The action type 'UPDATE_CODE_CONTENT' is dispatched but this action type is not defined in any reducer. This will cause the Redux store to ignore these actions and the code changes won't be persisted.
| import bindAll from 'lodash.bindall'; | ||
| import PropTypes from 'prop-types'; | ||
| import React from 'react'; | ||
| import {injectIntl} from 'react-intl'; |
There was a problem hiding this comment.
Remove unused import. The injectIntl import is not used anywhere in the component.
| import {injectIntl} from 'react-intl'; | |
| import {injectIntl} from 'react-intl'; |
| import errorBoundaryHOC from '../lib/error-boundary-hoc.jsx'; | ||
| import {Theme} from '../lib/themes'; | ||
| import {connect} from 'react-redux'; | ||
| import {updateToolbox} from '../reducers/toolbox.js'; |
There was a problem hiding this comment.
Remove unused import. The updateToolbox import is not used anywhere in the component.
src/containers/blocks.jsx
Outdated
| import log from '../lib/log.js'; | ||
| import Prompt from './prompt.jsx'; | ||
| import BlocksComponent from '../components/blocks/blocks.jsx'; | ||
| import CodeComponent from '../components/code-editor/code-editor.jsx'; |
There was a problem hiding this comment.
🛑 Component Mismatch: The import is changed to CodeComponent from code-editor.jsx but this component is not designed to replace the blocks functionality. The CodeComponent is a simple Monaco editor wrapper, while BlocksComponent was the visual block editor interface. This breaks the blocks tab functionality.
There was a problem hiding this comment.
/q yeah it's wip still dum dum and we're replacing the entire block functionality anyways
There was a problem hiding this comment.
I understand this is WIP, but the parent comment raises a valid architectural concern. The current approach imports CodeComponent from code-editor.jsx but still uses all the Scratch blocks infrastructure (VMScratchBlocks, workspace, toolbox, etc.).
This creates a disconnect between the imported component and the container logic. Consider either:
- Keeping
BlocksComponentimport until the blocks infrastructure is actually replaced - Creating a proper abstraction layer that can handle both blocks and code editor modes
The "replacing the entire block functionality" approach will be a significant undertaking - might be worth discussing the migration strategy to avoid breaking changes during development.
| // ^^^ While this code is pretty good, this doesn't entirely prevent the GUI from freezing when a really large sb3 is downloading. | ||
| // In fact, this is a massive issue with scratch-gui in general, as everything is on the main thread and something as simple as a nifty | ||
| // scratch-vm trick like that one clone script that creates thousands of clones before a single frame render, consequently crashing the GUI | ||
| // and even taking up all the RAM if you have the "Infinite Clones" option in TurboWarp/OmniBlocks. | ||
| // This means it will freeze everything permanently and lose progress. | ||
| // The solution is quite simple (in theory, easier said than done): Move a bunch of stuff to web workers. Including this script right here, which I will work on eventually. | ||
| // While I gotta cut the Scratch Team some slack for, y'know, being an underfunded non-profit, and also probably didn't expect people to make projects that were 1GB+ big | ||
| // (which is quite common now, lol), but web workers were were established by the time Scratch 3 was rushed out back in 2019. | ||
|
|
||
| // Enough ranting, though. | ||
| // TODO: Move this downloading logic to a web worker. |
There was a problem hiding this comment.
Remove excessive commentary. While the technical insights are valid, this lengthy comment block (12 lines) disrupts code readability and belongs in documentation or issue discussions rather than production code.
| // ^^^ While this code is pretty good, this doesn't entirely prevent the GUI from freezing when a really large sb3 is downloading. | |
| // In fact, this is a massive issue with scratch-gui in general, as everything is on the main thread and something as simple as a nifty | |
| // scratch-vm trick like that one clone script that creates thousands of clones before a single frame render, consequently crashing the GUI | |
| // and even taking up all the RAM if you have the "Infinite Clones" option in TurboWarp/OmniBlocks. | |
| // This means it will freeze everything permanently and lose progress. | |
| // The solution is quite simple (in theory, easier said than done): Move a bunch of stuff to web workers. Including this script right here, which I will work on eventually. | |
| // While I gotta cut the Scratch Team some slack for, y'know, being an underfunded non-profit, and also probably didn't expect people to make projects that were 1GB+ big | |
| // (which is quite common now, lol), but web workers were were established by the time Scratch 3 was rushed out back in 2019. | |
| // Enough ranting, though. | |
| // TODO: Move this downloading logic to a web worker. | |
| // TODO: Move this downloading logic to a web worker to prevent GUI freezing with large projects. |
|
@supervoidcoder Hey void! 🎉 Absolutely! I can see you've put in some serious work on this Monaco Editor integration - love the detailed docs about your journey (including the nap that saved the day 😄). I'll dive into the code and give you a thorough review. Since this is WIP, I'll focus on what you've built so far and can provide feedback on the architecture and implementation to help as you continue working on the file system integration and IDE features! 🚀 Let me get that review rolling for you! 🧠 Learnings used✅ Actions performedReview triggered.
|
coderabbitai
bot
added
needs-careful-testing
![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
♻️ Duplicate comments (7)
src/containers/sb3-downloader.jsx (1)
145-156: Condense verbose commentary to actionable TODO.This 12-line commentary block disrupts readability and includes subjective opinions better suited for documentation or issue discussions. The only actionable item is the TODO on line 156.
🔎 Proposed fix
// Projects can be very large, so we'll utilize JSZip's stream API to avoid having the // entire sb3 in memory at the same time. - - // ^^^ While this code is pretty good, this doesn't entirely prevent the GUI from freezing when a really large sb3 is downloading. - // In fact, this is a massive issue with scratch-gui in general, as everything is on the main thread and something as simple as a nifty - // scratch-vm trick like that one clone script that creates thousands of clones before a single frame render, consequently crashing the GUI - // and even taking up all the RAM if you have the "Infinite Clones" option in TurboWarp/OmniBlocks. - // This means it will freeze everything permanently and lose progress. - // The solution is quite simple (in theory, easier said than done): Move a bunch of stuff to web workers. Including this script right here, which I will work on eventually. - // While I gotta cut the Scratch Team some slack for, y'know, being an underfunded non-profit, and also probably didn't expect people to make projects that were 1GB+ big - // (which is quite common now, lol), but web workers were were established by the time Scratch 3 was rushed out back in 2019. - - // Enough ranting, though. // TODO: Move this downloading logic to a web worker. const jszipStream = this.props.saveProjectSb3Stream();src/containers/blocks.jsx (1)
13-13: Critical: Component mismatch breaks blocks editor functionality.Replacing
BlocksComponentwithCodeComponentin theDroppableBlockswrapper is architecturally problematic:
Incompatible interfaces: The Blocks container (lines 89-735) expects to inject a ScratchBlocks workspace via
this.blocksref (line 167:this.ScratchBlocks.inject(this.blocks, workspaceConfig)). CodeComponent is a simple Box wrapper that doesn't support workspace injection.Missing functionality: The visual block editor features (drag-drop, toolbox, flyout, block glowing, etc.) depend on the ScratchBlocks workspace being properly attached to the component.
Integration concerns: VM listeners, extension handling, and workspace updates (lines 353-510) all assume a ScratchBlocks workspace exists.
Recommendations for WIP:
- Consider maintaining separate code paths for Blocks vs CodeEditor during the migration
- Use conditional rendering or feature flags to toggle between editors
- Create a proper Monaco-based container (like
MonacoBlocks) rather than retrofitting the existing Blocks containerThis aligns with the past review feedback flagging the component mismatch.
Also applies to: 87-87
src/containers/code-editor.jsx (5)
4-4: Remove unused import.The
injectIntlimport is not used anywhere in the component.
13-13: Remove unused import.The
updateToolboximport is not used anywhere in the component.
145-153: Missing theme PropType.The
themeprop is used in the component (line 106) but not declared in PropTypes.
164-166: Redux state properties don't exist.The Redux state is accessing
state.scratchGui.codeContentandstate.scratchGui.editorLanguagebut these state properties don't exist in the current Redux store structure. The component will always receive empty fallback values.
175-175: Redux action type doesn't exist.The action type
'UPDATE_CODE_CONTENT'is dispatched but this action type is not defined in any reducer. The Redux store will ignore these actions and code changes won't be persisted.
🧹 Nitpick comments (4)
src/components/code-editor/code-editor.css (1)
100-103: Remove misleading comment block.The comment "/* Global styles.css */" is confusing in a file named
code-editor.css. This appears to be leftover from copy-paste or refactoring. Consider removing lines 100-103 entirely.🔎 Proposed fix
.blocks :global(.blocklyMinimalBody) { min-width: auto; min-height: auto; } - -/* Global styles.css */ - -text-editor.md (1)
1-34: Consider formalizing or relocating temporary documentation before merge.This file provides valuable development context but is explicitly temporary. Before merging (when ready), consider:
- Moving to a properly formatted design document in docs/
- Extracting key technical decisions into ADRs (Architecture Decision Records)
- Relocating development journal entries to issue discussions or PR descriptions
The technical content (file system support, theming approach, commit references) is useful for future maintainers but needs professional formatting.
Note: Since the PR is WIP and explicitly marked "DO NOT MERGE," this can be addressed later.
src/components/code-editor/code-editor.jsx (1)
7-27: Consider renaming BlocksComponent for clarity.The component is named
BlocksComponentbut resides incode-editor.jsx. This naming inconsistency may confuse future maintainers. Consider renaming toCodeEditorComponentorMonacoEditorComponentto match the file purpose.Since this is a WIP PR, this refactor can be deferred to a later commit.
src/containers/code-editor.jsx (1)
54-62: Consider debouncing for performance.
handleEditorChangefires on every keystroke, dispatching a Redux action each time. For large documents or slow devices, this could cause performance issues.Consider debouncing the Redux dispatch (e.g., 300-500ms) to reduce the frequency of state updates while maintaining responsiveness.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/components/gui/icon--code.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
package.jsonsrc/components/code-editor/code-editor.csssrc/components/code-editor/code-editor.jsxsrc/components/gui/gui.jsxsrc/containers/blocks.jsxsrc/containers/code-editor.jsxsrc/containers/sb3-downloader.jsxsrc/lib/themes/global-styles.csstext-editor.mdwebpack.config.js
🧰 Additional context used
📓 Path-based instructions (5)
src/lib/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Avoid adding internationalization features; i18n is planned for the future but not currently implemented
Files:
src/lib/themes/global-styles.css
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Use readable variable names over clever ones; prioritize legibility even if variable names are longer or unconventional
Add comments explaining complex logic, especially when code doesn't follow conventional patterns
No profanity in code comments or variable naming - keep the project family-friendly and accessible to all ages
Use camelCase for variable names
Files:
src/containers/sb3-downloader.jsxsrc/containers/code-editor.jsxsrc/components/code-editor/code-editor.jsxwebpack.config.jssrc/containers/blocks.jsxsrc/components/gui/gui.jsx
src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use async/await for promises instead of .then() chains
Files:
src/containers/sb3-downloader.jsxsrc/containers/code-editor.jsxsrc/components/code-editor/code-editor.jsxsrc/containers/blocks.jsxsrc/components/gui/gui.jsx
package.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
npm cifor dependency installation instead ofnpm installto keep package-lock.json consistent; if using npm install, usenpm install --legacy-peer-deps
Files:
package.json
src/components/**/*.{js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer interface definitions in TypeScript for defining object shapes
Files:
src/components/code-editor/code-editor.jsxsrc/components/gui/gui.jsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:48:23.435Z
Learning: OmniBlocks/scratch-gui workflow preference: Open tracking issues for CI/automation behavior changes; assign to supervoidcoder; include backlinks to the PR and the exact comment requesting the change.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:48:23.435Z
Learning: OmniBlocks/scratch-gui: For CI comment hygiene, prefer not mentioning bot/app requesters in the Mega PR Test & Analysis sticky comment; only mention humans. Track such behavior changes via a GitHub issue assigned to supervoidcoder with backlinks to the initiating PR and comment.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:50:28.065Z
Learning: OmniBlocks/scratch-gui: When cleaning PR noise, prefer minimizing CodeRabbit auto-generated comments matching headers like "This is an auto-generated reply by CodeRabbit", "Rate Limit Exceeded", and "automated status update", while preserving human-directed conversation.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T21:20:08.837Z
Learning: In the OmniBlocks/scratch-gui repository, supervoidcoder (“void”) prefers single, complete PRs over multi-part (“Part 1/2”) submissions. If a split is unavoidable, they want explicit linkage and gating before approval.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:17:25.684Z
Learning: OmniBlocks/scratch-gui has an “ESLint Autofix” workflow that can be triggered on PRs by checking the “Auto-fix ESLint issues” box in the sticky comment or commenting `/fix lint`. It lints only changed files, pushes fixes on same-repo branches, and uploads a patch artifact for fork PRs.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-28T03:54:03.336Z
Learning: OmniBlocks/scratch-gui preference: supervoidcoder wants energetic, playful tone in CI sticky comments (emojis, fun copy), while preserving single-comment strategy and not @-mentioning bots.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-09T18:19:24.693Z
Learning: OmniBlocks/scratch-gui: The Blocks component now bridges FieldCustom in componentDidMount by assigning this.ScratchBlocks.FieldCustom = window.Blockly.FieldCustom immediately after ScratchMsgs.setLocale (added Nov 9, 2025 via PR #159). This enables SPjavascriptV2’s registerInput usage at runtime.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:12:25.578Z
Learning: OmniBlocks/scratch-gui PR #576: The repository runs Jest 29 for unit/integration tests and currently lacks a Babel transform and babel.config.js; adding babel-jest and a transform in package.json plus a babel.config.js is required for importing ESM modules from src (e.g., src/addons/addons/middle-click-popup/WorkspaceQuerier.js) in tests.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-28T17:30:18.714Z
Learning: OmniBlocks/scratch-gui: Enhanced VRT workflow successfully completed after multiple build fixes involving rimraf/mkdirp PATH issues, working-directory complications, and cache configuration. Final solution uses native shell commands (rm -rf, mkdir -p) with npx webpack instead of npm run build when in subdirectories.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-20T00:38:31.706Z
Learning: When integrating PenguinMod’s JS editor blocks, the FieldCustom plugin must run before opening the extension library. If using scratch-blocks develop-builds, either import 'scratch-blocks/core/field_customInput.js' early in scratch-gui (e.g., src/playground/import-first.js) and bridge Blockly.FieldCustom → ScratchBlocks.FieldCustom in Blocks.componentDidMount, or include the plugin in scratch-blocks’ bundle entry so the develop-builds dist executes it.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-04T16:32:04.480Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder wants to maintain OmniBlocks-specific UltraBox modifications by forking ultrabox_typescript into a separate OmniBlocks/ultrabox repository with clean TypeScript source code, then using GitHub Actions to automatically build and deploy the minified beepbox_editor.min.js to scratch-gui's static/ folder on every push.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-14T00:47:37.673Z
Learning: The OmniBlocks/scratch-gui repository is taking a phased migration approach to React 19: React 16 → 17 → 18 → 19, starting with React 17 as the easiest first step.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T19:01:57.123Z
Learning: In the OmniBlocks/scratch-gui repository, team member supervoidcoder can be called "void" in reviews and conversations.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-01T12:10:19.386Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder prefers a division of labor where Amazon Q handles fast review for ethics and project vision concerns, while CodeRabbit performs deep technical review of code quality, security, architecture, and implementation details.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-13T22:43:34.163Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder prefers React over Svelte for UI development (React is the framework they know and want to use consistently).
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 519
File: src/playground/index.ejs:176-176
Timestamp: 2025-12-05T17:47:27.981Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder's Scratch username is scratchcode1_2_3.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-11T17:03:47.901Z
Learning: In OmniBlocks/scratch-gui chaos testing: supervoidcoder prefers NO error filtering in tests - all console errors should be caught and tracked properly with issues, not swept under the rug. Tests should help expose real bugs (like the UltraBox service worker 404) rather than hide them with filters.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T02:00:40.666Z
Learning: In OmniBlocks/scratch-gui, when cleanup is requested, supervoidcoder prefers deletion of CodeRabbit’s spam comments (not just minimizing) on the PR; for non-CodeRabbit bot comments, minimize unless a maintainer explicitly asks for hard deletion.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T19:01:57.123Z
Learning: The team members in OmniBlocks/coders include: supervoidcoder, Graison-P (gvyt), ampelectrecuted (amp), NotTheBaton (TheBaton), and kRxZykRxZy.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-04T03:26:35.542Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder appreciates detailed, thorough technical responses but wants the tone to remain friendly, energetic, and conversational (with emojis and personality) even when processing large amounts of file content or technical context. Avoid shifting to overly formal, stoic, or robotic language when diving deep into technical details.
📚 Learning: 2025-10-21T11:30:02.766Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-21T11:30:02.766Z
Learning: In TurboWarp/Scratch addons, the _runtime_entry.js file must use the webpack loader prefix when importing CSS: `import _css from "!css-loader!./userstyle.css";`. Without this prefix, CSS variables won't work and addon settings won't map to CSS styles.
Applied to files:
src/lib/themes/global-styles.csswebpack.config.js
📚 Learning: 2025-10-21T11:30:02.766Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-21T11:30:02.766Z
Learning: In TurboWarp/Scratch addon CSS files, settings are accessed as CSS variables using the format `--<addon-id>-<settingId>`, where addon-id is the kebab-case directory name and settingId is the camelCase setting ID from the manifest. For example, ghost-blocks addon with ghostOpacity setting becomes `--ghost-blocks-ghostOpacity`.
Applied to files:
src/lib/themes/global-styles.css
📚 Learning: 2025-11-09T18:19:24.693Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-09T18:19:24.693Z
Learning: OmniBlocks/scratch-gui: The Blocks component now bridges FieldCustom in componentDidMount by assigning this.ScratchBlocks.FieldCustom = window.Blockly.FieldCustom immediately after ScratchMsgs.setLocale (added Nov 9, 2025 via PR #159). This enables SPjavascriptV2’s registerInput usage at runtime.
Applied to files:
src/containers/code-editor.jsxsrc/components/code-editor/code-editor.jsxsrc/containers/blocks.jsxsrc/components/gui/gui.jsx
📚 Learning: 2025-12-01T23:52:49.306Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-01T23:52:49.306Z
Learning: In OmniBlocks/scratch-gui, the theme system uses a Theme class with three properties: `accent` (aqua/purple/blue/red/rainbow), `gui` (light/dark), and `blocks` (three/dark/high-contrast/custom). Use `theme.set(property, value)` to create new Theme instances, dispatch via Redux `setTheme()` action, and persistence is handled by `persistTheme()` from src/lib/themes/themePersistance.js.
Applied to files:
src/containers/code-editor.jsx
📚 Learning: 2025-10-07T00:28:59.495Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-07T00:28:59.495Z
Learning: PenguinMod's scratch-gui uses the same dynamic `require('../reducers/gui')` pattern in app-state-hoc.jsx as TurboWarp, so this pattern is correct and not the cause of circular dependency errors.
Applied to files:
src/containers/code-editor.jsx
📚 Learning: 2025-12-02T01:02:20.932Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-02T01:02:20.932Z
Learning: In OmniBlocks/scratch-gui PR #449, the welcome modal required importing WelcomeModal component into src/components/gui/gui.jsx (not just importing hasCompletedWelcome in the container). The component file needs: `import WelcomeModal from '../welcome-modal/welcome-modal.jsx';` to render the modal conditionally based on showWelcomeModal prop passed from container.
Applied to files:
src/containers/code-editor.jsxsrc/containers/blocks.jsxsrc/components/gui/gui.jsx
📚 Learning: 2025-12-26T23:25:05.409Z
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:25:05.409Z
Learning: In OmniBlocks/scratch-gui PR #576, the Jest setup on the PR branch has no babel-jest and no transform mapping, and there is no Babel config file; importing ESM from src in Jest requires adding babel-jest, a "^.+\\.[jt]sx?$" transform, and a babel.config.js.
Applied to files:
webpack.config.js
📚 Learning: 2025-12-26T23:12:25.578Z
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:12:25.578Z
Learning: OmniBlocks/scratch-gui PR #576: The repository runs Jest 29 for unit/integration tests and currently lacks a Babel transform and babel.config.js; adding babel-jest and a transform in package.json plus a babel.config.js is required for importing ESM modules from src (e.g., src/addons/addons/middle-click-popup/WorkspaceQuerier.js) in tests.
Applied to files:
webpack.config.jssrc/components/gui/gui.jsx
📚 Learning: 2025-12-26T23:07:46.713Z
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:07:46.713Z
Learning: The repository’s unit tests are run with Jest 29 (scripts test:unit/test:integration) and require a Babel transform to import ESM from src; add babel-jest + transform in package.json and a babel.config.js to ensure tests pass when importing files like src/addons/addons/middle-click-popup/WorkspaceQuerier.js.
Applied to files:
webpack.config.js
📚 Learning: 2025-12-26T23:13:13.375Z
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:13:13.375Z
Learning: In OmniBlocks/scratch-gui, unit/integration tests run with Jest 29. To import ESM from src (e.g., src/addons/addons/middle-click-popup/WorkspaceQuerier.js with export default), package.json must include babel-jest in devDependencies and a Jest transform mapping ("^.+\\.(js|jsx)$": "babel-jest"). The repo already has a .babelrc that babel-jest can use.
Applied to files:
webpack.config.jssrc/containers/blocks.jsx
📚 Learning: 2025-12-19T03:35:35.943Z
Learnt from: CR
Repo: OmniBlocks/scratch-gui PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T03:35:35.943Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use camelCase for variable names
Applied to files:
webpack.config.js
📚 Learning: 2025-10-20T00:38:31.706Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-20T00:38:31.706Z
Learning: When integrating PenguinMod’s JS editor blocks, the FieldCustom plugin must run before opening the extension library. If using scratch-blocks develop-builds, either import 'scratch-blocks/core/field_customInput.js' early in scratch-gui (e.g., src/playground/import-first.js) and bridge Blockly.FieldCustom → ScratchBlocks.FieldCustom in Blocks.componentDidMount, or include the plugin in scratch-blocks’ bundle entry so the develop-builds dist executes it.
Applied to files:
src/containers/blocks.jsx
📚 Learning: 2025-11-28T17:30:18.714Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-28T17:30:18.714Z
Learning: OmniBlocks/scratch-gui: Enhanced VRT workflow successfully completed after multiple build fixes involving rimraf/mkdirp PATH issues, working-directory complications, and cache configuration. Final solution uses native shell commands (rm -rf, mkdir -p) with npx webpack instead of npm run build when in subdirectories.
Applied to files:
text-editor.md
📚 Learning: 2025-10-20T01:34:10.747Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-20T01:34:10.747Z
Learning: Importing Closure source files from scratch-gui (e.g., 'scratch-blocks/core/field_customInput.js') causes 'goog is not defined'. FieldCustom must be compiled into scratch-blocks’ develop-builds bundle and then bridged in GUI; GUI should not import Closure sources directly.
Applied to files:
src/components/gui/gui.jsx
📚 Learning: 2025-11-26T23:10:09.344Z
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-26T23:10:09.344Z
Learning: OmniBlocks/scratch-gui VRT: To avoid mis-clicking the Songs tab, select editor tabs by index using [role="tablist"] → [role="tab"]. Indices: Code=0, Costumes=1, Sounds=2, Songs=3. Apply in Playwright tests across CI.
Applied to files:
src/components/gui/gui.jsx
🧬 Code graph analysis (1)
src/containers/code-editor.jsx (1)
src/reducers/modals.js (2)
openConnectionModal(93-95)closeExtensionLibrary(126-128)
🪛 LanguageTool
text-editor.md
[grammar] ~18-~18: Ensure spelling is correct
Context: ...lly works! I can type in it perfectly!! YESS!! But it looks jarring in light mode, but ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~25-~25: Use a hyphen to join words.
Context: ...f it's a rabbit ifykyk) I then took a 2 hour nap, woke up, and copied the logic ...
(QB_NEW_EN_HYPHEN)
[style] ~25-~25: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...logic from gui.jsx, and it was actually quite simple. The AI kept making up nonexisted prop ...
(EN_WEAK_ADJECTIVE)
[grammar] ~25-~25: Ensure spelling is correct
Context: ...s just trash all the way. After my nap, where my headache was gone, I looked at gui.j...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (3)
- GitHub Check: build
- GitHub Check: mega-test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
src/lib/themes/global-styles.css (1)
45-45: LGTM!Whitespace-only formatting change with no functional impact.
src/components/gui/gui.jsx (2)
12-12: LGTM - Component import updated for Monaco integration.The import change from Blocks to CodeEditor aligns with the PR's goal of integrating Monaco Editor. The component interface appears compatible based on the props passed in lines 403-415.
Since this is a fundamental change to the editor component, ensure the CodeEditor container properly handles all the props and maintains feature parity with the Blocks component, particularly:
- Extension library integration (onOpenCustomExtensionModal)
- Theme switching (theme prop)
- Visibility handling (isVisible)
- Cloud variable support (canUseCloud)
403-415: Verify CodeEditor maintains feature parity with Blocks.The component replacement preserves all props, which is good. However, given the fundamental architectural difference between a block-based editor and Monaco (text editor), verify:
- How does CodeEditor render blocks visually?
- Are block drag-drop, toolbox, and flyout features supported?
- Does the extension button (lines 417-429) still work correctly?
Based on the linked past review comment on blocks.jsx, there may be a component mismatch where CodeComponent (a simple Monaco wrapper) doesn't provide blocks functionality.
src/components/code-editor/code-editor.jsx (1)
5-5: LGTM - CSS import path updated appropriately.Changing the import from
blocks.csstocode-editor.cssaligns with the component's new location and purpose.package.json (1)
38-39: Monaco Editor versions are compatible and secure.The specified versions (@monaco-editor/react@4.4.6 and @monaco-editor/loader@1.3.2) have no known security vulnerabilities, are compatible with React 16 (required peer dependency is ^16.8.0 || ^17.0.0 || ^18.0.0), and are stable, non-deprecated versions suitable for production. Newer versions (4.7.0 and 1.7.0) are available if an upgrade is desired.
webpack.config.js (3)
65-65: LGTM! Correct .mjs extension support.Adding
.mjsto the extensions array enables Webpack to resolve ES module imports from Monaco packages.
73-77: LGTM! Proper .mjs handling for Webpack 4.The
type: 'javascript/auto'configuration correctly tells Webpack to treat.mjsfiles as ES modules rather than CommonJS, which is essential for Monaco packages.
78-97: LGTM! Monaco packages properly included in Babel transpilation.The expanded include paths ensure that
@monaco-editor/reactand@monaco-editor/loaderare transpiled for compatibility with the React 16/Webpack 4 environment, aligning with the downgrade strategy mentioned in the PR description.src/containers/code-editor.jsx (4)
19-36: LGTM! Clean constructor and state initialization.The constructor properly binds handlers and initializes state with instance variables for editor and Monaco references.
38-52: LGTM! Standard Monaco initialization pattern.The
setTimeoutforeditor.layout()is a common pattern to ensure the DOM is fully rendered before calculating layout.
103-122: LGTM! Well-configured Monaco Editor options.The editor configuration includes useful IDE features like minimap, line numbers, bracket colorization, and automatic layout, providing a solid foundation for the code editing experience.
178-183: LGTM! Proper HOC composition.The component is correctly wrapped with an error boundary and connected to Redux, following standard React/Redux patterns.
| handlePromptCallback (input) { | ||
| this.state.prompt.callback(input); | ||
| this.handlePromptClose(); | ||
| } |
There was a problem hiding this comment.
Add null safety check.
Direct access to this.state.prompt.callback without verifying that prompt still exists could throw if the prompt is cleared between the render and the callback execution.
🔎 Proposed fix
handlePromptCallback (input) {
- this.state.prompt.callback(input);
+ if (this.state.prompt && this.state.prompt.callback) {
+ this.state.prompt.callback(input);
+ }
this.handlePromptClose();
}📝 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.
| handlePromptCallback (input) { | |
| this.state.prompt.callback(input); | |
| this.handlePromptClose(); | |
| } | |
| handlePromptCallback (input) { | |
| if (this.state.prompt && this.state.prompt.callback) { | |
| this.state.prompt.callback(input); | |
| } | |
| this.handlePromptClose(); | |
| } |
🤖 Prompt for AI Agents
In src/containers/code-editor.jsx around lines 70 to 73, the handler calls
this.state.prompt.callback directly which can throw if prompt was cleared; add a
null-safety guard by reading const prompt = this.state.prompt (or this.getState
snapshot), check prompt && typeof prompt.callback === 'function' before invoking
prompt.callback(input), then call this.handlePromptClose(); this ensures you
only call the callback when prompt still exists and is callable.
| <Editor | ||
| height="100%" | ||
| language={language || 'javascript'} | ||
| theme={this.props.theme.isDark() ? 'vs-dark' : 'vs'} |
There was a problem hiding this comment.
Critical: theme prop not passed from Redux.
Line 106 accesses this.props.theme.isDark() but theme is not mapped in mapStateToProps (lines 160-166). This will cause a runtime error: "Cannot read property 'isDark' of undefined".
🔎 Proposed fix
Add theme to mapStateToProps:
const mapStateToProps = state => ({
anyModalVisible: Object.keys(state.scratchGui.modals).some(key => state.scratchGui.modals[key]),
extensionLibraryVisible: state.scratchGui.modals.extensionLibrary,
code: state.scratchGui.codeContent || '',
- language: state.scratchGui.editorLanguage || 'javascript'
+ language: state.scratchGui.editorLanguage || 'javascript',
+ theme: state.scratchGui.theme
});And add to PropTypes:
MonacoBlocks.propTypes = {
code: PropTypes.string,
extensionLibraryVisible: PropTypes.bool,
isVisible: PropTypes.bool,
language: PropTypes.string,
onCodeChange: PropTypes.func,
onOpenConnectionModal: PropTypes.func,
- onRequestCloseExtensionLibrary: PropTypes.func
+ onRequestCloseExtensionLibrary: PropTypes.func,
+ theme: PropTypes.instanceOf(Theme).isRequired
};Based on learnings, the theme system uses a Theme class from src/lib/themes, and the state path is state.scratchGui.theme.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/containers/code-editor.jsx around line 106 (and mapStateToProps at lines
160-166), the component calls this.props.theme.isDark() but theme is not mapped
from Redux, causing a runtime "Cannot read property 'isDark' of undefined";
update mapStateToProps to return theme: state.scratchGui.theme (the Theme class
from src/lib/themes) so this.props.theme is defined, and add theme:
PropTypes.instanceOf(require('../lib/themes').Theme).isRequired (or equivalent
import) to the component PropTypes to document the prop requirement.
There was a problem hiding this comment.
Nope! You gave me the same review comment when I reviewed locally! This in fact works just fine as it is, as I've tested it locally and the theme switching works like a charm!
In fact, when I added it to the props, it broke it!!!!!
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how theme is passed to components in this codebase
# First, check gui.jsx to see what props are passed to CodeEditor
cat src/components/gui/gui.jsx | grep -A 20 -B 5 "CodeEditor"
Length of output: 3051
🏁 Script executed:
#!/bin/bash
# Look for other components that use theme.isDark() to understand the pattern
rg -n "theme\.isDark\(\)" --type=jsx --type=js -C 3
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check the errorBoundaryHOC to see if it injects theme
cat src/lib/error-boundary-hoc.jsx
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Look for theme context or provider
rg -n "ThemeProvider|ThemeContext" --type=jsx --type=js -C 3
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check how the original Blocks component accessed theme
cat src/components/blocks/blocks.jsx | head -100
Length of output: 64
@supervoidcoder You're absolutely right, and I apologize for the incorrect review! 🙏
I can now see that gui.jsx explicitly passes the theme prop to CodeEditor:
<CodeEditor
theme={theme}
// ... other props
/>The theme flows from the parent GUI component down to your MonacoBlocks component as a regular prop - it's not supposed to come from Redux mapStateToProps. Adding it to the Redux mapping would actually conflict with the parent-passed prop, which is exactly why it broke when you tried!
This is a perfectly valid React pattern where the parent component controls and passes down the theme. My static analysis missed the parent component's prop-passing, leading to the false positive. Thank you for testing locally and catching this! 🎯
✏️ Learnings added
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 589
File: src/containers/code-editor.jsx:106-106
Timestamp: 2025-12-29T02:03:07.803Z
Learning: In OmniBlocks/scratch-gui src/containers/code-editor.jsx (MonacoBlocks component), the theme prop is passed from the parent gui.jsx component, NOT from Redux mapStateToProps. Adding theme to mapStateToProps causes conflicts with the parent-passed prop and breaks theme functionality.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-09T18:19:24.693Z
Learning: OmniBlocks/scratch-gui: The Blocks component now bridges FieldCustom in componentDidMount by assigning this.ScratchBlocks.FieldCustom = window.Blockly.FieldCustom immediately after ScratchMsgs.setLocale (added Nov 9, 2025 via PR #159). This enables SPjavascriptV2’s registerInput usage at runtime.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T19:01:57.123Z
Learning: In the OmniBlocks/scratch-gui repository, team member supervoidcoder can be called "void" in reviews and conversations.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-01T12:10:19.386Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder prefers a division of labor where Amazon Q handles fast review for ethics and project vision concerns, while CodeRabbit performs deep technical review of code quality, security, architecture, and implementation details.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-28T03:54:03.336Z
Learning: OmniBlocks/scratch-gui preference: supervoidcoder wants energetic, playful tone in CI sticky comments (emojis, fun copy), while preserving single-comment strategy and not @-mentioning bots.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-13T22:43:34.163Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder prefers React over Svelte for UI development (React is the framework they know and want to use consistently).
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 519
File: src/playground/index.ejs:176-176
Timestamp: 2025-12-05T17:47:27.981Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder's Scratch username is scratchcode1_2_3.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T21:20:08.837Z
Learning: In the OmniBlocks/scratch-gui repository, supervoidcoder (“void”) prefers single, complete PRs over multi-part (“Part 1/2”) submissions. If a split is unavoidable, they want explicit linkage and gating before approval.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-11T17:03:47.901Z
Learning: In OmniBlocks/scratch-gui chaos testing: supervoidcoder prefers NO error filtering in tests - all console errors should be caught and tracked properly with issues, not swept under the rug. Tests should help expose real bugs (like the UltraBox service worker 404) rather than hide them with filters.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T02:00:40.666Z
Learning: In OmniBlocks/scratch-gui, when cleanup is requested, supervoidcoder prefers deletion of CodeRabbit’s spam comments (not just minimizing) on the PR; for non-CodeRabbit bot comments, minimize unless a maintainer explicitly asks for hard deletion.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-10-22T19:01:57.123Z
Learning: The team members in OmniBlocks/coders include: supervoidcoder, Graison-P (gvyt), ampelectrecuted (amp), NotTheBaton (TheBaton), and kRxZykRxZy.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-04T03:26:35.542Z
Learning: In OmniBlocks/scratch-gui, supervoidcoder appreciates detailed, thorough technical responses but wants the tone to remain friendly, energetic, and conversational (with emojis and personality) even when processing large amounts of file content or technical context. Avoid shifting to overly formal, stoic, or robotic language when diving deep into technical details.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:48:23.435Z
Learning: OmniBlocks/scratch-gui workflow preference: Open tracking issues for CI/automation behavior changes; assign to supervoidcoder; include backlinks to the PR and the exact comment requesting the change.
Learnt from: supervoidcoder
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:48:23.435Z
Learning: OmniBlocks/scratch-gui: For CI comment hygiene, prefer not mentioning bot/app requesters in the Mega PR Test & Analysis sticky comment; only mention humans. Track such behavior changes via a GitHub issue assigned to supervoidcoder with backlinks to the initiating PR and comment.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-11-05T01:50:28.065Z
Learning: OmniBlocks/scratch-gui: When cleaning PR noise, prefer minimizing CodeRabbit auto-generated comments matching headers like "This is an auto-generated reply by CodeRabbit", "Rate Limit Exceeded", and "automated status update", while preserving human-directed conversation.
Learnt from: github-actions[bot]
Repo: OmniBlocks/scratch-gui PR: 0
File: :0-0
Timestamp: 2025-12-26T23:17:25.684Z
Learning: OmniBlocks/scratch-gui has an “ESLint Autofix” workflow that can be triggered on PRs by checking the “Auto-fix ESLint issues” box in the sticky comment or commenting `/fix lint`. It lints only changed files, pushes fixes on same-repo branches, and uploads a patch artifact for fork PRs.
i have an idea: should work then |
|
@coderabbitai do you think that will work |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11151 🏁 Script executed: Length of output: 227 @8to16 Hey! Great idea, but actually - it's already there! 😄 Looking at the webpack config (line 60), you've got comprehensive .mjs support already in place: resolve: {
symlinks: false,
extensions: ['.mjs', '.js', '.jsx', '.json'], // ✅ .mjs is first!
alias: { ... }
}Plus there's a dedicated .mjs rule (lines 67-71): {
test: /\.mjs$/,
include: /node_modules/,
type: 'javascript/auto' // This tells webpack to handle it as ES module
}And the Monaco packages are already in the babel-loader include list (lines 77-78): include: [
// ... other paths
/node_modules[\\/]@monaco-editor[\\/]react/,
/node_modules[\\/]@monaco-editor[\\/]loader/
]So the webpack config is fully equipped to handle .mjs files from Monaco! 🎯 This means: You might actually be able to upgrade back to Want to give the upgrade a shot? If it was a fresh error from a clean install, the existing config should handle it! 🚀 🧠 Learnings used |
I already fixed it |
|
@8to16 are you still there sorry for being an hour late |
hi |
|
@8to16 yay |
|
make the live thing |
yea |
🕐 Stale PR ReminderHey @supervoidcoder! 🎉 Just swinging by to check on your "Code Editor [WIP]" PR #589 — it’s looking super exciting! 🎨 It’s been 7 days since your last update, and I’m curious if there’s anything you’re needing help with or if something might be blocking your progress? 🛠️ Remember, we’re all here to cheer you on, so let us know how we can support you! Keep up the great work! 🌟 This is an automated friendly reminder. No pressure! 💙 |
🤖 Auto-Component Test Results
|
| Component | Screenshot (with highlighting) |
|---|---|
clickable-tabsrc/components/gui/gui.jsx:358 |
 Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:380 |
 Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:394 |
 Screenshot not available |
💡 Note: If screenshots appear broken, they may still be processing. Check the screenshots branch for the latest images.
🧪 Test Status: skipped
Auto-generated tests have been created and executed!
📦 Check the workflow artifacts for:
- Generated test code
- Full test results
- All screenshots
Triggered by @8to16 • Commit a4e0927
💡 Tip: These are basic smoke tests. Consider expanding them for better coverage!
|
❌ MEGA PR Test Failed @supervoidcoder Something went wrong during the build or analysis process. Check the logs for details: View Logs |
❌ ULTRA Visual Regression Testing - Build FailedHi @supervoidcoder! The build phase failed, so visual regression testing could not run. Build errors occurred - unable to create build artifacts for base and/or PR. Please check the logs for details: Common issues: webpack errors, dependency problems, or syntax errors in the code. |
|
@supervoidcoder can we do livessharre pretty please |
i can't cuz im in school |
can we do it on the weekend |
🤖 Auto-Component Test Results
|
| Component | Screenshot (with highlighting) |
|---|---|
clickable-tabsrc/components/gui/gui.jsx:358 |
Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:380 |
Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:394 |
Screenshot not available |
💡 Note: If screenshots appear broken, they may still be processing. Check the screenshots branch for the latest images.
🧪 Test Status: skipped
Auto-generated tests have been created and executed!
📦 Check the workflow artifacts for:
- Generated test code
- Full test results
- All screenshots
Triggered by @8to16 • Commit 9a614cc
💡 Tip: These are basic smoke tests. Consider expanding them for better coverage!
|
❌ MEGA PR Test Failed @supervoidcoder Something went wrong during the build or analysis process. Check the logs for details: View Logs |
❌ ULTRA Visual Regression Testing - Build FailedHi @supervoidcoder! The build phase failed, so visual regression testing could not run. Build errors occurred - unable to create build artifacts for base and/or PR. Please check the logs for details: Common issues: webpack errors, dependency problems, or syntax errors in the code. |
|
@coderabbitai skibidi-webpack-plugin |
|
Tip For best results, initiate chat on the files or code changes.
Current approach: // Using `@monaco-editor/react` (React wrapper)
import Editor from '@monaco-editor/react';
<Editor theme="dark" language="javascript" ... />monaco-editor-webpack-plugin approach: // Using monaco-editor directly
import * as monaco from 'monaco-editor';
monaco.editor.create(element, { theme: 'dark', language: 'javascript' });Trade-offs: Benefits of switching to the plugin:
Drawbacks:
Current setup works! The webpack config already handles everything needed for Since this is WIP and void's already made good progress with the React wrapper, switching now would be a decent-sized refactor. But if you want maximum bundle size optimization and more control, the webpack plugin route is totally valid! What do you think 🧠 Learnings used💡 Use |
|
Tip For best results, initiate chat on the files or code changes.
But actually, real talk — your earlier suggestion about Why it might be worth adding:
Right now the PR uses Worth considering as you continue building out the IDE features! 🚀 (But if you actually make a skibidi-webpack-plugin package, I'll be genuinely impressed 😄) 🧠 Learnings used💡 Use |
|
@coderabbitai well the whole thing is broken anyways, since during that live share a bunch of stuff changed and now everything's a mess but idk |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
Let me check what's actually in the files right now so we can figure out what needs fixing! 🔍 🏁 Script executed: Length of output: 11122
Good news: The main Monaco container ( Bad news: Here's what the component file should look like (it's just a simple wrapper): import PropTypes from 'prop-types';
import classNames from 'classnames';
import React from 'react';
import Box from '../box/box.jsx';
import styles from './code-editor.css';
const BlocksComponent = props => {
const {
containerRef,
dragOver,
...componentProps
} = props;
return (
<Box
className={classNames(styles.blocks, {
[styles.dragOver]: dragOver
})}
{...componentProps}
componentRef={containerRef}
/>
);
};
BlocksComponent.propTypes = {
containerRef: PropTypes.func,
dragOver: PropTypes.bool
};
export default BlocksComponent;Just replace the corrupted file with this and you should be back in business! The container and everything else looks great. 🚀 Want me to check anything else or help debug if there are other issues? 🧠 Learnings used💡 Use |
|
git reset |
🤖 Auto-Component Test Results
|
| Component | Screenshot (with highlighting) |
|---|---|
clickable-tabsrc/components/gui/gui.jsx:358 |
Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:380 |
Screenshot not available |
clickable-tabsrc/components/gui/gui.jsx:394 |
Screenshot not available |
💡 Note: If screenshots appear broken, they may still be processing. Check the screenshots branch for the latest images.
🧪 Test Status: skipped
Auto-generated tests have been created and executed!
📦 Check the workflow artifacts for:
- Generated test code
- Full test results
- All screenshots
Triggered by @8to16 • Commit 41b38c9
💡 Tip: These are basic smoke tests. Consider expanding them for better coverage!
|
❌ MEGA PR Test Failed @supervoidcoder Something went wrong during the build or analysis process. Check the logs for details: View Logs |
❌ ULTRA Visual Regression Testing - Build FailedHi @supervoidcoder! The build phase failed, so visual regression testing could not run. Build errors occurred - unable to create build artifacts for base and/or PR. Please check the logs for details: Common issues: webpack errors, dependency problems, or syntax errors in the code. |
🕐 Stale PR ReminderHey @supervoidcoder! 👋 This PR has been quiet for a while. Need any help getting it across the finish line? Feel free to ask! 🚀 This is an automated friendly reminder. No pressure! 💙 |
|
@coderabbitai @supervoidcoder do we even need this anymore considering https://github.com/OmniBlocks/ide-shell/ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3502
What I found:ide-shell (created ~1 week ago, Jan 21):
PR
The architectural choice:Option A: Continue PR
Option B: Go all-in on ide-shell (scratch-gui inside IDE shell)
My take:ide-shell aligns better with the long-term IDE vision from the learnings, but it's very new and still WIP. PR @supervoidcoder - this is your call! Should we:
What's your strategic vision here? 🚀 🧠 Learnings used💡 Use |
🕐 Stale PR ReminderHey @supervoidcoder! 👋 This PR has been quiet for a while. Need any help getting it across the finish line? Feel free to ask! 🚀 This is an automated friendly reminder. No pressure! 💙 |
2 participants
This is a work-in-progress branch for the text editor.
resolves #588
#588
I have gotten some pretty good work done today, but I will stop working and start working with @8to16 (formerly ampelectrecuted) on this tomorrow Monday.
Things done:
Things yet to do:
For reference, I will include my text editor docs in this PR comment:
This is a temporary documentation file so I can yap about my process in making the code editor without making a super ultra formal docs file fromt he beggingniing..
I want the editor to work with 3 different file systems:
Commit 6dc626d
Updates...
I have successfully changed the entire blocks editor into a new directory with entirely different names. Now it's time to start dismantling it into monaco-react.
Well, it turns out webpack is freaking out at an .mjs file, since this codebase is ancient and uses React 16 and Webpack 4.
Welp, all I can do is downgrade @monaco-editor/react from ^4.7.0 to a lower version that doesn't use .mjs yet.
Yes! It finally works! I can type in it perfectly!! YESS!!
But it looks jarring in light mode, but perfect in dark mode.
Trying to make it dynamic to the scratch-gui theme sent me down a rabbit hole of pain.
I spent an hour trying to lazily vibe code the dynamic theme switching with AI, which only let do a crap ton of hallucinations and me crashing out a few times, further proving my point that AI absolutely sucks for coding (but it's great for reviewing 😉 especially if it's a rabbit ifykyk)
I then took a 2 hour nap, woke up, and copied the logic from gui.jsx, and it was actually quite simple. The AI kept making up nonexisted prop names, calling
Themeas a class, and even using a randomisDark()function that, well, didn't work. It tried a bunch ofComponentDidUpdateshenanigans as well, and well yeah it was just trash all the way. After my nap, where my headache was gone, I looked at gui.jsx for two seconds before finding that you simply imported{Theme}from'src/lib/themes', and then in the Editor component for@monaco-editor/react, you simply pass the theme as a prop using a ternary operator, which logic i copied frompaint-editor.wrapper.jsx, giving me this:I was overjoyed that this instantly worked exactly how I want it. Now I just need to scrap the entire scratch vm and replace it with something more IDE-y, but I guess that can wait. This is a great milestone, I will commit now (Commit 6e329bc). I'll work on the file explorer, like the one that shows up in the left to VS Code with the folders and whatnot.
Honestly, I feel much better now that I took that nap. The reason I used AI in the first place was because I had a strong headache, and thinking hurt more, so I couldn't look and read the code properly or try to understand it.