refactor(app): extract pawwork workspace dialogs from layout.tsx - #1063
Conversation
Slice 6 of the layout governance line (#1056). Pure extraction: moves the DialogDeleteWorkspace / DialogResetWorkspace confirm-dialog components out of pages/layout.tsx into pages/layout/pawwork-workspace-dialogs.tsx via a createWorkspaceDialogs(input) factory. layout.tsx 1358 -> 1234. No user-visible behavior change (DOM / copy / aria / i18n keys / styles untouched). The two components are verbatim copies with an input. prefix; the factory takes minimal Pick<> capability slices plus the slice-5 lifecycle deleteWorkspace / resetWorkspace as input. The dialog.show(() => <DialogX .../>) call sites in workspaceSidebarCtx are unchanged (the components are now destructured from the factory). Removed now-unused layout.tsx imports: Dialog, Button (only the extracted dialogs used them). These are presentational components; the logic-bearing deleteWorkspace / resetWorkspace they call are already unit-tested in slice 5. Solid component render tests in this repo require the browser-subprocess harness (@/testing/browser-subprocess) because JSX templates need a client solid-js/web build, which is disproportionate for two confirm dialogs — so this slice relies on typecheck + the full app suite + codex equivalence review. Verification: bun run typecheck 8/8; cd packages/app && bun test = 1667 pass; eslint clean. Refs #1056. #1053 (automation) also touches pages/layout.tsx and will rebase on top.
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/pages/layout.tsx, packages/app/src/pages/layout/pawwork-workspace-dialogs.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
📝 WalkthroughWalkthroughThis PR extracts workspace delete and reset dialog components from ChangesWorkspace dialog extraction and refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the Layout component in layout.tsx by extracting the DialogDeleteWorkspace and DialogResetWorkspace dialog components into a new file, pawwork-workspace-dialogs.tsx. The new file exports a createWorkspaceDialogs function that encapsulates the logic and state for these dialogs, improving code organization and readability. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Align the factory + input type with the file name (pawwork-workspace-dialogs.tsx) and the sibling controllers (createPawworkWorkspaceLifecycle / -RoutingActions / -ProjectControls). The factory was inconsistently unprefixed while its file and siblings carry the pawwork-* fork-marking convention.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/app/src/pages/layout/pawwork-workspace-dialogs.tsx (1)
53-58: ⚡ Quick winOptional: de-duplicate the status-loading and
description()logic.Both dialogs contain an identical
description()(Lines 53-58 and 120-125) and a near-identicalonMountfile.statusloader (Lines 31-42 and 99-111). Since they now live side-by-side in this new module, a small shared helper (e.g.useWorkspaceDirtyStatus(directory)returning{ status, dirty, description }) would remove the duplication without changing behavior. Safe to defer given this is a mechanical extraction.Also applies to: 99-125
🤖 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 `@packages/app/src/pages/layout/pawwork-workspace-dialogs.tsx` around lines 53 - 58, Extract the duplicated status/description and the file.status onMount loader into a small reusable hook (e.g., useWorkspaceDirtyStatus(directory)) and replace both local description() functions and their onMount loaders with calls to that hook; the hook should encapsulate returning { status, dirty, description } (compute description using the same language.t keys used in description()) and perform the file.status fetch/subscribe logic currently duplicated in the two onMount blocks so callers (the dialogs) simply call useWorkspaceDirtyStatus(directory) and use its returned values.
🤖 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.
Nitpick comments:
In `@packages/app/src/pages/layout/pawwork-workspace-dialogs.tsx`:
- Around line 53-58: Extract the duplicated status/description and the
file.status onMount loader into a small reusable hook (e.g.,
useWorkspaceDirtyStatus(directory)) and replace both local description()
functions and their onMount loaders with calls to that hook; the hook should
encapsulate returning { status, dirty, description } (compute description using
the same language.t keys used in description()) and perform the file.status
fetch/subscribe logic currently duplicated in the two onMount blocks so callers
(the dialogs) simply call useWorkspaceDirtyStatus(directory) and use its
returned values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c4e2874-fe38-4002-bb53-d78c49df959e
📒 Files selected for processing (2)
packages/app/src/pages/layout.tsxpackages/app/src/pages/layout/pawwork-workspace-dialogs.tsx
Summary
Slice 6 of the layout governance line (#1056). Pure extraction: moves the
DialogDeleteWorkspace/DialogResetWorkspaceconfirm-dialog components out ofpages/layout.tsxintopages/layout/pawwork-workspace-dialogs.tsxvia acreateWorkspaceDialogs(input)factory.layout.tsx1358 → 1234.No user-visible behavior change (DOM / copy / aria / i18n keys / styles untouched).
Touched files
packages/app/src/pages/layout.tsx— remove the two component definitions + now-unusedDialog/Buttonimports; add the factory destructure (placed after the slice-5 lifecycle controller sodeleteWorkspace/resetWorkspaceare in scope).packages/app/src/pages/layout/pawwork-workspace-dialogs.tsx— new factorycreateWorkspaceDialogs.Review focus
input.injection (especiallyDialogDeleteWorkspace.handleDelete:leaveDeletedWorkspacecomputation → navigate →dialog.close()→deleteWorkspace(root, dir, leave)ordering).props: { root, directory }and thedialog.show(() => <DialogX .../>)call sites are unchanged.Dialog/Buttonwere only used by the extracted dialogs (confirmed zero other refs in layout.tsx).Verification
bun run typecheck8/8cd packages/app && bun test→ 1667 pass (no source-guard / shell-frame-contract breakage)eslintcleanNo new unit test: these are presentational components and the logic-bearing
deleteWorkspace/resetWorkspaceare already unit-tested in slice 5 (#1062). Solid render tests here need the@/testing/browser-subprocessharness (JSX needs a client solid-js/web build), which is disproportionate for two confirm dialogs.Risk
Low. Mechanical extraction; behavior preserved. #1053 (automation) also touches
pages/layout.tsxand will rebase on top.Refs #1056.
Summary by CodeRabbit