Repository navigation
refactor(desktop): remove the old workspace layout - #9052
saddlepaddle wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Route workspace interactions through the right pane apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx, .../components/AddTabMenu/AddTabMenu.tsx, .../components/SidebarSlide/*, .../components/WorkspaceSplitLayout/*, .../hooks/usePaneAreaMoveActions/*, .../hooks/usePaneRegistry/*, .../hooks/useRightPaneAreaLifecycle/*, .../hooks/useWorkspaceFileNavigation/*, .../hooks/useWorkspaceHotkeys/*, .../hooks/useWorkspacePaneOpeners/*, .../hooks/useWorkspaceRightPane/*, .../hooks/useV2WorkspacePaneLayout/* |
The workspace page now routes viewer and Changes-pane interactions through the right store. Pane hooks and rendering no longer use the legacy sidebar branches or pane-location flags. SidebarSlide no longer accepts keepMounted; it remains mounted after it first opens. |
Remove legacy sidebar and Changes controls apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/ChangesControl/*, .../components/WorkspaceSidebar/*, .../components/V2WorkspaceOpenInButton/*, .../components/V2WorkspaceRunButton/*, .../components/WorkspaceActivityMenu/*, .../state/rowlessSidebarTabStore/*, .../utils/openChangesPaneInStore/*, apps/desktop/src/renderer/lib/github/openPullRequestLink/pullRequestLinkSurfaces.test.ts |
The legacy sidebar, shipping and pull-request controls, and rowless sidebar-tab store were removed. The activity menu no longer displays pinned pages. openChangesPaneInStore now focuses the first diff pane in tab order or adds a diff tab. |
Retire sidebar and Changes preferences apps/desktop/src/renderer/hooks/useV2UserPreferences/*, apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/*, .../routes/_authenticated/components/V2NotificationController/*, .../routes/_authenticated/settings/*, apps/desktop/src/renderer/stores/*, apps/desktop/src/renderer/stores/workspace-creates/*, packages/i18n/locales/*/messages.po |
The workspace sidebar-tab and global right-sidebar preferences, and the Changes open-target setting, were removed from schemas, stores, and settings UI. Notification filtering now uses workspace-local sidebar state. Obsolete message entries were removed from locale catalogs. |
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Refactor
Possibly related issues
- Clicking a file in the Changes sidebar opens a split pane even when Changes is set to open in a new tab #9039: This change removes the Changes open-target setting and related legacy pane-opening paths. The summaries do not establish whether it resolves the issue’s specific file-opening behavior.
Suggested reviewers: kitenite, harshithmullapudi
Merge Risk: 
Minimal · up to b4f94
-
No actionable issue remains from the reviewed changes; the workspace can proceed through normal merge checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "refactor(desktop): remove the old workspace layout" follows conventional commit format and matches the main change. The PR removes the legacy center-pane layout and right WorkspaceSidebar, leaving the right pane area as the only workspace layout. The title is specific and concise, and it names the scope (desktop).
Full details: Description check
-
Explanation
The description follows the repository template. It has the 'What & why' section, which explains the removed dead code and lists deletions and flag parameter removals. It has the 'How I tested it' section, which lists typecheck, lint, and test runs, plus an i18n check and an explicit note that the running app was not verified. The checklist is present. Two checklist items are unchecked: 'Allow edits from maintainers' and the CLA signature. These items are optional for this internal change and do not block the check. No linked issue is given in the 'What & why' section, but the template only says to link one if it exists, and the PR cites
#9008and#9204as context.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 15 functions across 23 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
|
Preview update published for development build |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
[Medium impact] The PR appears safe to merge; the removed layout branches were unreachable. SummaryRemoves the old workspace layout and keeps the right pane as the only layout.
No actionable issues were found. This review used source checks; it did not verify the running app. Acknowledged scope: Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
Navigation[Workspace navigation] --> Center[Center: terminals and chats]
Navigation --> Viewer[Viewer actions]
Viewer --> Right[Right pane: files, changes, browser and review]
Center -->|Move viewer panes| Right
Center --> CenterSaved[Saved paneLayout]
Right --> RightSaved[Saved rightPaneLayout]
Reviews (1) · Last reviewed commit: "refactor(desktop): remove the old worksp..." · Reviewed by Greptile |
The right pane area is on for everyone (#9008) and useRightPaneAreaEnabled always returned true, so every branch for the old layout was dead code. Remove the old right sidebar (WorkspaceSidebar, its header and PR header), the sidebar tab state (setWorkspaceSidebarTab, rowlessSidebarTabStore, sidebarState.activeTab), the center Changes pane and its changesOpenTarget setting, ChangesControl with ShipControl and PRStatusGroup, V2WorkspaceOpenInButton, V2WorkspaceRunButton, the old AddTabMenu items, the right-pane-area flag store, the user preference fields rightSidebarOpen, rightSidebarTab and rightSidebarWidth, and the file pane branches for files in the center area. Hooks and components lose the flag parameters: useWorkspaceRightPane, useRightPaneMode, useRightPaneAreaLifecycle, usePaneAreaMoveActions, WorkspaceSplitLayout, SidebarSlide, WorkspaceActivityMenu, V2NotificationController and LinkTierMapper. The tab hooks and FilesTab under WorkspaceSidebar/ stay: the right pane tabs still import them.
fde9059 to
b4f9493
Compare
What & why
The right pane area has been on for everyone since #9008.
useRightPaneAreaEnabled()always returnedtrue, so every branch for the old layout (everything in the center plus a rightWorkspaceSidebarwith Changes/Files/Review tabs) was dead code. This removes it, so the right pane area is the only layout. It is the first step of making the right pane the default model for workspace view state.No behaviour change is intended for anyone: every removed branch was unreachable.
Deleted
WorkspaceSidebarwith itsSidebarHeaderandPRActionHeader.setWorkspaceSidebarTab,rowlessSidebarTabStore,sidebarState.activeTabandWORKSPACE_SIDEBAR_TABS.openChangesPane/toggleChangesPaneinuseWorkspacePaneOpeners, and the"pane"mode ofopenChangesPaneInStore.changesOpenTargetsetting, with its Behavior settings row and settings-search entry.ChangesControl, withShipControlandPRStatusGroup. It was only rendered by the old tab bar; the new toolbar and activity menu cover it.V2WorkspaceOpenInButtonandV2WorkspaceRunButton.AddTabMenu's Browser / Changes / Desktop items (the right pane options replace them).stores/right-pane-area.ts. Its key is already inDEAD_KEYS.flag === falseeffect inuseRightPaneAreaLifecyclethat moved tabs back to the center, andSIDEBAR_PANE_KINDS.selectMenuPages'orgPages/favoritePageIds.showTree/inRightPane.moveViewerPanesToRightkeeps files out of the center.useWorkspaceFileNavigation'sselectedFilePath/pendingReveal, which only the oldFilesTabread.Flag parameters removed from:
useWorkspaceRightPane,useRightPaneMode(canExpand; full screen itself now lives instores/right-pane-full-screen.tssince feat(cli): let agents drive the right sidebar with superset panes #9204),useRightPaneAreaLifecycle,usePaneAreaMoveActions;WorkspaceSplitLayout/SidebarSlide(keepMounted),WorkspaceActivityMenu(changesis now required),V2NotificationController,LinkTierMapper;usePaneRegistry(rightStore).Saved state. The user preference fields
rightSidebarOpen,rightSidebarTabandrightSidebarWidth(with their setters),sidebarState.activeTab, andsettings.changesOpenTargetare removed from the schemas.DEAD_KEYSentries are needed.Kept on purpose
WorkspaceSidebar/hooks/useChangesTab,useReviewTab,components/FilesTab,utilsandtypes.ts. The right-pane tabs import them. Moving them under the panes that use them is a follow-up.RightPaneLinkTargetstays nullable.PullRequestCodeTabandDiffPanerender on the pull-requests page too, wherenullmeans "not in a workspace".splitWithViewer's same-store branch: it still splits inside the right pane from the right pane's own context menu.Existing gap, not changed here. Revealing a path in the file tree from a terminal link opens the right pane but reveals nothing: only the old sidebar's
FilesTabreadpendingReveal. That was already the case before this PR.How I tested it
bun run typecheckinapps/desktop,bun run lint, and the full desktopbun test(9,039 pass, 0 fail), re-run after rebasing on main (feat(cli): let agents drive the right sidebar with superset panes #9204, the newuklocale).bun run check:i18n: the catalogs only lose 15 messages that belonged to removed UI. No new messages, and every locale, including the newuk, is still fully translated.SidebarSlide,WorkspaceSplitLayout,useRightPaneMode,openChangesPaneInStore,selectMenuPages, the schema/heal tests, and the PR-link surfaces test (drops the deletedPRStatusGroup). Deleted: the sidebar-tab tests.Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)Summary by CodeRabbit