Repository navigation
feat(desktop): modifier+click in file tree — external editor / new tab - #3398
Merged
Conversation
…w tab - Meta/Ctrl+click: open in external editor (works on files and folders) - Shift+click: open file in a new internal tab - Plain click: unchanged (preview pane / toggle folder) Adds the behavior to both v1 (FileTreeItem, FileSearchResultItem) and v2 (WorkspaceFilesTreeItem) file trees. v2 guards the external-editor path with an isLocalWorkspace check so remote hosts toast instead of silently calling local electron IPC against a path that does not exist.
Contributor
📝 WalkthroughWalkthroughAdded an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TreeItem as FilesTab/TreeItem
participant LocalQuery as useLiveQuery\n(v2Workspaces & v2Hosts)
participant ElectronRPC as Electron tRPC\nexternal.openInApp
participant Toast as Toast Notifier
User->>TreeItem: Click file (meta/ctrl)
TreeItem->>LocalQuery: Query workspace locality & default app
LocalQuery-->>TreeItem: { isLocal, defaultOpenInApp }
alt isLocal
TreeItem->>ElectronRPC: mutate({ path, app, cwd, projectId })
ElectronRPC-->>TreeItem: success / error
alt error
TreeItem->>Toast: show error
else success
TreeItem-->>User: editor opened
end
else notLocal
TreeItem->>Toast: show "workspace not local" error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
Greptile SummaryThis PR adds modifier-key click handling to both the v1 ( Key additions:
One P1 concern: Confidence Score: 4/5Safe to merge after addressing the loading-state false-negative in handleOpenInEditor One P1 UX bug (spurious toast during query loading window) needs a small guard fix; the rest of the feature is well-structured and correct FilesTab.tsx — isLocalWorkspace evaluation needs a loading-state guard before the error toast is reachable Important Files Changed
Reviews (1): Last reviewed commit: "feat(desktop): modifier+click in file tr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+239
to
+259
| const isLocalWorkspace = workspacesWithHost[0]?.hostMachineId === machineId; | ||
|
|
||
| const handleOpenInEditor = useCallback( | ||
| (absolutePath: string) => { | ||
| if (!isLocalWorkspace) { | ||
| toast.error("Opening in editor is only supported on local workspaces"); | ||
| return; | ||
| } | ||
| electronTrpcClient.external.openFileInEditor | ||
| .mutate({ | ||
| path: absolutePath, | ||
| cwd: rootPath || undefined, | ||
| projectId, | ||
| }) | ||
| .catch((err) => { | ||
| toast.error("Couldn't open file", { | ||
| description: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| }); | ||
| }, | ||
| [isLocalWorkspace, rootPath, projectId], |
Contributor
There was a problem hiding this comment.
isLocalWorkspace during loading shows spurious error toast
workspacesWithHost defaults to [] (via = []) while the live query is still resolving. During that window workspacesWithHost[0]?.hostMachineId is undefined, so undefined === machineId evaluates to false — making isLocalWorkspace always false until data arrives. Because handleOpenInEditor immediately toasts an error when !isLocalWorkspace, a user who Meta+clicks a file in a local workspace before the query resolves will see the misleading "Opening in editor is only supported on local workspaces" toast.
In contrast, V2WorkspaceOpenInButton silently hides the button while loading (Boolean(workspace) && ...), so there's no visible error path during the loading window. Here the file-tree rows are already rendered and clickable, so the guard needs to be explicit.
Consider distinguishing the "not yet loaded" state from the "loaded but remote" state:
const hasLoaded = workspacesWithHost.length > 0;
const isLocalWorkspace =
hasLoaded && workspacesWithHost[0]?.hostMachineId === machineId;
const handleOpenInEditor = useCallback(
(absolutePath: string) => {
if (!hasLoaded) return; // Still initialising — silently abort
if (!isLocalWorkspace) {
toast.error("Opening in editor is only supported on local workspaces");
return;
}
// ...
},
[hasLoaded, isLocalWorkspace, rootPath, projectId],
);| const state = store.getState(); | ||
| if (openInNewTab) { | ||
| state.addTab({ | ||
| titleOverride: filePath.split("/").pop(), |
Contributor
There was a problem hiding this comment.
pop() can return undefined for edge-case paths
String.prototype.split("/").pop() returns undefined when the resulting array is empty. Since CreateTabInput.titleOverride is typed as string | undefined, TypeScript accepts this and runtime behavior is fine (tab gets no title). A fallback makes the intent clearer:
Suggested change
| titleOverride: filePath.split("/").pop(), | |
| titleOverride: filePath.split("/").pop() ?? filePath, |
Comment on lines
+224
to
+239
| const collections = useCollections(); | ||
| const { machineId } = useLocalHostService(); | ||
| const { data: workspacesWithHost = [] } = useLiveQuery( | ||
| (q) => | ||
| q | ||
| .from({ workspaces: collections.v2Workspaces }) | ||
| .leftJoin({ hosts: collections.v2Hosts }, ({ workspaces, hosts }) => | ||
| eq(workspaces.hostId, hosts.id), | ||
| ) | ||
| .where(({ workspaces }) => eq(workspaces.id, workspaceId)) | ||
| .select(({ hosts }) => ({ | ||
| hostMachineId: hosts?.machineId ?? null, | ||
| })), | ||
| [collections, workspaceId], | ||
| ); | ||
| const isLocalWorkspace = workspacesWithHost[0]?.hostMachineId === machineId; |
Contributor
There was a problem hiding this comment.
layout.tsx and V2WorkspaceOpenInButton already resolve hostMachineId and compute isLocal for this same workspace. Adding a third independent live query here spreads the join logic further and adds unnecessary reactive overhead. Consider lifting isLocalWorkspace from the layout as a prop (WorkspaceSidebar → FilesTab), or exposing it via shared context.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…idebarProjects Align with the per-project v2 "Open In" pattern established in #3393: resolve the editor from collections.v2SidebarProjects.defaultOpenInApp (falling back to finder) and call electronTrpc.external.openInApp — instead of openFileInEditor, which does v1-era backend resolution off the projects.defaultApp column that v2 doesn't write to.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 6 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="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:111">
P2: Deriving the tab title with `split("/")` is not cross-platform and breaks basename extraction for Windows paths. Use a separator-safe approach.
(Based on your team's feedback about using cross-platform path utilities instead of manual split.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:239">
P2: `isLocalWorkspace` is `false` while the live query is still resolving (the default `[]` makes `workspacesWithHost[0]?.hostMachineId` evaluate to `undefined`). A user who Meta+clicks a file before the query completes will see the misleading "only supported on local workspaces" toast even on a local workspace. Distinguish "not yet loaded" from "loaded but remote" — e.g. gate `handleOpenInEditor` on `workspacesWithHost.length > 0` so it silently no-ops until data arrives.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 1
🤖 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/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 227-267: The issue is that useLiveQuery results are defaulted to
[] which hides the loading state and causes isLocalWorkspace and
resolvedOpenInApp to be derived incorrectly; remove the default [] for
workspacesWithHost and sidebarProjectRows and add explicit undefined checks
before using them (e.g., don't compute isLocalWorkspace or resolvedOpenInApp
until workspacesWithHost and sidebarProjectRows are not undefined), update
handleOpenInEditor to guard early if those query results are undefined (show a
loading/disabled state or no-op) and ensure useCallback dependencies include any
values used for the guards (workspacesWithHost, sidebarProjectRows, machineId,
projectId) so the derived behavior updates once queries resolve.
🪄 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: d858e7b4-b1a0-4fcb-808b-d5e46c04428d
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
Comment on lines
+227
to
+267
| const { data: workspacesWithHost = [] } = useLiveQuery( | ||
| (q) => | ||
| q | ||
| .from({ workspaces: collections.v2Workspaces }) | ||
| .leftJoin({ hosts: collections.v2Hosts }, ({ workspaces, hosts }) => | ||
| eq(workspaces.hostId, hosts.id), | ||
| ) | ||
| .where(({ workspaces }) => eq(workspaces.id, workspaceId)) | ||
| .select(({ hosts }) => ({ | ||
| hostMachineId: hosts?.machineId ?? null, | ||
| })), | ||
| [collections, workspaceId], | ||
| ); | ||
| const isLocalWorkspace = workspacesWithHost[0]?.hostMachineId === machineId; | ||
|
|
||
| const { data: sidebarProjectRows = [] } = useLiveQuery( | ||
| (q) => | ||
| q | ||
| .from({ sp: collections.v2SidebarProjects }) | ||
| .where(({ sp }) => eq(sp.projectId, projectId ?? "")) | ||
| .select(({ sp }) => ({ defaultOpenInApp: sp.defaultOpenInApp })), | ||
| [collections, projectId], | ||
| ); | ||
| const resolvedOpenInApp: ExternalApp = | ||
| (sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? "finder"; | ||
|
|
||
| const handleOpenInEditor = useCallback( | ||
| (absolutePath: string) => { | ||
| if (!isLocalWorkspace) { | ||
| toast.error("Opening in editor is only supported on local workspaces"); | ||
| return; | ||
| } | ||
| electronTrpcClient.external.openInApp | ||
| .mutate({ path: absolutePath, app: resolvedOpenInApp }) | ||
| .catch((err) => { | ||
| toast.error("Couldn't open file", { | ||
| description: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| }); | ||
| }, | ||
| [isLocalWorkspace, resolvedOpenInApp], |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the version of @tanstack/react-dbused in this repository, canuseLiveQueryexposedataasundefined before the first result is available?
💡 Result:
Yes, useLiveQuery can expose data as undefined before the first result is available.
Citations:
- 1: https://tanstack.com/db/latest/docs/framework/react/reference/functions/useLiveQuery
- 2: https://github.com/TanStack/db/blob/main/packages/react-db/src/useLiveQuery.ts
- 3: https://tanstack.com/db/latest/docs/guides/live-queries
- 4: https://tanstack.com/db/latest/docs/framework/react/overview
🏁 Script executed:
# Check the version of `@tanstack/react-db` in the repo
fd -t f "package.json" | head -1 | xargs cat | jq '.dependencies, .devDependencies' | grep -A 2 -B 2 "tanstack/react-db"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for useLiveQuery usages in the codebase to understand patterns
rg "useLiveQuery" --type tsx --type ts -B 2 -A 8 | head -100Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the specific file to see the full context of handleOpenInEditor usage
cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx | sed -n '227,280p'Repository: superset-sh/superset
Length of output: 2229
🏁 Script executed:
# Find and check package.json
find . -name "package.json" -type f -exec grep -l "tanstack/react-db" {} \; | head -1 | xargs cat | grep -A 1 "tanstack/react-db"Repository: superset-sh/superset
Length of output: 190
🏁 Script executed:
# Search for useLiveQuery usages with proper type flags
rg "useLiveQuery" -A 5 | head -150Repository: superset-sh/superset
Length of output: 17996
🏁 Script executed:
# Check if there are existing tri-state patterns for loading in the codebase
rg "data\s*=" -A 2 | grep -E "(undefined|null)" | head -50Repository: superset-sh/superset
Length of output: 2149
🏁 Script executed:
# Check if FilesTab.tsx uses isLoading flag from useLiveQuery
grep -n "isLoading\|isFetching\|isLoading\|isLoading" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx | head -20Repository: superset-sh/superset
Length of output: 98
🏁 Script executed:
# Check how other similar components handle loading state - look at AccountSettings pattern more closely
sed -n '1,50p' apps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsxRepository: superset-sh/superset
Length of output: 1759
🏁 Script executed:
# Verify the handleOpenInEditor is called directly from UI or through a guarded callback
grep -n "handleOpenInEditor\|onOpenInEditor" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx | head -20Repository: superset-sh/superset
Length of output: 316
Check loading state before deriving locality and app configuration.
useLiveQuery initially returns data: undefined before the first result arrives. The current code defaults to [], which conflates loading with empty-result, causing isLocalWorkspace to evaluate as false and resolvedOpenInApp to default to "finder" during mount. A quick modifier-click right after the component mounts will incorrectly show the remote-workspace toast for a local workspace, or open Finder before the configured editor preference loads.
Remove the default [] values and check for undefined before using the results:
Suggested fix
- const { data: workspacesWithHost = [] } = useLiveQuery(
+ const { data: workspacesWithHost } = useLiveQuery(
...
);
- const isLocalWorkspace = workspacesWithHost[0]?.hostMachineId === machineId;
+ const isLocalWorkspace =
+ workspacesWithHost == null
+ ? undefined
+ : workspacesWithHost[0]?.hostMachineId === machineId;
- const { data: sidebarProjectRows = [] } = useLiveQuery(
+ const { data: sidebarProjectRows } = useLiveQuery(
...
);
- const resolvedOpenInApp: ExternalApp =
- (sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? "finder";
+ const resolvedOpenInApp: ExternalApp | undefined =
+ sidebarProjectRows == null
+ ? undefined
+ : ((sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ??
+ "finder");
const handleOpenInEditor = useCallback(
(absolutePath: string) => {
+ if (isLocalWorkspace == null || resolvedOpenInApp == null) return;
if (!isLocalWorkspace) {📝 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 { data: workspacesWithHost = [] } = useLiveQuery( | |
| (q) => | |
| q | |
| .from({ workspaces: collections.v2Workspaces }) | |
| .leftJoin({ hosts: collections.v2Hosts }, ({ workspaces, hosts }) => | |
| eq(workspaces.hostId, hosts.id), | |
| ) | |
| .where(({ workspaces }) => eq(workspaces.id, workspaceId)) | |
| .select(({ hosts }) => ({ | |
| hostMachineId: hosts?.machineId ?? null, | |
| })), | |
| [collections, workspaceId], | |
| ); | |
| const isLocalWorkspace = workspacesWithHost[0]?.hostMachineId === machineId; | |
| const { data: sidebarProjectRows = [] } = useLiveQuery( | |
| (q) => | |
| q | |
| .from({ sp: collections.v2SidebarProjects }) | |
| .where(({ sp }) => eq(sp.projectId, projectId ?? "")) | |
| .select(({ sp }) => ({ defaultOpenInApp: sp.defaultOpenInApp })), | |
| [collections, projectId], | |
| ); | |
| const resolvedOpenInApp: ExternalApp = | |
| (sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? "finder"; | |
| const handleOpenInEditor = useCallback( | |
| (absolutePath: string) => { | |
| if (!isLocalWorkspace) { | |
| toast.error("Opening in editor is only supported on local workspaces"); | |
| return; | |
| } | |
| electronTrpcClient.external.openInApp | |
| .mutate({ path: absolutePath, app: resolvedOpenInApp }) | |
| .catch((err) => { | |
| toast.error("Couldn't open file", { | |
| description: err instanceof Error ? err.message : String(err), | |
| }); | |
| }); | |
| }, | |
| [isLocalWorkspace, resolvedOpenInApp], | |
| const { data: workspacesWithHost } = useLiveQuery( | |
| (q) => | |
| q | |
| .from({ workspaces: collections.v2Workspaces }) | |
| .leftJoin({ hosts: collections.v2Hosts }, ({ workspaces, hosts }) => | |
| eq(workspaces.hostId, hosts.id), | |
| ) | |
| .where(({ workspaces }) => eq(workspaces.id, workspaceId)) | |
| .select(({ hosts }) => ({ | |
| hostMachineId: hosts?.machineId ?? null, | |
| })), | |
| [collections, workspaceId], | |
| ); | |
| const isLocalWorkspace = | |
| workspacesWithHost == null | |
| ? undefined | |
| : workspacesWithHost[0]?.hostMachineId === machineId; | |
| const { data: sidebarProjectRows } = useLiveQuery( | |
| (q) => | |
| q | |
| .from({ sp: collections.v2SidebarProjects }) | |
| .where(({ sp }) => eq(sp.projectId, projectId ?? "")) | |
| .select(({ sp }) => ({ defaultOpenInApp: sp.defaultOpenInApp })), | |
| [collections, projectId], | |
| ); | |
| const resolvedOpenInApp: ExternalApp | undefined = | |
| sidebarProjectRows == null | |
| ? undefined | |
| : ((sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? | |
| "finder"); | |
| const handleOpenInEditor = useCallback( | |
| (absolutePath: string) => { | |
| if (isLocalWorkspace == null || resolvedOpenInApp == null) return; | |
| if (!isLocalWorkspace) { | |
| toast.error("Opening in editor is only supported on local workspaces"); | |
| return; | |
| } | |
| electronTrpcClient.external.openInApp | |
| .mutate({ path: absolutePath, app: resolvedOpenInApp }) | |
| .catch((err) => { | |
| toast.error("Couldn't open file", { | |
| description: err instanceof Error ? err.message : String(err), | |
| }); | |
| }); | |
| }, | |
| [isLocalWorkspace, resolvedOpenInApp], |
🤖 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/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
around lines 227 - 267, The issue is that useLiveQuery results are defaulted to
[] which hides the loading state and causes isLocalWorkspace and
resolvedOpenInApp to be derived incorrectly; remove the default [] for
workspacesWithHost and sidebarProjectRows and add explicit undefined checks
before using them (e.g., don't compute isLocalWorkspace or resolvedOpenInApp
until workspacesWithHost and sidebarProjectRows are not undefined), update
handleOpenInEditor to guard early if those query results are undefined (show a
loading/disabled state or no-op) and ensure useCallback dependencies include any
values used for the guards (workspacesWithHost, sidebarProjectRows, machineId,
projectId) so the derived behavior updates once queries resolve.
- Gate the isLocal check on the host live query actually resolving, so a meta+click before the collections query settles silently no-ops instead of toasting a misleading "only supported on local workspaces" error (cubic/greptile/coderabbit). - Use a separator-safe split when deriving the new-tab title from a file path so Windows host paths (backslash-separated) don't produce a full path as the tab title (cubic).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx (1)
227-251:⚠️ Potential issue | 🟡 MinorDon’t collapse unresolved live-query state into Finder/remote fallbacks.
hostMachineIdis explicitly nullable here, andsidebarProjectRows = []turns “still loading” into “no preference configured”. A quick modifier-click can still show the remote-workspace toast while the host join is unresolved, or open Finder beforedefaultOpenInApphas loaded.Suggested fix
- const { data: workspacesWithHost = [] } = useLiveQuery( + const { data: workspacesWithHost } = useLiveQuery( ... ); - const workspaceHost = workspacesWithHost[0]; + const workspaceHost = workspacesWithHost?.[0]; - const { data: sidebarProjectRows = [] } = useLiveQuery( + const { data: sidebarProjectRows } = useLiveQuery( ... ); - const resolvedOpenInApp: ExternalApp = - (sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? "finder"; + const resolvedOpenInApp: ExternalApp | undefined = + sidebarProjectRows == null + ? undefined + : ((sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? + "finder"); const handleOpenInEditor = useCallback( (absolutePath: string) => { - if (!workspaceHost) return; - if (workspaceHost.hostMachineId !== machineId) { + if (!workspaceHost || workspaceHost.hostMachineId == null) return; + if (resolvedOpenInApp == null) return; + if (workspaceHost.hostMachineId !== machineId) { toast.error("Opening in editor is only supported on local workspaces"); return; }Also applies to: 253-268
🤖 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/WorkspaceSidebar/components/FilesTab/FilesTab.tsx around lines 227 - 251, The live-query results are being collapsed into fallbacks too early; stop treating an unresolved query as an empty result by checking for undefined before mapping to fallback values. In the useLiveQuery for workspacesWithHost and for sidebarProjectRows, keep the raw hook result (e.g. workspacesWithHost may be undefined while loading) and only derive workspaceHost.hostMachineId and resolvedOpenInApp when the corresponding data array is present and has an element; reference useLiveQuery, workspacesWithHost, workspaceHost, hostMachineId, sidebarProjectRows, defaultOpenInApp, and resolvedOpenInApp and ensure you only apply the "finder" fallback after confirming sidebarProjectRows is loaded (not when it's undefined).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 227-251: The live-query results are being collapsed into fallbacks
too early; stop treating an unresolved query as an empty result by checking for
undefined before mapping to fallback values. In the useLiveQuery for
workspacesWithHost and for sidebarProjectRows, keep the raw hook result (e.g.
workspacesWithHost may be undefined while loading) and only derive
workspaceHost.hostMachineId and resolvedOpenInApp when the corresponding data
array is present and has an element; reference useLiveQuery, workspacesWithHost,
workspaceHost, hostMachineId, sidebarProjectRows, defaultOpenInApp, and
resolvedOpenInApp and ensure you only apply the "finder" fallback after
confirming sidebarProjectRows is loaded (not when it's undefined).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5ce086a-acd5-405f-9425-ae5d32762481
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 13, 2026
ParadevAI#3398) * feat(desktop): modifier+click in file tree opens external editor / new tab - Meta/Ctrl+click: open in external editor (works on files and folders) - Shift+click: open file in a new internal tab - Plain click: unchanged (preview pane / toggle folder) Adds the behavior to both v1 (FileTreeItem, FileSearchResultItem) and v2 (WorkspaceFilesTreeItem) file trees. v2 guards the external-editor path with an isLocalWorkspace check so remote hosts toast instead of silently calling local electron IPC against a path that does not exist. * fix(desktop): v2 file-tree meta+click reads defaultOpenInApp from v2SidebarProjects Align with the per-project v2 "Open In" pattern established in ParadevAI#3393: resolve the editor from collections.v2SidebarProjects.defaultOpenInApp (falling back to finder) and call electronTrpc.external.openInApp — instead of openFileInEditor, which does v1-era backend resolution off the projects.defaultApp column that v2 doesn't write to. * fix(desktop): address PR review on v2 file-tree meta+click - Gate the isLocal check on the host live query actually resolving, so a meta+click before the collections query settles silently no-ops instead of toasting a misleading "only supported on local workspaces" error (cubic/greptile/coderabbit). - Use a separator-safe split when deriving the new-tab title from a file path so Windows host paths (backslash-separated) don't produce a full path as the tab title (cubic).
Kitenite
deleted the
can-we-wire-up-meta-left-click-to-open-file-in-file-tree-in-external-editor-metashiftleft-click-or-j
branch
April 13, 2026 16:35
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
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
Wired up in both v1 (
FileTreeItem,FileSearchResultItem) and v2 (WorkspaceFilesTreeItem) file trees — v2 had no modifier-key handling at all before, so Meta+click on a remote-host workspace would otherwise call local electron IPC against a path that doesn't exist on the user's machine. v2'shandleOpenInEditorguards onhostMachineId === machineId(same pattern aslayout.tsxandV2WorkspaceOpenInButton) and toasts "only supported on local workspaces" otherwise.v2 also gains a real "open in new tab" pathway —
openFilePanenow acceptsopenInNewTab?and callsstore.addTab(...)when true.Test plan
Summary by cubic
Add modifier-clicks to the desktop file tree: Meta/Ctrl+click opens items in the external app, and Shift+click opens a file in a new tab. Works in v1 and v2; v2 respects the per-project “Open In” setting and handles remote workspaces safely.
New Features
electronTrpcClient.external.openInApp; v2 resolves the app fromv2SidebarProjects.defaultOpenInApp(fallbackfinder) and toasts on remote hosts.openFilePane(filePath, openInNewTab?); plain click, double-click, and keyboard shortcuts are unchanged.Bug Fixes
Written for commit 444a897. Summary will update on new commits.
Summary by CodeRabbit