docs: apply vs-code-style design to playground docs #263
Conversation
WalkthroughReplaces CodeMirror with Monaco in package.json and integrates @monaco-editor/react across the playground. Adds modular playground components: EditorTabs, FileExplorer, InstallOverlay, RunModal, SwaggerPreview, TopBar, WorkspacePlaceholder. Introduces typed playground APIs (types.ts) and canned files/constants (constants.ts). Refactors playground/index.tsx to a multi-file tabbed editor flow, updates createProjectFiles to accept packageJson, and wires new components for run/preview/install flows. Stylesheet reworked to a darker theme with new explorer/tab/modal rules. pages/playground extracts PlaygroundHelpModal. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
itdoc-doc/src/components/Playground/index.tsx (1)
203-214: Keep the package.json tab aligned with fallback updates
switchItDocDependencyToRegistryrewritespackage.jsoninside the WebContainer, but the React state driving the editor still comes from the initialpackageJsonRef. After the fallback runs, thepackage.jsontab keeps showing the stale tarball dependency (and will overwrite the container change as soon as the user edits or re-runs). Please store the package.json contents in component state (e.g.,useState+ ref for initial mount), update that state when the fallback succeeds, and include it inactiveFileValue/handleEditorChangeso the editor always mirrors the real file. Returning the updated JSON string from this helper andsetPackageJson(...)in the caller is one straightforward way to fix this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
itdoc-doc/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
itdoc-doc/package.json(1 hunks)itdoc-doc/src/components/Playground/EditorTabs.tsx(1 hunks)itdoc-doc/src/components/Playground/FileExplorer.tsx(1 hunks)itdoc-doc/src/components/Playground/InstallOverlay.tsx(1 hunks)itdoc-doc/src/components/Playground/RunModal.tsx(1 hunks)itdoc-doc/src/components/Playground/SwaggerPreview.tsx(1 hunks)itdoc-doc/src/components/Playground/TopBar.tsx(1 hunks)itdoc-doc/src/components/Playground/WorkspacePlaceholder.tsx(1 hunks)itdoc-doc/src/components/Playground/constants.ts(1 hunks)itdoc-doc/src/components/Playground/index.tsx(8 hunks)itdoc-doc/src/components/Playground/styles.module.css(11 hunks)itdoc-doc/src/components/Playground/types.ts(1 hunks)itdoc-doc/src/pages/playground.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
itdoc-doc/src/components/Playground/EditorTabs.tsx (1)
itdoc-doc/src/components/Playground/types.ts (2)
PlaygroundFileMap(45-45)PlaygroundFileId(19-19)
itdoc-doc/src/components/Playground/constants.ts (1)
itdoc-doc/src/components/Playground/types.ts (4)
PlaygroundFileMap(45-45)ExplorerNode(43-43)PlaygroundFileId(19-19)PlaygroundFileDefinition(21-29)
itdoc-doc/src/components/Playground/TopBar.tsx (1)
itdoc-doc/src/components/Playground/types.ts (2)
InstallStatus(17-17)PlaygroundFileDefinition(21-29)
itdoc-doc/src/components/Playground/FileExplorer.tsx (1)
itdoc-doc/src/components/Playground/types.ts (3)
ExplorerNode(43-43)PlaygroundFileMap(45-45)PlaygroundFileId(19-19)
itdoc-doc/src/components/Playground/InstallOverlay.tsx (1)
itdoc-doc/src/components/Playground/types.ts (1)
InstallStatus(17-17)
itdoc-doc/src/components/Playground/index.tsx (2)
itdoc-doc/src/components/Playground/types.ts (1)
PlaygroundFileId(19-19)itdoc-doc/src/components/Playground/constants.ts (6)
initialExpressCode(27-58)initialTestCode(60-115)PLAYGROUND_FILES(184-184)ITDOC_TARBALL_ASSET(24-24)EXPLORER_NODES(186-194)installMilestones(117-137)
🔇 Additional comments (5)
itdoc-doc/package.json (1)
27-34: Monaco dependency alignment confirmed.Bringing in
@monaco-editor/reactalongside the matchingmonaco-editorversion keeps the new Playground stack consistent with the Monaco-based components you introduced. Nicely scoped update.itdoc-doc/src/components/Playground/WorkspacePlaceholder.tsx (1)
17-58: Graceful placeholder UX.The conditional tip card plus optional “Show another idea” hook gives a polished loading state without overcomplicating the component. Nicely composed.
itdoc-doc/src/components/Playground/SwaggerPreview.tsx (1)
17-65: Accessible modal wiring looks solid.Good call stopping backdrop propagation and wiring
aria-labelledby/aria-modal; the empty-state path also reads cleanly.itdoc-doc/src/components/Playground/FileExplorer.tsx (1)
30-104: Explorer state handling reads well.The split between folder and file nodes, plus the
aria-currenthints, makes the tree both legible and keyboard-friendly. Nicely structured.itdoc-doc/src/pages/playground.tsx (1)
25-124: Modal overflow cleanup handled.Capturing the original body overflow before hiding scroll and restoring it on teardown keeps the page state tidy while the help dialog is open. Clean refactor.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
itdoc-doc/src/components/Playground/constants.ts (1)
148-178: Consider adding JSDoc comments for public exports.The file definitions are correctly structured with appropriate metadata. However, adding JSDoc comments to
PLAYGROUND_FILES(and other exported constants) would improve developer experience by providing inline documentation.Example:
+/** + * Map of playground file definitions including metadata for Monaco Editor. + * @type {PlaygroundFileMap} + */ export const PLAYGROUND_FILES: PlaygroundFileMap = fileDefinitionsitdoc-doc/src/components/Playground/index.tsx (1)
256-266: Simplify the switch statement structure.The
"package"case falls through to thedefaultcase. For clarity, either remove the empty"package"case or add a comment explaining the fall-through intent.Apply this diff to remove the redundant case:
const activeFileValue = useMemo(() => { switch (activeFileId) { case "app": return expressCode case "test": return testCode - case "package": default: return packageJsonRef.current } }, [activeFileId, expressCode, testCode])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
itdoc-doc/src/components/Playground/constants.ts(1 hunks)itdoc-doc/src/components/Playground/index.tsx(8 hunks)
🔇 Additional comments (16)
itdoc-doc/src/components/Playground/constants.ts (6)
1-16: LGTM!Standard Apache 2.0 license header is properly formatted.
17-19: LGTM!Type imports and tarball asset path are correctly defined.
21-52: LGTM!The Express boilerplate is well-structured with appropriate routes and validation. The hardcoded user ID
"user_123"(line 43) is acceptable for example/boilerplate code.
54-109: LGTM!The itdoc test code correctly tests both Express routes with appropriate request/response validation. The test structure aligns well with the
initialExpressCode.
180-188: LGTM!The explorer structure correctly references the file definitions and maintains a logical hierarchy. The
fileIdvalues properly match the keys inPLAYGROUND_FILES.
133-146: Verify emoji shortcode rendering.Line 140 uses the emoji shortcode
:grin:which may not render correctly depending on the UI framework being used. Ensure that the consuming component properly converts emoji shortcodes to actual emoji characters or Unicode.itdoc-doc/src/components/Playground/index.tsx (10)
25-42: LGTM! Clean modularization.The refactor extracts components and constants into separate modules, improving maintainability and separation of concerns. The import structure is well-organized.
216-221: LGTM! Proper state structure for multi-file editing.The new state variables and refs correctly support the multi-file editing feature with appropriate TypeScript types.
230-230: LGTM! More descriptive ref name.Renaming to
didAutofocusEditorRefbetter conveys that this tracks whether autofocus has occurred.
273-295: LGTM! Robust tab closing logic.The
handleCloseTabimplementation correctly handles edge cases:
- Prevents closing the last remaining tab
- Manages active file fallback when closing the current tab
- Safe guards against closing files not in the open list
314-319: LGTM! Proper focus management.Using a ref to track whether the editor has been focused prevents unwanted refocusing on component remounts.
365-365: LGTM! Centralized constant.Moving the tarball URL to an imported constant improves maintainability and reduces duplication.
466-470: LGTM! Correct signature update.The
createProjectFilescall correctly passes the newpackageJsonparameter.
786-864: LGTM! Clean modular component composition.The refactor successfully decomposes the monolithic UI into focused, reusable components. The prop passing appears comprehensive, with proper state, handlers, and refs wired through.
Since the component definitions aren't provided in this review, ensure that all props match the expected interfaces of the new components (TopBar, FileExplorer, EditorTabs, RunModal, SwaggerPreview, InstallOverlay, WorkspacePlaceholder).
104-109: Verify that all callers have been updated.The function signature now requires a
packageJsonparameter. Ensure that all invocations ofcreateProjectFilesthroughout the codebase have been updated to match this new signature.
297-312: Verify package.json editability intent.The
handleEditorChangefunction only updates state for"app"and"test"files, even though it checks if the file is editable. Ifpackage.jsonis intended to be editable in the future, this handler would need to be extended to persist those changes topackageJsonRef.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
itdoc-doc/src/components/Playground/styles.module.css (1)
534-534: Consider reducing z-index values to a reasonable range.The z-index values are excessively high (9999999999 and 999999999999), which suggests poor z-index management. Standard practice is to define a z-index scale (e.g., 1-100 or 1-1000) to maintain predictable stacking contexts.
Consider defining CSS custom properties for z-index layers:
:root { --z-modal: 1000; --z-modal-overlay: 1010; --z-preview: 1020; } .runModalBackdrop { z-index: var(--z-modal); } .previewModalBackdrop { z-index: var(--z-preview); }Also applies to: 1134-1134
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
itdoc-doc/src/components/Playground/SwaggerPreview.tsx(1 hunks)itdoc-doc/src/components/Playground/index.tsx(9 hunks)itdoc-doc/src/components/Playground/styles.module.css(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
itdoc-doc/src/components/Playground/index.tsx (2)
itdoc-doc/src/components/Playground/types.ts (1)
PlaygroundFileId(19-19)itdoc-doc/src/components/Playground/constants.ts (6)
initialExpressCode(21-52)initialTestCode(54-109)PLAYGROUND_FILES(178-178)ITDOC_TARBALL_ASSET(19-19)EXPLORER_NODES(180-188)installMilestones(111-131)
🔇 Additional comments (7)
itdoc-doc/src/components/Playground/SwaggerPreview.tsx (1)
62-62: Verify necessity of permissive iframe sandbox attributes.The sandbox allows both
allow-scriptsandallow-same-origin, which significantly reduces isolation. While this may be required for Redoc to function, it exposes the iframe to XSS risks if the content is not fully trusted.Verify whether Redoc requires both attributes or if you can remove
allow-same-originfor better security isolation. Check the Redoc documentation for minimum sandbox requirements.itdoc-doc/src/components/Playground/index.tsx (6)
253-253: Consider future updates to packageJsonRef if package.json becomes editable.The
packageJsonRefis initialized but never updated. If the package.json file becomes editable in future iterations, this could lead to stale data being used during project file creation.Since the package file is currently not marked as editable in PLAYGROUND_FILES, this is acceptable for now. However, if you plan to allow editing package.json, ensure
packageJsonRef.currentis updated when changes are made.
344-349: Autofocus flag never resets, preventing refocus on remount.The
didAutofocusEditorRefflag is set totrueon first mount but never resets, which means the editor won't autofocus if the component remounts (e.g., after navigation).If this behavior is intentional to prevent unexpected focus changes, that's fine. Otherwise, consider resetting the flag in a cleanup function or when installStatus changes to "ready" to ensure the editor gains focus when the workspace becomes available.
138-175: Verify that the generated OpenAPI spec is trusted data.The
createRedocPreviewHtmlfunction injects the OpenAPI spec directly into a script tag. While basic escaping is applied (lines 139-141), this could pose an XSS risk if the spec contains malicious content.Confirm that the OpenAPI spec is always generated by itdoc from trusted sources and cannot be tampered with. If there's any possibility of untrusted data entering the spec, additional sanitization may be required.
Based on learnings, the spec is generated from the itdoc library output, so it should be safe. However, verifying this assumption is recommended for defense-in-depth.
96-127: LGTM! Function signature correctly updated.The
createProjectFilesfunction now properly accepts thepackageJsonparameter and uses it when creating the file system tree. This aligns with the modular refactor and the initialization flow.
303-325: LGTM! Tab closing logic handles edge cases well.The
handleCloseTabfunction correctly prevents closing the last tab and properly updates the active file when closing the currently active tab. The fallback logic ensures a valid tab remains active.
750-827: LGTM! Component composition is well-structured.The modular component composition with TopBar, FileExplorer, EditorTabs, RunModal, SwaggerPreview, and InstallOverlay is clean and follows React best practices. Props are properly wired to state and handlers.
PENEKhun
changed the title
PENEKhun
changed the title
TO-BE video
qwer.mov
changes
How to test for reviewer
localhost:3000with browserSummary by CodeRabbit
New Features
Refactor
Style
Chores