feat(desktop): file tree sidebar, file pane renderers, and alert refactor#3122
Merged
Conversation
- Add RightSidebar with file tree, toggled via Cmd+L, state persisted in workspace local collection - Create FilePane with CodeRenderer (Monaco), MarkdownRenderer (TipTap), and ImageRenderer — replaces WorkspaceFilePreview - File-type icons, italic title when unpinned, dirty indicator dot - onBeforeClose on PaneDefinition — registry-level close guards with Save/Don't Save/Cancel dialog - onHeaderClick on PaneDefinition — click header to pin, middle-click to close - selectedFilePath derived from store, sidebar highlights active file - Click file in sidebar: pin if already active, else open unpinned - Cmd+O wired up in OpenInMenuButton for both old and v2 workspace - setPanePinned: removed unnecessary tabId param - useFileTree: added reveal() method, fixed stale cache (staleTime: 0) - Alert component: refactored from onConfirm/onCancel to actions array API, migrated all 8 call sites
Contributor
📝 WalkthroughWalkthroughRefactors alert API to an actions-based model, adds FilePane and file renderers (Code/Image/Markdown) with external-change handling, implements a right sidebar with file-tree reveal and scrolling, removes legacy WorkspaceFilePreview, adjusts pane APIs (tab-agnostic setPanePinned, onBeforeClose/onHeaderClick), and adds related hotkeys and schema fields. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RightSidebar
participant FileTree
participant Workspace (PaneRegistry)
participant DOM
User->>RightSidebar: Click file item
RightSidebar->>FileTree: request reveal(selectedFilePath)
FileTree->>FileTree: expand ancestor dirs sequentially
FileTree-->>RightSidebar: resolved
RightSidebar->>DOM: querySelector(`[data-filepath="..."]`)
DOM-->>RightSidebar: element found
RightSidebar->>DOM: scrollIntoView(element)
RightSidebar->>Workspace: open/pin file pane (selectedFilePath)
Workspace-->>User: Pane with file renders (FilePane -> appropriate renderer)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 44 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/workspace-client/src/hooks/useFileTree/useFileTree.ts">
<violation number="1" location="packages/workspace-client/src/hooks/useFileTree/useFileTree.ts:485">
P2: `reveal` uses a prefix-only path check; this can expand/load directories outside the workspace when paths share the same prefix.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx:24">
P2: Use a cross-platform path split for the filename instead of `split("/")`.
(Based on your team's feedback about using cross-platform path utilities instead of split.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx">
<violation number="1" location="packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx:86">
P1: Handle `onBeforeClose` rejections with `try/catch` to avoid unhandled promise rejections from pane close handlers.
(Based on your team's feedback about handling async rejections in UI flows with explicit try/catch and logging.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx:28">
P1: The content-sync condition is impossible (`!onDirtyChange` is always false), so `savedContent` never updates on new `content`, causing stale dirty-state tracking.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:54">
P2: Avoid manual `split("/")` path parsing here; use a cross-platform basename helper so Windows paths are handled correctly.
(Based on your team's feedback about using cross-platform path utilities instead of split.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:63">
P1: The new “Save” action closes the pane without saving, which can drop unsaved edits. Wire this action to an actual save operation before resolving close.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx:12">
P2: Handle `onReload()` rejections in the click handler instead of discarding the promise.
(Based on your team's feedback about awaiting/catching async calls to avoid unhandled promise rejections.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx:300">
P2: Use a cross-platform basename extraction instead of splitting only on `/`.
(Based on your team's feedback about using cross-platform path utilities instead of split.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx:317">
P1: The “Save All” action currently closes tabs without saving. Do not resolve close-success until save is actually implemented.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+86
to
+87
| const allowed = await definition.onBeforeClose(pane); | ||
| if (!allowed) return; |
Contributor
There was a problem hiding this comment.
P1: Handle onBeforeClose rejections with try/catch to avoid unhandled promise rejections from pane close handlers.
(Based on your team's feedback about handling async rejections in UI flows with explicit try/catch and logging.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx, line 86:
<comment>Handle `onBeforeClose` rejections with `try/catch` to avoid unhandled promise rejections from pane close handlers.
(Based on your team's feedback about handling async rejections in UI flows with explicit try/catch and logging.) </comment>
<file context>
@@ -81,8 +81,13 @@ export function Pane<TData>({
- store.getState().closePane({ tabId: tab.id, paneId: pane.id }),
+ close: async () => {
+ if (definition?.onBeforeClose) {
+ const allowed = await definition.onBeforeClose(pane);
+ if (!allowed) return;
+ }
</file context>
Suggested change
| const allowed = await definition.onBeforeClose(pane); | |
| if (!allowed) return; | |
| try { | |
| const allowed = await definition.onBeforeClose(pane); | |
| if (!allowed) return; | |
| } catch (err) { | |
| console.warn("[Pane] onBeforeClose failed", err); | |
| return; | |
| } |
| const [savedContent, setSavedContent] = useState(content); | ||
|
|
||
| // Track the initial/saved content to detect dirty state | ||
| if (content !== savedContent && !onDirtyChange) { |
Contributor
There was a problem hiding this comment.
P1: The content-sync condition is impossible (!onDirtyChange is always false), so savedContent never updates on new content, causing stale dirty-state tracking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx, line 28:
<comment>The content-sync condition is impossible (`!onDirtyChange` is always false), so `savedContent` never updates on new `content`, causing stale dirty-state tracking.</comment>
<file context>
@@ -0,0 +1,59 @@
+ const [savedContent, setSavedContent] = useState(content);
+
+ // Track the initial/saved content to detect dirty state
+ if (content !== savedContent && !onDirtyChange) {
+ setSavedContent(content);
+ }
</file context>
| { | ||
| label: "Save", | ||
| onClick: () => { | ||
| // TODO: wire up save via editor ref |
Contributor
There was a problem hiding this comment.
P1: The new “Save” action closes the pane without saving, which can drop unsaved edits. Wire this action to an actual save operation before resolving close.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx, line 63:
<comment>The new “Save” action closes the pane without saving, which can drop unsaved edits. Wire this action to an actual save operation before resolving close.</comment>
<file context>
@@ -22,20 +24,60 @@ export function usePaneRegistry(
+ {
+ label: "Save",
+ onClick: () => {
+ // TODO: wire up save via editor ref
+ resolve(true);
+ },
</file context>
| label: "Save All", | ||
| onClick: () => { | ||
| // TODO: wire up save via editor refs | ||
| resolve(true); |
Contributor
There was a problem hiding this comment.
P1: The “Save All” action currently closes tabs without saving. Do not resolve close-success until save is actually implemented.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx, line 317:
<comment>The “Save All” action currently closes tabs without saving. Do not resolve close-success until save is actually implemented.</comment>
<file context>
@@ -216,43 +244,110 @@ function WorkspaceContent({
+ label: "Save All",
+ onClick: () => {
+ // TODO: wire up save via editor refs
+ resolve(true);
+ },
+ },
</file context>
|
|
||
| const reveal = useCallback( | ||
| async (absolutePath: string): Promise<void> => { | ||
| if (!rootPath || !absolutePath.startsWith(rootPath)) return; |
Contributor
There was a problem hiding this comment.
P2: reveal uses a prefix-only path check; this can expand/load directories outside the workspace when paths share the same prefix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/workspace-client/src/hooks/useFileTree/useFileTree.ts, line 485:
<comment>`reveal` uses a prefix-only path check; this can expand/load directories outside the workspace when paths share the same prefix.</comment>
<file context>
@@ -479,6 +480,27 @@ export function useFileTree({
+ const reveal = useCallback(
+ async (absolutePath: string): Promise<void> => {
+ if (!rootPath || !absolutePath.startsWith(rootPath)) return;
+
+ // Collect ancestor directories from rootPath down to the parent of the target
</file context>
Suggested change
| if (!rootPath || !absolutePath.startsWith(rootPath)) return; | |
| if (!rootPath || !isWithinPath(rootPath, absolutePath)) return; |
| <div className="flex h-full items-center justify-center overflow-auto bg-background p-4"> | ||
| <img | ||
| src={dataUrl} | ||
| alt={filePath.split("/").pop() ?? ""} |
Contributor
There was a problem hiding this comment.
P2: Use a cross-platform path split for the filename instead of split("/").
(Based on your team's feedback about using cross-platform path utilities instead of split.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx, line 24:
<comment>Use a cross-platform path split for the filename instead of `split("/")`.
(Based on your team's feedback about using cross-platform path utilities instead of split.) </comment>
<file context>
@@ -0,0 +1,30 @@
+ <div className="flex h-full items-center justify-center overflow-auto bg-background p-4">
+ <img
+ src={dataUrl}
+ alt={filePath.split("/").pop() ?? ""}
+ className="max-h-full max-w-full object-contain"
+ draggable={false}
</file context>
| onBeforeClose: (pane) => { | ||
| const data = pane.data as FilePaneData; | ||
| if (!data.hasChanges) return true; | ||
| const name = data.filePath.split("/").pop(); |
Contributor
There was a problem hiding this comment.
P2: Avoid manual split("/") path parsing here; use a cross-platform basename helper so Windows paths are handled correctly.
(Based on your team's feedback about using cross-platform path utilities instead of split.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx, line 54:
<comment>Avoid manual `split("/")` path parsing here; use a cross-platform basename helper so Windows paths are handled correctly.
(Based on your team's feedback about using cross-platform path utilities instead of split.) </comment>
<file context>
@@ -22,20 +24,60 @@ export function usePaneRegistry(
+ onBeforeClose: (pane) => {
+ const data = pane.data as FilePaneData;
+ if (!data.hasChanges) return true;
+ const name = data.filePath.split("/").pop();
+ return new Promise<boolean>((resolve) => {
+ alert({
</file context>
Suggested change
| const name = data.filePath.split("/").pop(); | |
| const name = getFileName(data.filePath); |
| <button | ||
| type="button" | ||
| className="underline hover:no-underline" | ||
| onClick={() => void onReload()} |
Contributor
There was a problem hiding this comment.
P2: Handle onReload() rejections in the click handler instead of discarding the promise.
(Based on your team's feedback about awaiting/catching async calls to avoid unhandled promise rejections.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx, line 12:
<comment>Handle `onReload()` rejections in the click handler instead of discarding the promise.
(Based on your team's feedback about awaiting/catching async calls to avoid unhandled promise rejections.) </comment>
<file context>
@@ -0,0 +1,18 @@
+ <button
+ type="button"
+ className="underline hover:no-underline"
+ onClick={() => void onReload()}
+ >
+ Reload
</file context>
| p.kind === "file" && (p.data as FilePaneData).hasChanges, | ||
| ) | ||
| .map((p) => | ||
| (p.data as FilePaneData).filePath.split("/").pop(), |
Contributor
There was a problem hiding this comment.
P2: Use a cross-platform basename extraction instead of splitting only on /.
(Based on your team's feedback about using cross-platform path utilities instead of split.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx, line 300:
<comment>Use a cross-platform basename extraction instead of splitting only on `/`.
(Based on your team's feedback about using cross-platform path utilities instead of split.) </comment>
<file context>
@@ -216,43 +244,110 @@ function WorkspaceContent({
+ p.kind === "file" && (p.data as FilePaneData).hasChanges,
+ )
+ .map((p) =>
+ (p.data as FilePaneData).filePath.split("/").pop(),
+ );
+ if (dirtyFiles.length === 0) return true;
</file context>
Suggested change
| (p.data as FilePaneData).filePath.split("/").pop(), | |
| (p.data as FilePaneData).filePath.split(/[\\/]/).pop(), |
![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: 12
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx (1)
12-16: Consider performance for large images.The current base64 conversion iterates through each byte and builds intermediate strings, which can be slow and memory-intensive for large images. A more efficient approach uses
Uint8Arraydirectly withTextDecoderor chunked processing.♻️ More efficient base64 encoding
- const base64 = btoa( - Array.from(content) - .map((b) => String.fromCharCode(b)) - .join(""), - ); + // Process in chunks to avoid call stack limits and reduce memory pressure + let binary = ""; + const chunkSize = 8192; + for (let i = 0; i < content.length; i += chunkSize) { + binary += String.fromCharCode(...content.subarray(i, i + chunkSize)); + } + const base64 = btoa(binary);🤖 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/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx around lines 12 - 16, The current byte-to-base64 conversion in ImageRenderer (the base64 variable) is inefficient for large images because it maps every byte to a char and joins strings; replace it with a streaming/Blob-based approach: create a Blob from the ArrayBuffer/Uint8Array (the content), then use FileReader.readAsDataURL or URL.createObjectURL to get an efficient data URL/object URL for the image; update the code in the ImageRenderer where base64 is built to use the Blob + FileReader/URL.createObjectURL approach and remove the Array.from(...).map(...).join("") path to avoid O(n) string concatenation overhead and memory spikes.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsx (1)
39-56: Alert migration is correct, but consider consolidating duplicate components.The alert API migration is implemented correctly, matching the pattern used elsewhere. However, this component appears to be nearly identical to
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsx.Consider extracting the shared
SessionSelectorIteminto a common location to avoid maintaining duplicate code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsx` around lines 39 - 56, This component duplicates another SessionSelectorItem; extract the shared SessionSelectorItem (including props like sessionId and onDeleteSession and the alert/toast delete flow) into a single common module (e.g., a shared components directory), export its prop types, update both implementations to import the shared component, and ensure the alert call and toast.promise logic remain identical; also reconcile any small prop or handler differences between the two files so both callers pass the same props (sessionId, onDeleteSession, display props) to the new shared SessionSelectorItem.
🤖 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/components/RightSidebar/RightSidebar.tsx:
- Around line 65-84: prevSelectedRef is seeded with selectedFilePath which
prevents the initial selection from being revealed; change prevSelectedRef to
start empty (e.g., useRef<string | null>(null)) and keep the existing useEffect
comparison so that when selectedFilePath exists and prevSelectedRef.current is
null/different you call fileTree.reveal(selectedFilePath) and scroll into view
via scrollContainerRef; after the reveal set prevSelectedRef.current =
selectedFilePath so subsequent changes behave as before (references:
prevSelectedRef, selectedFilePath, useEffect, scrollContainerRef,
fileTree.reveal).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx:
- Around line 40-47: handleSave currently resolves even when document.save()
returns a non-"saved" status, which violates the onSave contract and causes
callers to treat failures as success; update handleSave (the async callback that
calls document.save) so that it only calls handleDirtyChange(false) and returns
successfully when result.status === "saved", and otherwise rejects (throw an
Error or return Promise.reject(result)) so the onSave promise is rejected on
non-saved results.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx:
- Around line 24-42: The component fails to resync its baseline when the content
prop changes; add a useEffect that runs when content changes and sets
currentContentRef.current = content, calls setSavedContent(content), and invokes
onDirtyChange(false) to reset dirty state; update references to
currentContentRef and savedContent in the effect so the editor/pane reused for a
different file or a clean reload correctly reflects the new baseline. Ensure you
reference currentContentRef, setSavedContent, and onDirtyChange in the new
effect and include content in its dependency array.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx:
- Around line 26-40: The markdown dirty-state isn't resynced when the incoming
content prop changes; update currentContentRef and savedContent whenever content
changes and ensure onDirtyChange is called to reflect the new comparison. Add a
useEffect that runs on content (and optionally onDirtyChange) which sets
currentContentRef.current = content, setSavedContent(content), and calls
onDirtyChange(false) (or onDirtyChange(content !== savedContent) if you prefer
to preserve earlier semantics) so dirty tracking resets when the underlying
document is reloaded or the pane is reused; keep handleChange and handleSave
logic unchanged otherwise.
- Line 25: The MarkdownRenderer component currently initializes internal state
const [viewMode, _setViewMode] = useState<MarkdownViewMode>("rendered") but
never exposes or uses the setter, making the "raw" branch unreachable; update
MarkdownRenderer to accept a controlled prop (e.g., viewMode: MarkdownViewMode
and onViewModeChange: (m: MarkdownViewMode) => void) or at minimum use an
internal setter (rename _setViewMode to setViewMode) and wire the UI controls
that toggle modes to call the setter; ensure the component reads the external
prop if provided (falling back to internal state) so the "raw" rendering branch
(references to viewMode in render logic) becomes reachable and update any
related handlers/controls in this file to invoke the new onViewModeChange or
setViewMode.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 61-65: The "Save" button's onClick currently immediately calls
resolve(true) (closing the pane) despite the TODO; change the handler in
usePaneRegistry's Save branch to perform the actual save via the editor ref or
save function (e.g., call the editorRef.save() or the workspace save method),
await its completion, and only call resolve(true) if the save succeeded; if the
save fails, surface an error to the user and do not resolve (keeping the pane
open). Ensure the onClick is async and references the existing editor ref or
save function instead of unconditionally calling resolve(true).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 313-318: The "Save All" primary action currently just calls
resolve(true) in the onClick handler (the anonymous onClick that contains
resolve(true)) and therefore closes the dialog discarding unsaved edits; change
this handler to invoke the actual save flow (e.g., call a
saveDirtyFilePanes(tab) or the existing editor ref save functions) and only call
resolve(true) after the save completes successfully, or alternatively
disable/remove the "Save All" action until editor refs are wired; specifically
update the onClick for the "Save All" action (the handler that currently
contains resolve(true)) to perform saveDirtyFilePanes or the real save method
and handle success/failure before resolving.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx`:
- Around line 96-103: The onClick handler for the "Revoke" action calls
authClient.apiKey.delete({ keyId: id }) without error handling; wrap that async
call in a try/catch similar to handleGenerateKey, calling toast.success on
success and toast.error with a helpful message (and optionally logging the
error) on failure to avoid unhandled rejections and provide user feedback;
update the anonymous onClick async function in the ApiKeysSettings component to
perform the try/catch around authClient.apiKey.delete and use toast.error on
catch.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx`:
- Around line 64-70: The header click handlers (onClick and onAuxClick) are
firing for clicks that bubble up from nested controls (toolbar/actionsContent),
so update the handlers in PaneHeader.tsx to ignore bubbled events by checking
event.target !== event.currentTarget (or event.currentTarget.contains check) and
returning early; apply the same guard in the middle-click onAuxClick branch
before calling e.preventDefault() and onMiddleClick so clicks on nested buttons
won’t trigger header actions.
In `@packages/ui/src/atoms/Alert/Alert.tsx`:
- Around line 56-68: The dialog's passive close currently only hides the UI
(handleClose -> setIsOpen(false)) leaving any caller waiting on an alert
unresolved; update the onOpenChange/handleClose path so that when open becomes
false it also triggers the alert's caller-visible outcome: call a provided
dismissal/resolution hook on alertOptions (e.g., alertOptions.onDismiss or
alertOptions.resolve/reject) or, if none exists, invoke a sensible fallback such
as the "cancel" action callback from alertOptions.actions (use the same action
callback code path used when clicking buttons) so callers waiting on the alert
get resolved/rejected when the user passively closes the Dialog.
In `@packages/workspace-client/src/hooks/useFileTree/useFileTree.ts`:
- Around line 496-499: reveal() can resolve before an ancestor's contents are
actually populated because await expand(dir) returns immediately if
loadDirectory() sees the directory in loadingDirectories; fix by ensuring each
ancestor is fully loaded into rootEntries before proceeding: after calling
expand(dir) (or inside expand), wait for a directory-loaded signal (e.g. a new
helper like waitForDirectoryLoaded(dir) that resolves when rootEntries contains
the node or when loadingDirectories no longer contains dir), and use that helper
for each ancestor in the for loop so reveal() only continues once the node is
present in rootEntries; reference expand(), loadDirectory(), loadingDirectories,
rootEntries and reveal() when adding this wait logic.
- Around line 483-485: The reveal function's boundary check uses
absolutePath.startsWith(rootPath) which incorrectly matches sibling paths;
replace that check with the segment-aware helper isWithinPath(rootPath,
absolutePath) inside the async reveal(absolutePath: string) callback so the
function only proceeds when the path is truly inside the mounted tree (update
the conditional in reveal to call isWithinPath and return early if it returns
false).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx:
- Around line 12-16: The current byte-to-base64 conversion in ImageRenderer (the
base64 variable) is inefficient for large images because it maps every byte to a
char and joins strings; replace it with a streaming/Blob-based approach: create
a Blob from the ArrayBuffer/Uint8Array (the content), then use
FileReader.readAsDataURL or URL.createObjectURL to get an efficient data
URL/object URL for the image; update the code in the ImageRenderer where base64
is built to use the Blob + FileReader/URL.createObjectURL approach and remove
the Array.from(...).map(...).join("") path to avoid O(n) string concatenation
overhead and memory spikes.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsx`:
- Around line 39-56: This component duplicates another SessionSelectorItem;
extract the shared SessionSelectorItem (including props like sessionId and
onDeleteSession and the alert/toast delete flow) into a single common module
(e.g., a shared components directory), export its prop types, update both
implementations to import the shared component, and ensure the alert call and
toast.promise logic remain identical; also reconcile any small prop or handler
differences between the two files so both callers pass the same props
(sessionId, onDeleteSession, display props) to the new shared
SessionSelectorItem.
🪄 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: b4092ee5-6e1b-413d-a168-d6103d492e7f
📒 Files selected for processing (44)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/WorkspaceFilesSearchResultItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/WorkspaceFilesToolbar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/useWorkspaceFileSearch.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/WorkspaceFilePreview.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/AddSecretSheet/AddSecretSheet.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsxpackages/panes/src/core/store/store.test.tspackages/panes/src/core/store/store.tspackages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsxpackages/panes/src/react/types.tspackages/ui/src/atoms/Alert/Alert.tsxpackages/ui/src/atoms/Alert/index.tspackages/workspace-client/src/hooks/useFileTree/useFileTree.ts
💤 Files with no reviewable changes (6)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/index.ts
- packages/panes/src/core/store/store.test.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/WorkspaceFilePreview.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/index.ts
Comment on lines
+65
to
+84
| const scrollContainerRef = useRef<HTMLDivElement>(null); | ||
| const prevSelectedRef = useRef(selectedFilePath); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| selectedFilePath && | ||
| selectedFilePath !== prevSelectedRef.current && | ||
| rootPath | ||
| ) { | ||
| void fileTree.reveal(selectedFilePath).then(() => { | ||
| requestAnimationFrame(() => { | ||
| const el = scrollContainerRef.current?.querySelector( | ||
| `[data-filepath="${CSS.escape(selectedFilePath)}"]`, | ||
| ); | ||
| el?.scrollIntoView({ block: "nearest" }); | ||
| }); | ||
| }); | ||
| } | ||
| prevSelectedRef.current = selectedFilePath; | ||
| }, [selectedFilePath, rootPath, fileTree]); |
Contributor
There was a problem hiding this comment.
Reveal the initial selection too.
Because prevSelectedRef is seeded from selectedFilePath and updated on every effect run, the first selected file is treated as already handled. When the sidebar mounts with an active pane, the tree never expands/scrolls to that file until the selection changes again.
🩹 Suggested fix
- const prevSelectedRef = useRef(selectedFilePath);
+ const prevSelectedRef = useRef<string | undefined>(undefined);
useEffect(() => {
- if (
- selectedFilePath &&
- selectedFilePath !== prevSelectedRef.current &&
- rootPath
- ) {
- void fileTree.reveal(selectedFilePath).then(() => {
- requestAnimationFrame(() => {
- const el = scrollContainerRef.current?.querySelector(
- `[data-filepath="${CSS.escape(selectedFilePath)}"]`,
- );
- el?.scrollIntoView({ block: "nearest" });
- });
- });
- }
- prevSelectedRef.current = selectedFilePath;
+ if (
+ !selectedFilePath ||
+ !rootPath ||
+ selectedFilePath === prevSelectedRef.current
+ ) {
+ return;
+ }
+
+ void fileTree.reveal(selectedFilePath).then(() => {
+ requestAnimationFrame(() => {
+ const el = scrollContainerRef.current?.querySelector(
+ `[data-filepath="${CSS.escape(selectedFilePath)}"]`,
+ );
+ el?.scrollIntoView({ block: "nearest" });
+ });
+ prevSelectedRef.current = selectedFilePath;
+ });
}, [selectedFilePath, rootPath, fileTree]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const scrollContainerRef = useRef<HTMLDivElement>(null); | |
| const prevSelectedRef = useRef(selectedFilePath); | |
| useEffect(() => { | |
| if ( | |
| selectedFilePath && | |
| selectedFilePath !== prevSelectedRef.current && | |
| rootPath | |
| ) { | |
| void fileTree.reveal(selectedFilePath).then(() => { | |
| requestAnimationFrame(() => { | |
| const el = scrollContainerRef.current?.querySelector( | |
| `[data-filepath="${CSS.escape(selectedFilePath)}"]`, | |
| ); | |
| el?.scrollIntoView({ block: "nearest" }); | |
| }); | |
| }); | |
| } | |
| prevSelectedRef.current = selectedFilePath; | |
| }, [selectedFilePath, rootPath, fileTree]); | |
| const scrollContainerRef = useRef<HTMLDivElement>(null); | |
| const prevSelectedRef = useRef<string | undefined>(undefined); | |
| useEffect(() => { | |
| if ( | |
| !selectedFilePath || | |
| !rootPath || | |
| selectedFilePath === prevSelectedRef.current | |
| ) { | |
| return; | |
| } | |
| void fileTree.reveal(selectedFilePath).then(() => { | |
| requestAnimationFrame(() => { | |
| const el = scrollContainerRef.current?.querySelector( | |
| `[data-filepath="${CSS.escape(selectedFilePath)}"]`, | |
| ); | |
| el?.scrollIntoView({ block: "nearest" }); | |
| }); | |
| prevSelectedRef.current = selectedFilePath; | |
| }); | |
| }, [selectedFilePath, rootPath, fileTree]); |
🤖 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/components/RightSidebar/RightSidebar.tsx
around lines 65 - 84, prevSelectedRef is seeded with selectedFilePath which
prevents the initial selection from being revealed; change prevSelectedRef to
start empty (e.g., useRef<string | null>(null)) and keep the existing useEffect
comparison so that when selectedFilePath exists and prevSelectedRef.current is
null/different you call fileTree.reveal(selectedFilePath) and scroll into view
via scrollContainerRef; after the reveal set prevSelectedRef.current =
selectedFilePath so subsequent changes behave as before (references:
prevSelectedRef, selectedFilePath, useEffect, scrollContainerRef,
fileTree.reveal).
Comment on lines
+40
to
+47
| const handleSave = useCallback( | ||
| async (content: string) => { | ||
| const result = await document.save({ content }); | ||
| if (result.status === "saved") { | ||
| handleDirtyChange(false); | ||
| } | ||
| return result; | ||
| }, |
Contributor
There was a problem hiding this comment.
Fail the onSave contract when the document was not actually saved.
This callback resolves even when document.save() returns a non-saved status. Both renderers treat any fulfilled onSave as success and advance their local saved baseline, so soft-save failures can desync the editor’s dirty tracking from the pane state.
🤖 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/usePaneRegistry/components/FilePane/FilePane.tsx
around lines 40 - 47, handleSave currently resolves even when document.save()
returns a non-"saved" status, which violates the onSave contract and causes
callers to treat failures as success; update handleSave (the async callback that
calls document.save) so that it only calls handleDirtyChange(false) and returns
successfully when result.status === "saved", and otherwise rejects (throw an
Error or return Promise.reject(result)) so the onSave promise is rejected on
non-saved results.
Comment on lines
+24
to
+42
| const currentContentRef = useRef(content); | ||
| const [savedContent, setSavedContent] = useState(content); | ||
|
|
||
| // Track the initial/saved content to detect dirty state | ||
| if (content !== savedContent && !onDirtyChange) { | ||
| setSavedContent(content); | ||
| } | ||
|
|
||
| const handleChange = useCallback( | ||
| (value: string) => { | ||
| currentContentRef.current = value; | ||
| onDirtyChange(value !== savedContent); | ||
| }, | ||
| [onDirtyChange, savedContent], | ||
| ); | ||
|
|
||
| const handleSave = useCallback(async () => { | ||
| await onSave(currentContentRef.current); | ||
| setSavedContent(currentContentRef.current); |
Contributor
There was a problem hiding this comment.
Resync the editor baseline when content changes.
currentContentRef and savedContent survive prop changes, and the guard on Lines 28-30 never runs because onDirtyChange is required. If this pane is reused for another text file or the document reloads cleanly, dirty tracking and handleSave can still operate on the previous content.
🩹 Suggested fix
-import { useCallback, useRef, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
@@
- // Track the initial/saved content to detect dirty state
- if (content !== savedContent && !onDirtyChange) {
- setSavedContent(content);
- }
+ useEffect(() => {
+ currentContentRef.current = content;
+ setSavedContent(content);
+ onDirtyChange(false);
+ }, [content, onDirtyChange]);🤖 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/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx
around lines 24 - 42, The component fails to resync its baseline when the
content prop changes; add a useEffect that runs when content changes and sets
currentContentRef.current = content, calls setSavedContent(content), and invokes
onDirtyChange(false) to reset dirty state; update references to
currentContentRef and savedContent in the effect so the editor/pane reused for a
different file or a clean reload correctly reflects the new baseline. Ensure you
reference currentContentRef, setSavedContent, and onDirtyChange in the new
effect and include content in its dependency array.
| onReload, | ||
| onSave, | ||
| }: MarkdownRendererProps) { | ||
| const [viewMode, _setViewMode] = useState<MarkdownViewMode>("rendered"); |
Contributor
There was a problem hiding this comment.
The raw markdown mode is unreachable right now.
_setViewMode is never used, and MarkdownRenderer does not accept external view-mode state. The "raw" branch below is dead code, so users cannot actually switch out of rendered mode.
Also applies to: 46-63, 77-99
🤖 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/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx
at line 25, The MarkdownRenderer component currently initializes internal state
const [viewMode, _setViewMode] = useState<MarkdownViewMode>("rendered") but
never exposes or uses the setter, making the "raw" branch unreachable; update
MarkdownRenderer to accept a controlled prop (e.g., viewMode: MarkdownViewMode
and onViewModeChange: (m: MarkdownViewMode) => void) or at minimum use an
internal setter (rename _setViewMode to setViewMode) and wire the UI controls
that toggle modes to call the setter; ensure the component reads the external
prop if provided (falling back to internal state) so the "raw" rendering branch
(references to viewMode in render logic) becomes reachable and update any
related handlers/controls in this file to invoke the new onViewModeChange or
setViewMode.
Comment on lines
+26
to
+40
| const currentContentRef = useRef(content); | ||
| const [savedContent, setSavedContent] = useState(content); | ||
|
|
||
| const handleChange = useCallback( | ||
| (value: string) => { | ||
| currentContentRef.current = value; | ||
| onDirtyChange(value !== savedContent); | ||
| }, | ||
| [onDirtyChange, savedContent], | ||
| ); | ||
|
|
||
| const handleSave = useCallback(async () => { | ||
| await onSave(currentContentRef.current); | ||
| setSavedContent(currentContentRef.current); | ||
| }, [onSave]); |
Contributor
There was a problem hiding this comment.
Resync markdown dirty state when the underlying content changes.
Like CodeRenderer, currentContentRef and savedContent are initialized once and never refreshed from content. Reloading the file or reusing the pane for another markdown document leaves dirty tracking tied to the previous version.
🤖 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/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx
around lines 26 - 40, The markdown dirty-state isn't resynced when the incoming
content prop changes; update currentContentRef and savedContent whenever content
changes and ensure onDirtyChange is called to reflect the new comparison. Add a
useEffect that runs on content (and optionally onDirtyChange) which sets
currentContentRef.current = content, setSavedContent(content), and calls
onDirtyChange(false) (or onDirtyChange(content !== savedContent) if you prefer
to preserve earlier semantics) so dirty tracking resets when the underlying
document is reloaded or the pane is reused; keep handleChange and handleSave
logic unchanged otherwise.
Comment on lines
+96
to
+103
| { | ||
| label: "Revoke", | ||
| variant: "destructive", | ||
| onClick: async () => { | ||
| await authClient.apiKey.delete({ keyId: id }); | ||
| toast.success("API key revoked"); | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
Add error handling for the async delete operation.
The onClick handler lacks error handling. If authClient.apiKey.delete fails, the user receives no feedback and the promise rejection may go unhandled. Compare with handleGenerateKey (lines 69-88) which properly wraps the async operation in try/catch.
🛡️ Proposed fix
{
label: "Revoke",
variant: "destructive",
onClick: async () => {
+ try {
await authClient.apiKey.delete({ keyId: id });
toast.success("API key revoked");
+ } catch (error) {
+ console.error("[api-keys] Failed to revoke API key:", error);
+ toast.error("Failed to revoke API key");
+ }
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| label: "Revoke", | |
| variant: "destructive", | |
| onClick: async () => { | |
| await authClient.apiKey.delete({ keyId: id }); | |
| toast.success("API key revoked"); | |
| }, | |
| }, | |
| { | |
| label: "Revoke", | |
| variant: "destructive", | |
| onClick: async () => { | |
| try { | |
| await authClient.apiKey.delete({ keyId: id }); | |
| toast.success("API key revoked"); | |
| } catch (error) { | |
| console.error("[api-keys] Failed to revoke API key:", error); | |
| toast.error("Failed to revoke API key"); | |
| } | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx`
around lines 96 - 103, The onClick handler for the "Revoke" action calls
authClient.apiKey.delete({ keyId: id }) without error handling; wrap that async
call in a try/catch similar to handleGenerateKey, calling toast.success on
success and toast.error with a helpful message (and optionally logging the
error) on failure to avoid unhandled rejections and provide user feedback;
update the anonymous onClick async function in the ApiKeysSettings component to
perform the try/catch around authClient.apiKey.delete and use toast.error on
catch.
Comment on lines
+64
to
+70
| onClick={onClick} | ||
| onAuxClick={(e) => { | ||
| if (e.button === 1 && onMiddleClick) { | ||
| e.preventDefault(); | ||
| onMiddleClick(); | ||
| } | ||
| }} |
Contributor
There was a problem hiding this comment.
Ignore bubbled clicks from nested header controls.
Because these handlers sit on the outer header container, clicks from toolbar/actionsContent children also trigger the header action. In this PR that means action buttons can pin the pane, and middle-clicking a nested control can close the pane.
Possible fix
- onClick={onClick}
- onAuxClick={(e) => {
- if (e.button === 1 && onMiddleClick) {
- e.preventDefault();
- onMiddleClick();
- }
- }}
+ onClick={(e) => {
+ const target = e.target as HTMLElement;
+ if (target.closest("button,a,input,textarea,select,[role='button']")) {
+ return;
+ }
+ onClick?.();
+ }}
+ onAuxClick={(e) => {
+ const target = e.target as HTMLElement;
+ if (target.closest("button,a,input,textarea,select,[role='button']")) {
+ return;
+ }
+ if (e.button === 1 && onMiddleClick) {
+ e.preventDefault();
+ onMiddleClick();
+ }
+ }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx`
around lines 64 - 70, The header click handlers (onClick and onAuxClick) are
firing for clicks that bubble up from nested controls (toolbar/actionsContent),
so update the handlers in PaneHeader.tsx to ignore bubbled events by checking
event.target !== event.currentTarget (or event.currentTarget.contains check) and
returning early; apply the same guard in the middle-click onAuxClick branch
before calling e.preventDefault() and onMiddleClick so clicks on nested buttons
won’t trigger header actions.
Comment on lines
+56
to
+68
| const handleClose = () => { | ||
| setIsOpen(false); | ||
| }; | ||
|
|
||
| if (!alertOptions) return null; | ||
|
|
||
| const actions = [...alertOptions.actions].reverse(); | ||
|
|
||
| return ( | ||
| <Dialog | ||
| modal={true} | ||
| open={isOpen} | ||
| onOpenChange={(open) => !open && handleCancel()} | ||
| onOpenChange={(open) => !open && handleClose()} |
Contributor
There was a problem hiding this comment.
Passive dismissals need a caller-visible outcome.
handleClose() only hides the dialog. Any caller that wraps alert() in a promise now has no resolution path when onOpenChange(false) happens, so flows like the new unsaved-file prompt can get stuck waiting forever unless the user clicks one of the action buttons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/atoms/Alert/Alert.tsx` around lines 56 - 68, The dialog's
passive close currently only hides the UI (handleClose -> setIsOpen(false))
leaving any caller waiting on an alert unresolved; update the
onOpenChange/handleClose path so that when open becomes false it also triggers
the alert's caller-visible outcome: call a provided dismissal/resolution hook on
alertOptions (e.g., alertOptions.onDismiss or alertOptions.resolve/reject) or,
if none exists, invoke a sensible fallback such as the "cancel" action callback
from alertOptions.actions (use the same action callback code path used when
clicking buttons) so callers waiting on the alert get resolved/rejected when the
user passively closes the Dialog.
Comment on lines
+483
to
+485
| const reveal = useCallback( | ||
| async (absolutePath: string): Promise<void> => { | ||
| if (!rootPath || !absolutePath.startsWith(rootPath)) return; |
Contributor
There was a problem hiding this comment.
Use the existing segment-aware path check here.
Line 485 uses startsWith(rootPath), which also matches sibling paths like /repo-old for a root of /repo. That lets reveal() expand and fetch directories outside the mounted tree. Reuse isWithinPath(rootPath, absolutePath) instead.
Small fix
- if (!rootPath || !absolutePath.startsWith(rootPath)) return;
+ if (!rootPath || !isWithinPath(rootPath, absolutePath)) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const reveal = useCallback( | |
| async (absolutePath: string): Promise<void> => { | |
| if (!rootPath || !absolutePath.startsWith(rootPath)) return; | |
| const reveal = useCallback( | |
| async (absolutePath: string): Promise<void> => { | |
| if (!rootPath || !isWithinPath(rootPath, absolutePath)) return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workspace-client/src/hooks/useFileTree/useFileTree.ts` around lines
483 - 485, The reveal function's boundary check uses
absolutePath.startsWith(rootPath) which incorrectly matches sibling paths;
replace that check with the segment-aware helper isWithinPath(rootPath,
absolutePath) inside the async reveal(absolutePath: string) callback so the
function only proceeds when the path is truly inside the mounted tree (update
the conditional in reveal to call isWithinPath and return early if it returns
false).
Comment on lines
+496
to
+499
| // Expand all ancestors and load their contents | ||
| for (const dir of ancestors) { | ||
| await expand(dir); | ||
| } |
Contributor
There was a problem hiding this comment.
reveal() can resolve before the ancestors are actually loaded.
await expand(dir) is not enough when that directory is already in loadingDirectories: loadDirectory() returns immediately in that case, so reveal() resolves before the node exists in rootEntries. The first reveal-and-scroll path can therefore race the initial root fetch and miss the scroll entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workspace-client/src/hooks/useFileTree/useFileTree.ts` around lines
496 - 499, reveal() can resolve before an ancestor's contents are actually
populated because await expand(dir) returns immediately if loadDirectory() sees
the directory in loadingDirectories; fix by ensuring each ancestor is fully
loaded into rootEntries before proceeding: after calling expand(dir) (or inside
expand), wait for a directory-loaded signal (e.g. a new helper like
waitForDirectoryLoaded(dir) that resolves when rootEntries contains the node or
when loadingDirectories no longer contains dir), and use that helper for each
ancestor in the for loop so reveal() only continues once the node is present in
rootEntries; reference expand(), loadDirectory(), loadingDirectories,
rootEntries and reveal() when adding this wait logic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsx (1)
65-84:⚠️ Potential issue | 🟠 MajorThis reveal effect still skips the initial selection and can race on fast changes.
Line 66 seeds
prevSelectedReffromselectedFilePath, so the first mounted selection—and the common case whererootPathbecomes available after the workspace query resolves—never triggersreveal(). The async callback on Lines 74-80 also never re-checks the latest selection, so an olderreveal()can recenter the sidebar after a newer click.Suggested fix
const scrollContainerRef = useRef<HTMLDivElement>(null); - const prevSelectedRef = useRef(selectedFilePath); + const prevSelectedRef = useRef<string | undefined>(undefined); useEffect(() => { - if ( - selectedFilePath && - selectedFilePath !== prevSelectedRef.current && - rootPath - ) { - void fileTree.reveal(selectedFilePath).then(() => { - requestAnimationFrame(() => { - const el = scrollContainerRef.current?.querySelector( - `[data-filepath="${CSS.escape(selectedFilePath)}"]`, - ); - el?.scrollIntoView({ block: "center" }); - }); - }); - } - prevSelectedRef.current = selectedFilePath; + if (!selectedFilePath || !rootPath) { + prevSelectedRef.current = undefined; + return; + } + if (selectedFilePath === prevSelectedRef.current) { + return; + } + + const path = selectedFilePath; + prevSelectedRef.current = path; + void fileTree.reveal(path).then(() => { + if (prevSelectedRef.current !== path) { + return; + } + requestAnimationFrame(() => { + const el = scrollContainerRef.current?.querySelector( + `[data-filepath="${CSS.escape(path)}"]`, + ); + el?.scrollIntoView({ block: "center" }); + }); + }); }, [selectedFilePath, rootPath, fileTree]);🤖 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/components/RightSidebar/RightSidebar.tsx around lines 65 - 84, prevSelectedRef is initialized to selectedFilePath which prevents the initial reveal and the async reveal can race; change the effect to initialize prevSelectedRef to null (so first mount triggers) and, inside the useEffect that calls fileTree.reveal, capture the current selection into a local constant (e.g., const current = selectedFilePath) before awaiting the async call and after the promise resolves verify that prevSelectedRef.current === current (or that selectedFilePath still equals current) before calling scrollIntoView and before updating prevSelectedRef.current; also only set prevSelectedRef.current after a successful, still-relevant reveal and continue to require rootPath in the trigger conditions to avoid spurious reveals.
🧹 Nitpick comments (2)
packages/panes/src/core/store/store.ts (1)
333-351: Skip no-op state updates when pin value is unchanged.Optional polish: if
pane.pinned === args.pinned, returnsto avoid unnecessary updates/rerenders.♻️ Suggested diff
setPanePinned: (args) => { set((s) => { for (const tab of s.tabs) { const pane = tab.panes[args.paneId]; if (pane) { + if (pane.pinned === args.pinned) return s; return { tabs: s.tabs.map((t) => t.id === tab.id ? { ...t, panes: { ...t.panes, [args.paneId]: { ...pane, pinned: args.pinned }, }, } : t, ), }; } } return s; }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/panes/src/core/store/store.ts` around lines 333 - 351, When updating a pane's pinned state inside the reducer that iterates over s.tabs and uses args.paneId, first check if the found pane's current pane.pinned equals args.pinned and, if so, return s immediately to skip the no-op update; otherwise proceed to return the updated tabs structure (the block that maps s.tabs and replaces the pane with { ...pane, pinned: args.pinned }). This prevents unnecessary state replacements and re-renders when the pinned value is unchanged.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx (1)
70-97:MarkdownViewModeToggleis exported but not integrated.This toggle component is defined and exported, but
MarkdownRendererdoesn't use it internally (nor acceptviewModeas a controlled prop). The PR summary mentionsrenderHeaderExtrasbut the wiring appears incomplete—users currently cannot switch between rendered and raw modes.Consider either:
- Integrating the toggle inside
MarkdownRenderer(using the unused_setViewMode)- Lifting
viewModestate to the parent and passing it as a controlled prop🤖 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/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx around lines 70 - 97, The MarkdownViewModeToggle component is exported but not wired into MarkdownRenderer, so users can't switch modes; either wire it as an internal control by using the existing internal setter _setViewMode inside MarkdownRenderer or make viewMode controlled by lifting state to the parent: add a viewMode prop and onViewModeChange callback to MarkdownRenderer (and consume them where MarkdownRenderer is instantiated), then render <MarkdownViewModeToggle viewMode={viewMode} onViewModeChange={onViewModeChange} /> (or call _setViewMode in MarkdownRenderer when toggled) and ensure MarkdownRenderer switches between the raw and rendered branches based on the active viewMode.
🤖 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/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx:
- Around line 35-38: The handleSave callback currently calls setSavedContent
unconditionally after awaiting onSave, which desynchronizes savedContent when
onSave returns a non-"saved" result (e.g., user cancelled); update handleSave to
inspect the onSave result (or catch a rejection) and only call
setSavedContent(currentContentRef.current) when the returned result indicates
success (e.g., result.status === 'saved' or a truthy success indicator). Also
ensure the onSave prop contract is adjusted (or FilePane throws on non-saved) so
the check is reliable; keep references to handleSave, onSave, currentContentRef,
and setSavedContent when making the change.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/RightSidebar.tsx:
- Around line 65-84: prevSelectedRef is initialized to selectedFilePath which
prevents the initial reveal and the async reveal can race; change the effect to
initialize prevSelectedRef to null (so first mount triggers) and, inside the
useEffect that calls fileTree.reveal, capture the current selection into a local
constant (e.g., const current = selectedFilePath) before awaiting the async call
and after the promise resolves verify that prevSelectedRef.current === current
(or that selectedFilePath still equals current) before calling scrollIntoView
and before updating prevSelectedRef.current; also only set
prevSelectedRef.current after a successful, still-relevant reveal and continue
to require rootPath in the trigger conditions to avoid spurious reveals.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx:
- Around line 70-97: The MarkdownViewModeToggle component is exported but not
wired into MarkdownRenderer, so users can't switch modes; either wire it as an
internal control by using the existing internal setter _setViewMode inside
MarkdownRenderer or make viewMode controlled by lifting state to the parent: add
a viewMode prop and onViewModeChange callback to MarkdownRenderer (and consume
them where MarkdownRenderer is instantiated), then render
<MarkdownViewModeToggle viewMode={viewMode} onViewModeChange={onViewModeChange}
/> (or call _setViewMode in MarkdownRenderer when toggled) and ensure
MarkdownRenderer switches between the raw and rendered branches based on the
active viewMode.
In `@packages/panes/src/core/store/store.ts`:
- Around line 333-351: When updating a pane's pinned state inside the reducer
that iterates over s.tabs and uses args.paneId, first check if the found pane's
current pane.pinned equals args.pinned and, if so, return s immediately to skip
the no-op update; otherwise proceed to return the updated tabs structure (the
block that maps s.tabs and replaces the pane with { ...pane, pinned: args.pinned
}). This prevents unnecessary state replacements and re-renders when the pinned
value is unchanged.
🪄 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: ac5fdc7b-9957-4016-b55c-77a6e1ae5c69
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsxpackages/panes/src/core/store/store.tspackages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx
Comment on lines
+35
to
+38
| const handleSave = useCallback(async () => { | ||
| await onSave(currentContentRef.current); | ||
| setSavedContent(currentContentRef.current); | ||
| }, [onSave]); |
Contributor
There was a problem hiding this comment.
handleSave unconditionally updates savedContent without checking save result.
When onSave returns a non-"saved" result (e.g., user cancels the dialog), this still calls setSavedContent, desyncing dirty tracking from actual file state. This is the consumer-side counterpart to the FilePane.handleSave issue.
Proposed fix
const handleSave = useCallback(async () => {
- await onSave(currentContentRef.current);
- setSavedContent(currentContentRef.current);
+ const result = await onSave(currentContentRef.current);
+ if (result.status === "saved") {
+ setSavedContent(currentContentRef.current);
+ }
}, [onSave]);This requires updating the onSave prop type to return a result with a status field, or having FilePane throw/reject on non-saved status so setSavedContent is skipped.
🤖 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/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx
around lines 35 - 38, The handleSave callback currently calls setSavedContent
unconditionally after awaiting onSave, which desynchronizes savedContent when
onSave returns a non-"saved" result (e.g., user cancelled); update handleSave to
inspect the onSave result (or catch a rejection) and only call
setSavedContent(currentContentRef.current) when the returned result indicates
success (e.g., result.status === 'saved' or a truthy success indicator). Also
ensure the onSave prop contract is adjusted (or FilePane throws on non-saved) so
the check is reliable; keep references to handleSave, onSave, currentContentRef,
and setSavedContent when making the change.
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。 取り込み済み (cherry-pick / 手動移植): - be22b46 superset-sh#3125 — スキップ (下記参照) - 88bc7fb superset-sh#3127 — Revert DA1 ✓ - 92d0ff9 superset-sh#3054 — DA1 fix ✓ - c48450e superset-sh#3093 — file viewer pane fix ✓ - fffa8db superset-sh#3128 — version 1.4.7 ✓ - 589a7c7 superset-sh#3136 — fuzzy scorer (ハイブリッド方式) ✓ - ceb8c81 superset-sh#3150 — Electron 40.8.5 ✓ - 8922b94 superset-sh#3137 — terminalId分離 ✓ - c7508e5 superset-sh#3152 — GitHub無料化 ✓ - 2b91f11 superset-sh#3155 — v2 terminal theme ✓ - b8b11af superset-sh#3154 — TUI dimension fix ✓ - 7599ace superset-sh#3149 — v2 sidebar file tree (手動統合) ✓ - 4d7c612 superset-sh#3174 — DnD重複削除 ✓ - 864977d superset-sh#3157 — Host Service分離 ✓ 意図的にスキップ: - be22b46 superset-sh#3125 (GitHub polling簡素化) フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と 設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と github-sync-service.ts のFORK NOTEを参照。 ゴースト・マージ復元 (revert 134cfd5 で消失した内容): - 538f306 superset-sh#3120 — Patch vuln ✓ - 1588d20 superset-sh#3108 — terminal lifecycle分離 ✓ - 59426f6 superset-sh#3122 — file tree + FilePane + Alert refactor (手動統合) ✓ - 10d9a5d superset-sh#3097 — tiptap line-height ✓ - 337a9ae superset-sh#3121 — Codex hooks削除 ✓
This was referenced Apr 12, 2026
Merged
This was referenced Apr 23, 2026
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
<img>for imagesonConfirm/onCancelto flexibleactionsarray APIChanges
File tree sidebar
RightSidebarcomponent with file tree, search, toolbarCmd+Ltoggles sidebar, state persisted inv2WorkspaceLocalStatecollectionselectedFilePathderived from store's active pane — sidebar highlights active fileuseFileTree.reveal(path)expands ancestor dirs and scrolls into viewlistDirectorycache (staleTime: 0)File pane
FilePaneorchestrator routes toCodeRenderer,MarkdownRenderer, orImageRendererhasChangesin pane data, filled circle indicator in titleonBeforeCloseonPaneDefinition— Save/Don't Save/Cancel dialogonHeaderClickonPaneDefinition— click to pin, middle-click to closesetPanePinnedno longer requirestabIdAlert component
actions: AlertAction[]API replacesonConfirm/onCancel/confirmText/cancelTextlabel,variant(uses Button variants directly),onClickOther
Cmd+O(Open in App) wired up insideOpenInMenuButtoncomponentWorkspaceFilePreview(replaced byFilePane)FilesPane/toRightSidebar/Test plan
Summary by cubic
Adds a right-side file tree (Cmd+L) with per-workspace persistence and a new FilePane that renders code, markdown, and images with dirty tracking and external-change prompts. Refactors alerts to an actions API and updates pane behaviors (pin on header click, middle-click close, save prompts); the sidebar is resizable and auto-reveals and centers the active file.
New Features
v2WorkspaceLocalState. Highlights the active file, auto-expands ancestors, and scrolls the selection to the center; click opens or pins if already active. Resizable layout via@superset/ui/resizable.OpenInMenuButtonviauseAppHotkey.Refactors
@superset/ui: replaced confirm/cancel withactions: AlertAction[]; all call sites updated.@superset/panes: addedonBeforeCloseandonHeaderClick; header click pins, middle-click closes;setPanePinned({ paneId, pinned })(removedtabId); file-type icons and dirty indicator in tab headers.useFileTree: addedreveal()and fixed stalelistDirectorycache (staleTime: 0).Written for commit b7ccf6a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements