Skip to content

refactor: modularize playground page orchestration#231

Open
HimasreeKolathur24 wants to merge 3 commits into
piyushdotcomm:mainfrom
HimasreeKolathur24:refactor-playground-page
Open

refactor: modularize playground page orchestration#231
HimasreeKolathur24 wants to merge 3 commits into
piyushdotcomm:mainfrom
HimasreeKolathur24:refactor-playground-page

Conversation

@HimasreeKolathur24
Copy link
Copy Markdown
Contributor

@HimasreeKolathur24 HimasreeKolathur24 commented May 21, 2026

Summary

Refactored app/playground/[id]/page.tsx by extracting orchestration logic and UI sections into modular hooks and components.

Changes made

  • Extracted editor workspace into playground-editor-area.tsx
  • Extracted modal orchestration into playground-modals.tsx
  • Extracted save logic into useSaveHandlers.ts
  • Extracted keyboard shortcuts into useKeyboardShortcuts.ts
  • Extracted ZIP download logic into useDownload.ts
  • Extracted wrapped file handlers into useWrappedFileHandlers.ts
  • Extracted playground dialog state handling into usePlaygroundDialogs.ts

Why

The original page.tsx acted as a large orchestrator with heavy prop drilling and centralized logic. This refactor improves readability, modularity, and maintainability while reducing component complexity.

Type of change

  • Refactor

Related issue

Closes #31

Validation

  • npm run lint
  • npm test
  • npm run build

Manual verification

  • Verified application compiles successfully
  • Verified playground page renders correctly
  • Verified no runtime console errors after refactor

Checklist

  • I kept this PR focused on one primary change
  • I did not commit secrets, local logs, or scratch files
  • I am requesting review on the correct scope

Summary by CodeRabbit

  • New Features

    • Global keyboard shortcuts for common actions
    • Download project as a ZIP
    • Dedicated editor area and consolidated modals (command palette, AI settings, deploy)
  • Improvements

    • Layout restructured into sidebar + editor + status bar
    • Safer save flows: single-file and save-all with clearer success/warning/error toasts
    • Auto-opens a default file when preview becomes visible
    • Editor wrapped with crash fallback/reload and AI chat protected by an error boundary

Review Change Stack

@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions
Copy link
Copy Markdown

👋 Thanks for opening a PR, @HimasreeKolathur24!

Your PR has entered the 🚦 PR Review Pipeline.

Standard PR detected — your PR will follow the standard review pipeline.


What happens next

Stage Reviewer Checks
Stage 1 — Automated Validation 🤖 Bot DCO · Format · AI/Slop · Duplicate
Stage 2 — Human Review 👥 Maintainer Code + Quality Review
Stage 3 — PA / Maintainer Review 🔑 Project Admin Final Merge Decision

A pipeline status comment will appear below and update automatically as your PR progresses.


While you wait

  • Sign all commits (git commit -s)
  • Link your issue (Closes #123)
  • Use a feature branch (not main)
  • Avoid unrelated changes

This comment is posted only once.

@github-actions github-actions Bot added the help wanted Extra attention is needed label May 21, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Splits the monolithic playground page into focused hooks and components: dialog/keyboard hooks, save/download/wrapped file handlers, a PlaygroundEditorArea component, and PlaygroundModals; the main page composes these pieces and reorders side effects and rendering.

Changes

Playground Component Decomposition

Layer / File(s) Summary
Refactored MainPlaygroundPage
app/playground/[id]/page.tsx
Delegates UI orchestration to extracted hooks/components; reorganizes imports and effects (bootstrap, success toast, auto-open default file), wires keyboard shortcuts and save/download/wrapped handlers, and exports an expression-bodied Suspense wrapper.
PlaygroundEditorArea component
modules/playground/components/playground-editor-area.tsx
Consolidates editor/preview layout orchestration, rendering tabs, breadcrumbs, resizable panels, and a client-only dynamic editor; shows WelcomeScreen when no files are open and wraps the editor in an ErrorBoundary with reload fallback.
PlaygroundModals component
modules/playground/components/playground-modals.tsx
Renders and wires AISettingsDialog, CommandPalette, and DeployDialog with callbacks for dialog state, sidebar/chat toggling, and template/project selection/save/download actions.
useDownload
modules/playground/hooks/useDownload.ts
Recursively builds a ZIP from TemplateFolder tree and triggers a browser download via object URL; reports success/failure with toasts.
Dialog state and keyboard input hooks
modules/playground/hooks/usePlaygroundDialogs.ts, modules/playground/hooks/useKeyboardShortcuts.ts
usePlaygroundDialogs manages preview/AI/command/deploy boolean state. useKeyboardShortcuts registers a global keydown handler to trigger save, save-all, command palette, sidebar/preview/AI toggles, and close-file actions.
useSaveHandlers
modules/playground/hooks/useSaveHandlers.ts
Provides handleSave and handleSaveAll, updates the template tree, attempts WebContainer writes (writeFileSync or instance.fs.writeFile), persists via saveTemplateData, manages openFiles unsaved flags, and shows toasts.
useWrappedFileHandlers
modules/playground/hooks/useWrappedFileHandlers.ts
Returns memoized wrappers around useFileExplorer file/folder handlers that forward instance, writeFileSync, and saveTemplateData as required.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

type:refactor, mentor:piyushdotcomm, level:advanced, gssoc26

Suggested reviewers

  • piyushdotcomm

Poem

🐰 I hopped through hooks and split the page in two,
I zipped the files and taught the editor to chew.
Modals snug in pockets, shortcuts lined the way,
The playground hums at night and gleams by day. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: modularizing the playground page orchestration through refactoring.
Description check ✅ Passed The PR description provides a clear summary of changes, lists all extracted modules, explains the rationale for refactoring, and includes validation results and checklist items.
Linked Issues check ✅ Passed The PR successfully extracts logical UI groups and hooks, reducing prop-drilling and significantly lowering page.tsx line count from 503 to 146 lines (71% reduction), fully meeting issue #31 acceptance criteria.
Out of Scope Changes check ✅ Passed All changes are focused on decomposing the monolithic MainPlaygroundPage component into modular hooks and components, with no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/playground/components/playground-editor-area.tsx`:
- Around line 117-124: The call to WebContainerPreview is using a non-null
assertion serverUrl! which hides the actual string | null type; change the prop
to pass a nullable-safe value (e.g., serverUrl ?? "") when invoking
WebContainerPreview in playground-editor-area.tsx, or alternatively update
WebContainerPreviewProps to accept string | null/optional and propagate that
change through its props; locate the invocation of WebContainerPreview and
replace serverUrl! with a safe expression (serverUrl ?? "") or update the prop
type accordingly.

In `@modules/playground/components/playground-modals.tsx`:
- Line 64: The command palette's "close all files" action is currently a no-op
because onCloseAllFiles is set to () => {} in playground-modals.tsx; replace the
noop by passing a real handler prop from the parent/page: add an onCloseAllFiles
prop to the PlaygroundModals component (or the component that renders the
command palette) and wire that prop through to where onCloseAllFiles is used,
ensuring the parent page supplies the actual closeAllFiles handler (preserving
existing behavior) and update any relevant prop types/interfaces (e.g., the
PlaygroundModalsProps) so the handler is required.

In `@modules/playground/hooks/useKeyboardShortcuts.ts`:
- Around line 25-63: The keyboard handler handleKeyDown in
useKeyboardShortcuts.ts should ignore global shortcuts when the event target is
an editable element; add an early return at the top of handleKeyDown that checks
if e.target (or e.composedPath()[0]) is an INPUT, TEXTAREA or has
contentEditable not "false" (or otherwise matches editable elements) and if so
do nothing, then keep the existing shortcut logic (handleSave, handleSaveAll,
setIsCommandPaletteOpen, sidebar.toggleSidebar, setIsPreviewVisible,
useAI.getState().toggleChat, closeFile using activeFileId). Apply the identical
guard to the other handleKeyDown in usePlaygroundActions.ts so neither global
handler runs while focus is inside editable controls.

In `@modules/playground/hooks/useSaveHandlers.ts`:
- Around line 51-66: The current updateContentInTree routine matches files by
filename + fileExtension (in updateContentInTree, comparing
item.filename/item.fileExtension to fileToSave) which can hit duplicates across
folders; modify the logic to match a stable unique identifier (e.g.,
fileToSave.resolvedPath or fileToSave.id) instead of name+extension. Add or use
a fullPath/id property on TemplateFile (or compute the path while recursing by
carrying the parent path into updateContentInTree), compare that fullPath/id to
fileToSave.fullPath or fileToSave.id when deciding to replace content, and only
update the single matching node in updatedTemplateData.items to avoid updating
multiple files.

In `@modules/playground/hooks/useWrappedFileHandlers.ts`:
- Around line 25-28: The wrapped add-file handler passes writeFileSync (via
wrappedHandleAddFile in useWrappedFileHandlers) unguarded into handleAddFile,
but writeFileSync from useWebContainer throws if instance is not ready; change
the call to only pass writeFileSync when instance is non-null (or otherwise
guard the sync inside wrappedHandleAddFile), e.g. check instance before invoking
handleAddFile or pass null/undefined for writeFileSync until ready so
handleAddFile’s existing if (writeFileSync) check is reliable; update error
handling/message in handleAddFile to avoid surfacing a “Failed to create file”
for failures that only affect remote sync while local template update succeeded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 36e241e5-128f-49c8-8959-a42138280f6f

📥 Commits

Reviewing files that changed from the base of the PR and between 45caa37 and e064ffd.

📒 Files selected for processing (8)
  • app/playground/[id]/page.tsx
  • modules/playground/components/playground-editor-area.tsx
  • modules/playground/components/playground-modals.tsx
  • modules/playground/hooks/useDownload.ts
  • modules/playground/hooks/useKeyboardShortcuts.ts
  • modules/playground/hooks/usePlaygroundDialogs.ts
  • modules/playground/hooks/useSaveHandlers.ts
  • modules/playground/hooks/useWrappedFileHandlers.ts

Comment thread modules/playground/components/playground-editor-area.tsx
Comment thread modules/playground/components/playground-modals.tsx Outdated
Comment thread modules/playground/hooks/useKeyboardShortcuts.ts
Comment thread modules/playground/hooks/useSaveHandlers.ts
Comment thread modules/playground/hooks/useWrappedFileHandlers.ts
@HimasreeKolathur24 HimasreeKolathur24 force-pushed the refactor-playground-page branch from 6d24b15 to 13b7623 Compare May 21, 2026 08:46
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
modules/playground/hooks/useSaveHandlers.ts (1)

51-66: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a unique path-based match when updating the tree.

At Line 58, matching by filename + fileExtension can update multiple files in different folders with the same name. Use the resolved path (or a stable id) to update exactly one node.

Suggested fix
-        const updateContentInTree = (
-          items: (TemplateFile | TemplateFolder)[]
-        ): (TemplateFile | TemplateFolder)[] =>
-          items.map((item) => {
-            if ("folderName" in item) {
-              return { ...item, items: updateContentInTree(item.items) };
-            } else if (
-              item.filename === fileToSave.filename &&
-              item.fileExtension === fileToSave.fileExtension
-            ) {
-              return { ...item, content: fileToSave.content };
-            }
-            return item;
-          });
+        const normalizedTargetPath = filePath.replace(/^\/+/, "");
+        const updateContentInTree = (
+          items: (TemplateFile | TemplateFolder)[],
+          currentPath = ""
+        ): (TemplateFile | TemplateFolder)[] =>
+          items.map((item) => {
+            if ("folderName" in item) {
+              const nextPath = currentPath
+                ? `${currentPath}/${item.folderName}`
+                : item.folderName;
+              return { ...item, items: updateContentInTree(item.items, nextPath) };
+            }
+
+            const itemName = item.fileExtension
+              ? `${item.filename}.${item.fileExtension}`
+              : item.filename;
+            const itemPath = `${currentPath}/${itemName}`.replace(/^\/+/, "");
+            if (itemPath === normalizedTargetPath) {
+              return { ...item, content: fileToSave.content };
+            }
+            return item;
+          });
 
-        updatedTemplateData.items = updateContentInTree(updatedTemplateData.items);
+        updatedTemplateData.items = updateContentInTree(updatedTemplateData.items);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/playground/hooks/useSaveHandlers.ts` around lines 51 - 66, The
current updateContentInTree function matches files by filename + fileExtension
which can collide across folders; change it to match a unique path/id: modify
updateContentInTree to accept a currentPath string (or build a path as you
recurse), construct each node's resolvedPath (e.g.,
`${currentPath}/${folderName}` for folders and
`${currentPath}/${filename}.${fileExtension}` for files), and compare that
resolvedPath to fileToSave.resolvedPath (or fileToSave.path/id) when deciding to
replace content in updatedTemplateData.items; ensure recursion passes the
updated currentPath so only the exact node is updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/playground/`[id]/page.tsx:
- Around line 69-71: The useEffect currently fires toast.success when isSuccess
and templateData change, causing repeated toasts when templateData mutates;
change it so the load toast runs only once by tracking whether the toast has
already been shown (e.g., add a ref or state like hasShownLoadToast) and only
call toast.success in the useEffect when isSuccess is true, templateData exists,
and hasShownLoadToast is false, then set hasShownLoadToast to true; update the
useEffect dependencies to avoid including templateData changes (use isSuccess
and the ref/state) so the toast triggers only on the initial successful load in
the useEffect that references useEffect, isSuccess, templateData, and
toast.success.
- Around line 74-92: The effect opens a default file whenever activeFileId is
falsy, not only on the transition when the preview is opened; modify the
useEffect that contains findDefaultFile/openFile so it only runs when
dialogs.isPreviewVisible transitions from false to true (and activeFileId is
falsy and templateData exists). Implement this by tracking the previous
dialogs.isPreviewVisible (e.g., a useRef prevIsPreviewVisible) and inside the
effect require prevIsPreviewVisible === false && dialogs.isPreviewVisible ===
true before running findDefaultFile/openFile, then update the prev ref at the
end; keep the existing helper findDefaultFile and openFile references.

In `@modules/playground/hooks/useKeyboardShortcuts.ts`:
- Around line 36-67: Normalize the keyboard event's key once at the top of the
shortcut handler (e.g., const key = e.key.toLowerCase()) and use that lowercase
variable for all letter comparisons so Caps Lock doesn't break shortcuts; update
checks that currently test e.key === "s"/"S"/"k"/"b"/"w"/"A"/"P" to use key ===
"s"/"k"/"b"/"w"/"a"/"p" respectively while retaining existing ctrl/shift logic
and special-key checks (e.g., keep the backslash check for e.key === "\\" as-is
if needed), ensuring handlers like handleSave, handleSaveAll,
setIsCommandPaletteOpen, sidebar.toggleSidebar, setIsPreviewVisible, and
useAI.getState().toggleChat are invoked unchanged.

In `@modules/playground/hooks/useSaveHandlers.ts`:
- Around line 44-47: The code currently swallows unresolved-path failures by
calling toast.error and returning (e.g., the block checking filePath for
fileToSave in useSaveHandlers.ts), which lets higher-level "Save All" treat the
save as successful; change these early returns to throw a consistent Error
(include the same descriptive message used in the toast) so the promise rejects
and Save All can detect failure. Update every similar pattern in this file (the
blocks around filePath/fileToSave referenced at the other occurrences) to both
toast.error(...) and then throw new Error(<same message>), and ensure the caller
of the save function will propagate/reject accordingly.

---

Duplicate comments:
In `@modules/playground/hooks/useSaveHandlers.ts`:
- Around line 51-66: The current updateContentInTree function matches files by
filename + fileExtension which can collide across folders; change it to match a
unique path/id: modify updateContentInTree to accept a currentPath string (or
build a path as you recurse), construct each node's resolvedPath (e.g.,
`${currentPath}/${folderName}` for folders and
`${currentPath}/${filename}.${fileExtension}` for files), and compare that
resolvedPath to fileToSave.resolvedPath (or fileToSave.path/id) when deciding to
replace content in updatedTemplateData.items; ensure recursion passes the
updated currentPath so only the exact node is updated.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5415d1f2-6034-4d27-a42e-5fd1730e5043

📥 Commits

Reviewing files that changed from the base of the PR and between 6d24b15 and 13b7623.

📒 Files selected for processing (8)
  • app/playground/[id]/page.tsx
  • modules/playground/components/playground-editor-area.tsx
  • modules/playground/components/playground-modals.tsx
  • modules/playground/hooks/useDownload.ts
  • modules/playground/hooks/useKeyboardShortcuts.ts
  • modules/playground/hooks/usePlaygroundDialogs.ts
  • modules/playground/hooks/useSaveHandlers.ts
  • modules/playground/hooks/useWrappedFileHandlers.ts
✅ Files skipped from review due to trivial changes (1)
  • modules/playground/hooks/usePlaygroundDialogs.ts

Comment thread app/playground/[id]/page.tsx
Comment thread app/playground/[id]/page.tsx Outdated
Comment thread modules/playground/hooks/useKeyboardShortcuts.ts Outdated
Comment thread modules/playground/hooks/useSaveHandlers.ts
@HimasreeKolathur24 HimasreeKolathur24 force-pushed the refactor-playground-page branch from 238657d to 5e002cb Compare May 21, 2026 09:04
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
modules/playground/hooks/useSaveHandlers.ts (2)

57-61: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a unique identifier/path for tree updates.

Line 58-60 matches by filename + fileExtension, which can update multiple files when names repeat in different folders.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/playground/hooks/useSaveHandlers.ts` around lines 57 - 61, The update
currently matches items by filename and fileExtension (in the block comparing
item.filename === fileToSave.filename && item.fileExtension ===
fileToSave.fileExtension) which can collide across folders; change the
comparison to use a unique identifier such as item.path, item.fullPath or an
explicit id (e.g., item.id === fileToSave.id or item.fullPath ===
fileToSave.fullPath) and update any call sites that set fileToSave so the unique
field is populated; modify the update branch in useSaveHandlers.ts to match on
that unique key and then return the patched item with updated content to avoid
accidental updates of files with the same name.

44-47: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate unresolved-path failures instead of returning silently.

Line 46 returns without rejecting, so batch save can still appear successful after a file failed to resolve.

Suggested patch
         if (!filePath) {
-          toast.error(`Could not find path for file: ${fileToSave.filename}.${fileToSave.fileExtension}`);
-          return;
+          const msg = `Could not find path for file: ${fileToSave.filename}.${fileToSave.fileExtension}`;
+          toast.error(msg);
+          throw new Error(msg);
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/playground/hooks/useSaveHandlers.ts` around lines 44 - 47, The
current early return in useSaveHandlers (when filePath is falsy) swallows
failures causing batch saves to appear successful; instead propagate the error
by rejecting the save operation: inside the save handler where fileToSave and
filePath are checked, replace the plain return with throwing an Error or
returning a rejected Promise that includes the filename and extension so callers
(e.g., batchSave) can detect and surface the failure. Ensure the thrown/rejected
error is descriptive (e.g., "Could not find path for file: ...") so upstream
logic can handle or aggregate failures correctly.
modules/playground/hooks/useKeyboardShortcuts.ts (1)

42-63: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix unreachable Ctrl+Shift shortcuts after key normalization.

Line 26 lowercases e.key, but Line 42/47/62 compare against "S", "P", and "A". Those branches never match, so those shortcuts won’t fire.

Suggested patch
-      if (e.ctrlKey && e.shiftKey && key === "S") {
+      if (e.ctrlKey && e.shiftKey && key === "s") {
         e.preventDefault();
         handleSaveAll();
       }
@@
-      if ((e.ctrlKey && key === "k") || (e.ctrlKey && e.shiftKey && key === "P")) {
+      if ((e.ctrlKey && key === "k") || (e.ctrlKey && e.shiftKey && key === "p")) {
         e.preventDefault();
         setIsCommandPaletteOpen(true);
       }
@@
-      if (e.ctrlKey && e.shiftKey && key === "A") {
+      if (e.ctrlKey && e.shiftKey && key === "a") {
         e.preventDefault();
         useAI.getState().toggleChat();
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/playground/hooks/useKeyboardShortcuts.ts` around lines 42 - 63, In
useKeyboardShortcuts, the handler lowercases e.key but then compares to
uppercase letters, so Ctrl+Shift shortcuts (handleSaveAll,
setIsCommandPaletteOpen, AI chat) never match; fix by comparing against
lowercase keys (e.g., "s", "p", "a", "k", "b", "\\") or stop normalizing e.key —
update the comparisons in the keydown handler for handleSaveAll,
setIsCommandPaletteOpen, sidebar.toggleSidebar, setIsPreviewVisible and the AI
chat branch to use the normalized lowercase values and preserve the existing
ctrl/shift boolean checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/playground/hooks/useDownload.ts`:
- Around line 26-46: The download function handleDownloadZip currently creates
an object URL and a temporary anchor but only cleans them up inside the try
block, risking leaks if an exception is thrown; refactor handleDownloadZip so
url and link are declared in outer scope before the try, perform the JSZip
generation and link.click() inside try/catch, and move
document.body.removeChild(link) and window.URL.revokeObjectURL(url) into a
finally block that checks for non-null link and url (created via
window.URL.createObjectURL) to ensure removal and revocation always run;
reference handleDownloadZip, addFilesToZip, window.URL.createObjectURL,
window.URL.revokeObjectURL and the temporary link element when making the
changes.

In `@modules/playground/hooks/useSaveHandlers.ts`:
- Line 17: The setOpenFiles type and its usage cause lost updates when saves run
concurrently: change the declaration of setOpenFiles from "setOpenFiles: (files:
any[]) => void" to a React state dispatcher type (e.g.,
Dispatch<SetStateAction<any[]>>) so callers can use the functional updater, and
update handleSaveAll (the function around lines 90-100) to call
setOpenFiles(prev => { /* derive new state from prev */ }) instead of reading a
stale openFiles snapshot and setting it directly; make sure all updates to
openFiles inside save completion handlers use the functional form to avoid
races.

---

Duplicate comments:
In `@modules/playground/hooks/useKeyboardShortcuts.ts`:
- Around line 42-63: In useKeyboardShortcuts, the handler lowercases e.key but
then compares to uppercase letters, so Ctrl+Shift shortcuts (handleSaveAll,
setIsCommandPaletteOpen, AI chat) never match; fix by comparing against
lowercase keys (e.g., "s", "p", "a", "k", "b", "\\") or stop normalizing e.key —
update the comparisons in the keydown handler for handleSaveAll,
setIsCommandPaletteOpen, sidebar.toggleSidebar, setIsPreviewVisible and the AI
chat branch to use the normalized lowercase values and preserve the existing
ctrl/shift boolean checks.

In `@modules/playground/hooks/useSaveHandlers.ts`:
- Around line 57-61: The update currently matches items by filename and
fileExtension (in the block comparing item.filename === fileToSave.filename &&
item.fileExtension === fileToSave.fileExtension) which can collide across
folders; change the comparison to use a unique identifier such as item.path,
item.fullPath or an explicit id (e.g., item.id === fileToSave.id or
item.fullPath === fileToSave.fullPath) and update any call sites that set
fileToSave so the unique field is populated; modify the update branch in
useSaveHandlers.ts to match on that unique key and then return the patched item
with updated content to avoid accidental updates of files with the same name.
- Around line 44-47: The current early return in useSaveHandlers (when filePath
is falsy) swallows failures causing batch saves to appear successful; instead
propagate the error by rejecting the save operation: inside the save handler
where fileToSave and filePath are checked, replace the plain return with
throwing an Error or returning a rejected Promise that includes the filename and
extension so callers (e.g., batchSave) can detect and surface the failure.
Ensure the thrown/rejected error is descriptive (e.g., "Could not find path for
file: ...") so upstream logic can handle or aggregate failures correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b1ec1db1-2e53-46cf-a3fc-15378ff077eb

📥 Commits

Reviewing files that changed from the base of the PR and between 238657d and 5e002cb.

📒 Files selected for processing (8)
  • app/playground/[id]/page.tsx
  • modules/playground/components/playground-editor-area.tsx
  • modules/playground/components/playground-modals.tsx
  • modules/playground/hooks/useDownload.ts
  • modules/playground/hooks/useKeyboardShortcuts.ts
  • modules/playground/hooks/usePlaygroundDialogs.ts
  • modules/playground/hooks/useSaveHandlers.ts
  • modules/playground/hooks/useWrappedFileHandlers.ts

Comment thread modules/playground/hooks/useDownload.ts
Comment thread modules/playground/hooks/useSaveHandlers.ts
@HimasreeKolathur24 HimasreeKolathur24 force-pushed the refactor-playground-page branch from d696297 to aad92ca Compare May 21, 2026 09:17
Copy link
Copy Markdown
Owner

@piyushdotcomm piyushdotcomm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai full review

Copy link
Copy Markdown
Owner

@piyushdotcomm piyushdotcomm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve the merge conflict

@HimasreeKolathur24
Copy link
Copy Markdown
Contributor Author

resolve the merge conflict

@piyushdotcomm, resolved the merge conflicts as suggested. Please let me know if any additional changes or improvements are needed.

Copy link
Copy Markdown
Owner

@piyushdotcomm piyushdotcomm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is still merge conflict

@HimasreeKolathur24 HimasreeKolathur24 force-pushed the refactor-playground-page branch from c847cfe to e662be5 Compare May 26, 2026 06:56
@HimasreeKolathur24
Copy link
Copy Markdown
Contributor Author

@piyushdotcomm, re-verified and resolved the conflicts as suggested. Please let me know if any additional changes are needed.

Copy link
Copy Markdown
Owner

@piyushdotcomm piyushdotcomm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking compilation errors introduced by the refactor. See inline comments.

error: containerError,
instance,
writeFileSync,
} = useWebContainer({ templateData });
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\useWebContainer\ expects 0 arguments. Remove { templateData }.


// Bootstrap
useEffect(() => {
resetUI();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


esetUI\ is called but not imported or defined.

<TooltipProvider>
<>
<PlaygroundSidebar
templateData={templateData}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\PlaygroundSidebar\ does not accept these props. Update its interface.

className="flex-1 w-auto min-w-0 transition-all ease-linear duration-300 relative bg-background"
>
<PlaygroundHeader
id={id}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\PlaygroundHeader\ does not accept these props. Update its interface.

<WebContainerPreview
templateData={templateData}
instance={instance}
writeFileSync={writeFileSync}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\writeFileSync\ can be null. Provide a fallback like \writeFileSync || (async () => {}).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decompose Monolithic MainPlaygroundPage Component

2 participants