[codex] Refactor v2 workspace page#3747
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughExtracts workspace pane, file-navigation, pane-opening, attention-clearing, and dirty-tab close guard logic from the main page into five dedicated hooks: Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Workspace Page
participant Store as Workspace Zustand Store
participant Notif as V2 Notification Store
participant Docs as fileDocumentStore / FS
participant UI as User / Alert UI
Page->>Store: read active tab/pane, actions
Page->>useClearActivePaneAttention: subscribe activePane + status
useClearActivePaneAttention->>Notif: get clearSourceAttention
useClearActivePaneAttention->>Notif: clearSourceAttention(source, workspaceId) -- when status == "review"
Page->>useWorkspaceFileNavigation: request openFilePane/revealPath
useWorkspaceFileNavigation->>Store: openPane / add tab
useWorkspaceFileNavigation->>Docs: recordView or resolve worktreePath
Page->>useDirtyTabCloseGuard: onBeforeCloseTab(tab)
useDirtyTabCloseGuard->>Docs: check document.dirty for file panes
alt dirty files exist
useDirtyTabCloseGuard->>UI: show alert (Save All / Don't Save / Cancel)
UI->>Docs: save() or reload()
UI-->>useDirtyTabCloseGuard: resolve boolean
else
useDirtyTabCloseGuard-->>Page: resolve true
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryThis is a pure structural refactor that extracts five focused hooks from the Confidence Score: 5/5Safe to merge — pure structural refactor with no behavioral changes. All logic was mechanically moved with identical dependencies and return values. No new async paths, no changed state ownership, and no new external dependencies were introduced. All remaining findings would be P2 at best. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx | WorkspaceContent is significantly slimmed down by delegating to the five new hooks; orchestration and wiring remain intact. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts | Extracted from page.tsx — owns selected-file state, pending-reveal state, open-file-paths derivation, openFilePane, and revealPath. Logic is a faithful copy of the original inline code. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.ts | Extracted from page.tsx — provides openDiffPane, addTerminalTab, addChatTab, addBrowserTab, and openCommentPane. All callbacks are correctly memoised and functionally equivalent to the originals. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.ts | Extracted from page.tsx — implements the dirty-tab close dialog (Save All / Don't Save / Cancel) via useCallback. Logic is identical to the original inline handler. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/useClearActivePaneAttention.ts | Extracted from page.tsx — watches the active pane's notification status and clears attention when it enters "review" state. Logic and deps are identical to the original inline version. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/useDefaultPaneActions.tsx | Extracted from page.tsx — wraps the two default pane action configs (split/close) in useMemo with an empty deps array. Behavior is unchanged. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[WorkspaceContent / page.tsx] --> B[useWorkspaceFileNavigation]
A --> C[useWorkspacePaneOpeners]
A --> D[useDefaultPaneActions]
A --> E[useDirtyTabCloseGuard]
A --> F[useClearActivePaneAttention]
B --> B1[openFilePane]
B --> B2[revealPath]
B --> B3[selectedFilePath / pendingReveal]
B --> B4[recentFiles / openFilePaths]
C --> C1[openDiffPane]
C --> C2[addTerminalTab / addChatTab / addBrowserTab]
C --> C3[openCommentPane]
D --> D1[split & close action configs]
E --> E1[onBeforeCloseTab callback]
F --> F1[clears attention when active pane enters review]
Reviews (1): Last reviewed commit: "Refactor v2 workspace page" | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx (1)
140-141: Minor:useV2UserPreferencesis now called in two places.Page calls
useV2UserPreferences()forpreferences, anduseWorkspaceFileNavigationcalls it again forsetRightSidebarOpen/setRightSidebarTab. Functionally fine (assuming it's a Zustand/Context-backed hook), but it's a small duplicate subscription. If it ever becomes a perf hot spot, consider lifting the prefs read once and passing setters into the hook (or vice versa). Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx around lines 140 - 141, The hook useV2UserPreferences is being called twice (once in the page to read preferences and again inside useWorkspaceFileNavigation to get setRightSidebarOpen/setRightSidebarTab); to remove the duplicate subscription, call useV2UserPreferences only once in the page (keep const { preferences: v2UserPreferences, setRightSidebarOpen, setRightSidebarTab } = useV2UserPreferences()) and pass the setters into useWorkspaceFileNavigation (or alternatively have useWorkspaceFileNavigation return handlers and accept preferences) so that useWorkspaceFileNavigation no longer calls useV2UserPreferences itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.ts:
- Around line 32-72: The Promise returned by useDirtyTabCloseGuard can hang
because alert(...) provides no callback when the dialog is dismissed; update the
alert usage (or wrapping logic inside useDirtyTabCloseGuard) to ensure the
Promise always resolves on dialog dismissal by wiring the dialog's
onOpenChange/onDismiss to call the resolver with a default (e.g.,
resolve(false)). Specifically, when creating the alert/modal in
useDirtyTabCloseGuard, capture the resolve function in scope and add an
onOpenChange/onDismiss handler that calls resolve(false) if the dialog is closed
without an action, or alternatively make the dialog non-dismissible (disable
backdrop/escape) so only the explicit Save All / Don't Save / Cancel actions
resolve the Promise; reference the existing alert(...) call, the resolve
variable in the Promise, and the Save All / Don't Save / Cancel action handlers
when implementing the change.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts:
- Around line 105-118: openFilePane can race when worktreePath is not yet loaded
causing non-absolute file paths to be stored; in the openFilePane function check
the workspaceQuery/worktreePath (the worktreePath string) before constructing
absoluteFilePath and calling state.addTab or state.openPane, and if worktreePath
is empty either return early (no-op) or enqueue the request until worktreePath
resolves so only absolute paths are ever passed into state.addTab/state.openPane
(references: openFilePane, absoluteFilePath, state.addTab, state.openPane,
workspaceQuery/worktreePath).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 140-141: The hook useV2UserPreferences is being called twice (once
in the page to read preferences and again inside useWorkspaceFileNavigation to
get setRightSidebarOpen/setRightSidebarTab); to remove the duplicate
subscription, call useV2UserPreferences only once in the page (keep const {
preferences: v2UserPreferences, setRightSidebarOpen, setRightSidebarTab } =
useV2UserPreferences()) and pass the setters into useWorkspaceFileNavigation (or
alternatively have useWorkspaceFileNavigation return handlers and accept
preferences) so that useWorkspaceFileNavigation no longer calls
useV2UserPreferences itself.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77c0e981-1854-4f8b-8c74-30477fc9d626
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/useClearActivePaneAttention.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/useDefaultPaneActions.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
Comment on lines
+32
to
+72
| return new Promise<boolean>((resolve) => { | ||
| alert({ | ||
| title, | ||
| description: "Your changes will be lost if you don't save them.", | ||
| actions: [ | ||
| { | ||
| label: "Save All", | ||
| onClick: async () => { | ||
| for (const pane of dirtyPanes) { | ||
| const filePath = (pane.data as FilePaneData).filePath; | ||
| const doc = getDocument(workspaceId, filePath); | ||
| if (!doc) continue; | ||
| const result = await doc.save(); | ||
| if (result.status !== "saved") { | ||
| resolve(false); | ||
| return; | ||
| } | ||
| } | ||
| resolve(true); | ||
| }, | ||
| }, | ||
| { | ||
| label: "Don't Save", | ||
| variant: "secondary", | ||
| onClick: async () => { | ||
| for (const pane of dirtyPanes) { | ||
| const filePath = (pane.data as FilePaneData).filePath; | ||
| const doc = getDocument(workspaceId, filePath); | ||
| if (doc) await doc.reload(); | ||
| } | ||
| resolve(true); | ||
| }, | ||
| }, | ||
| { | ||
| label: "Cancel", | ||
| variant: "ghost", | ||
| onClick: () => resolve(false), | ||
| }, | ||
| ], | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the alert API definition to confirm dismissal semantics.
fd -t f 'Alert' --extension ts --extension tsx | head -50
rg -nP -C5 '\bexport\s+(?:async\s+)?function\s+alert\b|\bexport\s+const\s+alert\s*='
rg -nP -C3 '\b(onDismiss|onOpenChange|onClose|dismissible)\b' --type=ts --type=tsx -g '**/atoms/Alert/**'Repository: superset-sh/superset
Length of output: 148
🏁 Script executed:
cat packages/ui/src/atoms/Alert/Alert.tsxRepository: superset-sh/superset
Length of output: 2438
🏁 Script executed:
# Find where alert function is defined/exported
rg -n 'alert' packages/ui/src/atoms/Alert/ -A 3Repository: superset-sh/superset
Length of output: 2180
🏁 Script executed:
# Check if there's an alert function wrapper in the codebase
fd 'alert' --extension ts --extension tsx | grep -E '(index|alert)' | head -20Repository: superset-sh/superset
Length of output: 377
🏁 Script executed:
# Look for how alert is imported and used in the useDirtyTabCloseGuard file
cat apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.tsRepository: superset-sh/superset
Length of output: 2329
The Promise hangs if the dialog is dismissed without an action click.
The alert() function does not return a Promise and provides no callback mechanism for dismissal. If a user presses Escape or clicks outside the dialog (standard modal behavior), onOpenChange fires and calls handleClose(), which only sets the dialog closed state. The Promise returned by useDirtyTabCloseGuard will never resolve, causing the tab close operation to hang indefinitely.
The alert API needs either:
- A returned Promise that resolves with a default value (e.g.,
false) when dismissed without action, or - An
onDismisscallback to defensively resolve the Promise, or - A non-dismissible dialog (remove backdrop/escape dismissal)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.ts
around lines 32 - 72, The Promise returned by useDirtyTabCloseGuard can hang
because alert(...) provides no callback when the dialog is dismissed; update the
alert usage (or wrapping logic inside useDirtyTabCloseGuard) to ensure the
Promise always resolves on dialog dismissal by wiring the dialog's
onOpenChange/onDismiss to call the resolver with a default (e.g.,
resolve(false)). Specifically, when creating the alert/modal in
useDirtyTabCloseGuard, capture the resolve function in scope and add an
onOpenChange/onDismiss handler that calls resolve(false) if the dialog is closed
without an action, or alternatively make the dialog non-dismissible (disable
backdrop/escape) so only the explicit Save All / Don't Save / Cancel actions
resolve the Promise; reference the existing alert(...) call, the resolve
variable in the Promise, and the Save All / Don't Save / Cancel action handlers
when implementing the change.
Comment on lines
+105
to
+118
| if (openInNewTab) { | ||
| state.addTab({ | ||
| panes: [ | ||
| { | ||
| kind: "file", | ||
| data: { | ||
| filePath: absoluteFilePath, | ||
| mode: "editor", | ||
| } as FilePaneData, | ||
| }, | ||
| ], | ||
| }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Race when worktreePath hasn't loaded yet.
If openFilePane is invoked before workspaceQuery resolves (worktreePath === ""), the input filePath is used verbatim as absoluteFilePath and passed to state.addTab/state.openPane. For relative inputs, this stores a non-absolute path in pane data, which downstream consumers expect to be absolute. In practice this is unlikely (the workspace query usually resolves before user interaction), and matches the prior behavior, but worth a guard if we want to be defensive — e.g., ignore the call (or queue it) until worktreePath is available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts
around lines 105 - 118, openFilePane can race when worktreePath is not yet
loaded causing non-absolute file paths to be stored; in the openFilePane
function check the workspaceQuery/worktreePath (the worktreePath string) before
constructing absoluteFilePath and calling state.addTab or state.openPane, and if
worktreePath is empty either return early (no-op) or enqueue the request until
worktreePath resolves so only absolute paths are ever passed into
state.addTab/state.openPane (references: openFilePane, absoluteFilePath,
state.addTab, state.openPane, workspaceQuery/worktreePath).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Kitenite
force-pushed
the
refactor-v2-workspace-page
branch
from
e8ece73 to
cc6a33d
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.ts (1)
28-38: Optional: tighten typing by droppingascasts.The cast pattern
data: { ... } as DiffPaneData(and similar forTerminalPaneData/ChatPaneData/BrowserPaneData/CommentPaneData/PaneViewerData) silences potential structural mismatches. Hoisting each literal into a typed local would catch missing/typo'd fields at compile time, e.g.:♻️ Example for the diff-new-tab branch
- if (openInNewTab) { - state.addTab({ - panes: [ - { - kind: "diff", - data: { - path: filePath, - collapsedFiles: [], - } as DiffPaneData, - }, - ], - }); - return; - } + if (openInNewTab) { + const data: DiffPaneData = { path: filePath, collapsedFiles: [] }; + state.addTab({ panes: [{ kind: "diff", data }] }); + return; + }The same shape applies to the terminal/chat/browser/comment factories.
Also applies to: 70-80, 83-91, 94-104, 107-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.ts around lines 28 - 38, The code is using inline "as" casts for pane data which hides structural mismatches; in useWorkspacePaneOpeners where state.addTab is called (e.g., the diff/new-tab and terminal/chat/browser/comment factories), create a properly typed local variable for each pane payload (e.g., const diffData: DiffPaneData = { path: filePath, collapsedFiles: [] }) and pass that variable into panes instead of using "data: { ... } as DiffPaneData"; do the same for TerminalPaneData, ChatPaneData, BrowserPaneData, CommentPaneData and PaneViewerData so the compiler will validate fields and catch missing/typo'd properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.ts:
- Around line 28-38: The code is using inline "as" casts for pane data which
hides structural mismatches; in useWorkspacePaneOpeners where state.addTab is
called (e.g., the diff/new-tab and terminal/chat/browser/comment factories),
create a properly typed local variable for each pane payload (e.g., const
diffData: DiffPaneData = { path: filePath, collapsedFiles: [] }) and pass that
variable into panes instead of using "data: { ... } as DiffPaneData"; do the
same for TerminalPaneData, ChatPaneData, BrowserPaneData, CommentPaneData and
PaneViewerData so the compiler will validate fields and catch missing/typo'd
properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82fbf905-ad11-451e-bea9-9fefd6650fb6
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/useClearActivePaneAttention.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/useDefaultPaneActions.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
✅ Files skipped from review due to trivial changes (5)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultPaneActions/useDefaultPaneActions.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
This was referenced Apr 26, 2026
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 26, 2026
3 PR (#442, #443, #444) で取り込み済みの 9 commits を git 履歴上もマージ済みに記録する。 内容差分は無し (merge -s ours)。 取り込み内容: - 6b96acd Improve sidebar group management UX (superset-sh#3745) - a4079e7 Update dashboard sidebar workspace icons (superset-sh#3755) - d3753d0 [codex] Use dynamic footer copyright years (superset-sh#3754) - ce606be Handle browser passthrough during v2 resize (superset-sh#3744) - b1e1eb7 Refactor v2 workspace page (superset-sh#3747) - 8693869 [codex] move v2 toggle to experimental settings (superset-sh#3748) - ef3f381 Revert "fix(desktop): refit v2 terminal after font settle (superset-sh#3742)" (superset-sh#3750) - 手動移植 (vibrancy patch 維持) - 25b2d52 Show terminal sessions from all workspaces in dropdown (superset-sh#3751) - 62737db fix v1 terminal resize repaint (superset-sh#3756)
AviPeltz
added a commit
that referenced
this pull request
Apr 27, 2026
Restores main's page.tsx (PR #3747 refactor) — the wip's inline openFilePane stored unconverted paths and tripped workspace-fs's ensureWithinRoot for callers that didn't pre-normalize. Re-adds the pendingReveal prop and widens onSelectDiffFile on WorkspaceSidebar so main's page.tsx props line up. PRActionHeader now mirrors v1's PRButton: when a PR exists and is open, the right side renders a link + chevron dropdown with squash / merge / rebase, wired to workspaceTrpc.github.mergePR. After a successful merge, triggers pullRequests.refreshByWorkspaces and refetches the local queries so the UI reflects merged state immediately instead of waiting for the 30s background sync. Closed/merged/draft PRs render the link without the dropdown. Gating: renamed PR_ACTION_HEADER_ENABLED → CREATE_PR_BUTTON_ENABLED to reflect that only the create-PR dispatch is gated (chat doesn't exist in v2 yet). The status group + merge dropdown always render. Also: refresh button on the Changes tab that invalidates getStatus, getDiff, listCommits, listBranches, and getBaseBranch in parallel and spins while in flight. getPullRequest now returns repoOwner/repoName (needed for the merge mutation).
AviPeltz
added a commit
that referenced
this pull request
Apr 27, 2026
* wip * chore(desktop): integrate PR action header behind a flag Wires the cherry-picked wip into v2 alongside the recent main refactor: restores `v2UserPreferencesSchema` + `pendingWorkspaceSchema` fields the wip dropped, points the sidebar-open read at user prefs (where the toggle hotkey writes), adds `focusRequestId` route param + `getDefaultBranchName` import the wip referenced without declaring, and drops the removed `changesSubtab` write. PR action header rendering is gated behind a `PR_ACTION_HEADER_ENABLED = false` constant — empty bar still renders so sidebar layout stays consistent. Flip to true once chat lands in v2 and the slash-command flow is verified end-to-end. * feat(desktop): v1-style PR status + restore main's v2 workspace page Restores main's page.tsx (PR #3747 refactor) — the wip's inline openFilePane stored unconverted paths and tripped workspace-fs's ensureWithinRoot for callers that didn't pre-normalize. Re-adds the pendingReveal prop and widens onSelectDiffFile on WorkspaceSidebar so main's page.tsx props line up. PRActionHeader now mirrors v1's PRButton: when a PR exists and is open, the right side renders a link + chevron dropdown with squash / merge / rebase, wired to workspaceTrpc.github.mergePR. After a successful merge, triggers pullRequests.refreshByWorkspaces and refetches the local queries so the UI reflects merged state immediately instead of waiting for the 30s background sync. Closed/merged/draft PRs render the link without the dropdown. Gating: renamed PR_ACTION_HEADER_ENABLED → CREATE_PR_BUTTON_ENABLED to reflect that only the create-PR dispatch is gated (chat doesn't exist in v2 yet). The status group + merge dropdown always render. Also: refresh button on the Changes tab that invalidates getStatus, getDiff, listCommits, listBranches, and getBaseBranch in parallel and spins while in flight. getPullRequest now returns repoOwner/repoName (needed for the merge mutation). * feat(desktop): render v2 sidebar full-height via portal Restores the full-height v2 sidebar layout that was on the wip: the sidebar now portals into the workspace-right-sidebar-slot div in layout.tsx (next to TopBar) instead of rendering inside the page below TopBar via ResizablePanelGroup. Width is persisted on v2UserPreferences.rightSidebarWidth (new field, default 340) with a setter on useV2UserPreferences. The renderer's explicit-width ResizablePanel handles drag-resize and double-click reset. Resize dragging is forwarded to useBrowserShellInteractionPassthrough so the browser pane keeps ignoring clicks during resize. The workspace area no longer needs the resizable panel group since it's the only thing in the page now — it expands to fill flex-1, and the sidebar floats next to it via portal. Also drops the unused commitCount prop on ChangesHeader (caught by lint after the wip's refactor stopped using it). * feat(desktop): icon-only PR action header + sidebar overflow guards PRActionHeader now mirrors v1's PRButton state machine using just icons — dropped the badge text on the left ("Open"/"Not published"/"Ready" etc.). State is conveyed by the icon itself: - loading → empty (was: text-only) - unavailable → muted PR icon with tooltip explaining why - no-pr → clickable PR icon (gated → muted icon, tooltip "Create PR coming soon") - pr-exists open → link with PR-state icon + chevron merge dropdown - pr-exists closed/ merged/draft → link with PR-state icon (no merge dropdown) - busy → link + spinner - error → muted icon, click to retry selectActionButton now returns the raw UnavailableReason so the component renders the tooltip (decouples copy from the selector). Also fixes the sidebar blowing past its width on the Review tab: ReviewTabContent only had overflow-y-auto, so wide content escaped horizontally. Added min-w-0 + overflow-x-hidden to ReviewTabContent and the WorkspaceSidebar tab content wrapper as a defense-in-depth guard for any future tab. * feat(desktop): CI/review status dots + hover detail card on PR badge PR badge now carries two ambient indicators next to the number — a CI rollup dot (success/failure/pending, circle icons) and a review decision dot (approved/changes-requested/pending, user icons). Distinct icon families so they read as "build" vs "people" at 12px. Suppressed for closed/merged PRs (post-state CI is noise) and for PRs with no checks or no review requested (quiet for trivial PRs). Hovering the link now opens a 320px card with the title (line-clamped), state pill, branch, full check rollup ("8 of 10 passing"), review status, "Updated 2h ago", and a "View on GitHub" link. Pulled in via @superset/ui/hover-card so animations + portal placement come for free. Extracted computeChecksRollup + coerceCheckStatus to a shared util so useReviewTab and the new dots draw from the same source — keeps the sidebar tab and the badge from drifting on edge cases. Co-located the new pieces under PRActionHeader/components/PRStatusGroup following the project's per-component-folder convention. * polish(desktop): state-tinted PR badge, drop review indicators, files first PR badge now wears the PR state color: open is emerald-tinted (border + bg + divider), merged violet, closed rose, draft muted. Tinting the whole bordered group makes state legible at a glance without the icon having to carry it alone. Dropped the review-decision dot and the "Review" line in the hover detail card — review noise was redundant with the Review tab's own indicator and made the badge feel busy. Only the CI checks dot remains next to the PR number. Sidebar tab order is now Files → Changes → Review (Files first). * fix(desktop): address PR review feedback Bot reviews flagged a mix of stale concerns (already addressed in earlier commits) and live issues. Fixing the live ones: - page.tsx: restore min-w-[320px] on the workspace area so dragging the sidebar wider can't collapse the editor pane to unusable width (lost when we portaled the sidebar out of the resizable panel group) - page.tsx: initialize sidebarSlotEl synchronously via useState init so users with rightSidebarOpen=true persisted don't see a 1-frame flash on mount; the slot is mounted by the parent layout before this child renders, so a synchronous lookup is safe - PRStatusGroup: catch refresh failures after a successful merge — previously the rejection escaped the async onSuccess silently and left the UI stale. Now logs and shows a soft toast - useChangesTab: catch refresh failures from the Changes-tab refresh button — previously void-discarded - git.ts getBranchSyncStatus: log when git.status() fails instead of silently misreporting a dirty repo as clean. Defaults to false on failure with a comment explaining why over-reporting on every transient blip would be more annoying than briefly under-reporting Cleanup: - Drop dead _unavailableTooltip helper in getPRFlowState (byte-for-byte copy of the one in PRActionHeader, orphaned during cherry-pick) - Drop dead rightSidebarOpen + rightSidebarWidth from workspaceLocalStateSchema — both moved to v2UserPreferences and the per-workspace fields are written by nobody and read by nobody Dead-code-but-harmless-to-fix-now (will matter once create-PR ships): - Replace deprecated unescape() with TextEncoder for UTF-8 base64 in usePRFlowDispatch.encodeAsDataUrl - buildPRContext: stop emitting literal `<default>` when defaultBranch is null; instruct the agent to resolve via `gh repo view` instead - create-pr.md: quote refs in `git push -u origin -- "<branch>"` and the `<base>..HEAD` ranges to prevent shell injection on crafted names
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
page.tsx.Regression Review
origin/main: no behavior changes found.Validation
bunx biome check --writeon touched filesbunx biome checkon touched filesbun --filter @superset/desktop typecheckbun --filter @superset/desktop test— 1821 pass, 0 failSummary by CodeRabbit
New Features
Refactor