From 2aa3cc701ec2d1ff43b0ef243dd09d1592526da4 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Mon, 2 Feb 2026 17:13:37 -0600 Subject: [PATCH] MAESTRO: Implement file link navigation behavior Regular click on file links replaces current tab content, while Cmd/Ctrl+Click opens a new tab adjacent to the current tab. --- .../hooks/useMainKeyboardHandler.test.ts | 15 ++ src/renderer/App.tsx | 146 +++++++++++++----- src/renderer/components/FilePreview.tsx | 12 +- src/renderer/components/MainPanel.tsx | 3 +- 4 files changed, 136 insertions(+), 40 deletions(-) diff --git a/src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts b/src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts index c3269715..4ed9f4d6 100644 --- a/src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts +++ b/src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts @@ -344,6 +344,7 @@ describe('useMainKeyboardHandler', () => { const mockActiveSession = { id: 'test-session', name: 'Test', + inputMode: 'ai', aiTabs: [], activeTabId: 'tab-1', unifiedTabOrder: [], @@ -353,6 +354,7 @@ describe('useMainKeyboardHandler', () => { hasOpenLayers: () => true, // Overlay is open (e.g., file preview) hasOpenModal: () => false, // But no true modal isTabShortcut: (_e: KeyboardEvent, actionId: string) => actionId === 'newTab', + activeSessionId: 'test-session', activeSession: mockActiveSession, createTab: vi.fn().mockReturnValue({ session: { ...mockActiveSession, aiTabs: [{ id: 'new-tab' }] }, @@ -383,10 +385,20 @@ describe('useMainKeyboardHandler', () => { const { result } = renderHook(() => useMainKeyboardHandler()); const mockSetTabSwitcherOpen = vi.fn(); + const mockActiveSession = { + id: 'test-session', + name: 'Test', + inputMode: 'ai', + aiTabs: [], + activeTabId: 'tab-1', + unifiedTabOrder: [], + }; result.current.keyboardHandlerRef.current = createMockContext({ hasOpenLayers: () => true, // Overlay is open (e.g., file preview) hasOpenModal: () => false, // But no true modal isTabShortcut: (_e: KeyboardEvent, actionId: string) => actionId === 'tabSwitcher', + activeSessionId: 'test-session', + activeSession: mockActiveSession, setTabSwitcherOpen: mockSetTabSwitcherOpen, }); @@ -418,6 +430,8 @@ describe('useMainKeyboardHandler', () => { const mockActiveSession = { id: 'test-session', name: 'Test', + inputMode: 'ai', + aiTabs: [], unifiedClosedTabHistory: [{ type: 'file', tab: { id: 'closed-tab' } }], }; @@ -425,6 +439,7 @@ describe('useMainKeyboardHandler', () => { hasOpenLayers: () => true, // Overlay is open (e.g., file preview) hasOpenModal: () => false, // But no true modal isTabShortcut: (_e: KeyboardEvent, actionId: string) => actionId === 'reopenClosedTab', + activeSessionId: 'test-session', activeSession: mockActiveSession, reopenUnifiedClosedTab: mockReopenUnifiedClosedTab, setSessions: mockSetSessions, diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index fb35e8a4..672643e9 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -3880,7 +3880,15 @@ function MaestroConsoleInner() { * For SSH remote files, pass sshRemoteId so content can be re-fetched if needed. */ const handleOpenFileTab = useCallback( - (file: { path: string; name: string; content: string; sshRemoteId?: string; lastModified?: number }) => { + ( + file: { path: string; name: string; content: string; sshRemoteId?: string; lastModified?: number }, + options?: { + /** If true, create new tab adjacent to current file tab. If false, replace current file tab content. Default: true (create new tab) */ + openInNewTab?: boolean; + } + ) => { + const openInNewTab = options?.openInNewTab ?? true; // Default to opening in new tab for backward compatibility + setSessions((prev) => prev.map((s) => { if (s.id !== activeSessionIdRef.current) return s; @@ -3907,6 +3915,42 @@ function MaestroConsoleInner() { }; } + // If not opening in new tab and there's an active file tab, replace its content + if (!openInNewTab && s.activeFileTabId) { + const currentTabId = s.activeFileTabId; + const extension = file.name.includes('.') + ? '.' + file.name.split('.').pop() + : ''; + const nameWithoutExtension = extension + ? file.name.slice(0, -extension.length) + : file.name; + + // Replace current tab's content with new file + const updatedTabs = s.filePreviewTabs.map((tab) => + tab.id === currentTabId + ? { + ...tab, + path: file.path, + name: nameWithoutExtension, + extension, + content: file.content, + scrollTop: 0, // Reset scroll for new file + searchQuery: '', // Clear search + editMode: false, + editContent: undefined, + lastModified: file.lastModified ?? Date.now(), + sshRemoteId: file.sshRemoteId, + isLoading: false, + } + : tab + ); + return { + ...s, + filePreviewTabs: updatedTabs, + // activeFileTabId stays the same since we're replacing in-place + }; + } + // Create a new file preview tab const newTabId = generateId(); const extension = file.name.includes('.') @@ -3935,10 +3979,32 @@ function MaestroConsoleInner() { // Create the unified tab reference const newTabRef: UnifiedTabRef = { type: 'file', id: newTabId }; + // If opening in new tab and there's an active file tab, insert adjacent to it + let updatedUnifiedTabOrder: UnifiedTabRef[]; + if (openInNewTab && s.activeFileTabId) { + const currentIndex = s.unifiedTabOrder.findIndex( + (ref) => ref.type === 'file' && ref.id === s.activeFileTabId + ); + if (currentIndex !== -1) { + // Insert right after the current file tab + updatedUnifiedTabOrder = [ + ...s.unifiedTabOrder.slice(0, currentIndex + 1), + newTabRef, + ...s.unifiedTabOrder.slice(currentIndex + 1), + ]; + } else { + // Fallback: append at end + updatedUnifiedTabOrder = [...s.unifiedTabOrder, newTabRef]; + } + } else { + // No active file tab or not specified - append at end + updatedUnifiedTabOrder = [...s.unifiedTabOrder, newTabRef]; + } + return { ...s, filePreviewTabs: [...s.filePreviewTabs, newFileTab], - unifiedTabOrder: [...s.unifiedTabOrder, newTabRef], + unifiedTabOrder: updatedUnifiedTabOrder, activeFileTabId: newTabId, // Deselect AI tab when file tab becomes active // Note: activeTabId stays as is - it tracks the last active AI tab for when user switches back @@ -6431,43 +6497,51 @@ You are taking over this conversation. Based on the context above, provide a bri // PERF: Memoized callbacks for MainPanel file preview navigation // These were inline arrow functions causing MainPanel re-renders on every keystroke // Updated to use file tabs (handleOpenFileTab) instead of legacy preview overlay - const handleMainPanelFileClick = useCallback(async (relativePath: string) => { - const currentSession = sessionsRef.current.find((s) => s.id === activeSessionIdRef.current); - if (!currentSession) return; - const filename = relativePath.split('/').pop() || relativePath; + const handleMainPanelFileClick = useCallback( + async (relativePath: string, options?: { openInNewTab?: boolean }) => { + const currentSession = sessionsRef.current.find((s) => s.id === activeSessionIdRef.current); + if (!currentSession) return; + const filename = relativePath.split('/').pop() || relativePath; - // Get SSH remote ID - const sshRemoteId = - currentSession.sshRemoteId || currentSession.sessionSshRemoteConfig?.remoteId || undefined; + // Get SSH remote ID + const sshRemoteId = + currentSession.sshRemoteId || currentSession.sessionSshRemoteConfig?.remoteId || undefined; - // Check if file should be opened externally (PDF, etc.) - if (!sshRemoteId && shouldOpenExternally(filename)) { - const fullPath = `${currentSession.fullPath}/${relativePath}`; - window.maestro.shell.openExternal(`file://${fullPath}`); - return; - } + // Check if file should be opened externally (PDF, etc.) + if (!sshRemoteId && shouldOpenExternally(filename)) { + const fullPath = `${currentSession.fullPath}/${relativePath}`; + window.maestro.shell.openExternal(`file://${fullPath}`); + return; + } - try { - const fullPath = `${currentSession.fullPath}/${relativePath}`; - // Fetch content and stat in parallel for efficiency - const [content, stat] = await Promise.all([ - window.maestro.fs.readFile(fullPath, sshRemoteId), - window.maestro.fs.stat(fullPath, sshRemoteId).catch(() => null), // stat is optional, don't fail if unavailable - ]); - const lastModified = stat?.modifiedAt ? new Date(stat.modifiedAt).getTime() : undefined; - // Open file in a tab (or select existing tab if already open) - handleOpenFileTab({ - path: fullPath, - name: filename, - content, - sshRemoteId, - lastModified, - }); - setActiveFocus('main'); - } catch (error) { - console.error('[onFileClick] Failed to read file:', error); - } - }, [handleOpenFileTab]); + try { + const fullPath = `${currentSession.fullPath}/${relativePath}`; + // Fetch content and stat in parallel for efficiency + const [content, stat] = await Promise.all([ + window.maestro.fs.readFile(fullPath, sshRemoteId), + window.maestro.fs.stat(fullPath, sshRemoteId).catch(() => null), // stat is optional, don't fail if unavailable + ]); + const lastModified = stat?.modifiedAt ? new Date(stat.modifiedAt).getTime() : undefined; + // Open file in a tab: + // - openInNewTab=true (Cmd/Ctrl+Click): create new tab adjacent to current + // - openInNewTab=false (regular click): replace current tab content + handleOpenFileTab( + { + path: fullPath, + name: filename, + content, + sshRemoteId, + lastModified, + }, + { openInNewTab: options?.openInNewTab ?? false } // Default to replacing current tab for in-content links + ); + setActiveFocus('main'); + } catch (error) { + console.error('[onFileClick] Failed to read file:', error); + } + }, + [handleOpenFileTab] + ); const handleNavigateBack = useCallback(() => { const currentSession = sessionsRef.current.find((s) => s.id === activeSessionIdRef.current); diff --git a/src/renderer/components/FilePreview.tsx b/src/renderer/components/FilePreview.tsx index 5d67217a..d9f15eef 100644 --- a/src/renderer/components/FilePreview.tsx +++ b/src/renderer/components/FilePreview.tsx @@ -84,8 +84,12 @@ interface FilePreviewProps { fileTree?: FileNode[]; /** Current working directory for proximity-based matching */ cwd?: string; - /** Callback when a file link is clicked */ - onFileClick?: (path: string) => void; + /** Callback when a file link is clicked + * @param path - The file path to open + * @param options - Options for how to open the file + * @param options.openInNewTab - If true, open in a new tab adjacent to current; if false, replace current tab content + */ + onFileClick?: (path: string, options?: { openInNewTab?: boolean }) => void; /** Whether back navigation is available */ canGoBack?: boolean; /** Whether forward navigation is available */ @@ -814,7 +818,9 @@ export const FilePreview = forwardRef(funct onClick={(e) => { e.preventDefault(); if (isMaestroFile && filePath && onFileClick) { - onFileClick(filePath); + // Cmd/Ctrl+Click opens in new tab, regular click replaces current tab + const openInNewTab = e.metaKey || e.ctrlKey; + onFileClick(filePath, { openInNewTab }); } else if (isAnchorLink && anchorId) { // Handle anchor links - scroll to the target element const targetElement = markdownContainerRef.current diff --git a/src/renderer/components/MainPanel.tsx b/src/renderer/components/MainPanel.tsx index 1af6b161..9cec4f73 100644 --- a/src/renderer/components/MainPanel.tsx +++ b/src/renderer/components/MainPanel.tsx @@ -236,7 +236,8 @@ interface MainPanelProps { // File tree for linking file references in AI responses fileTree?: import('../types/fileTree').FileNode[]; // Callback when a file link is clicked in AI response - onFileClick?: (relativePath: string) => void; + // options.openInNewTab: true = open in new tab adjacent to current, false = replace current tab content + onFileClick?: (relativePath: string, options?: { openInNewTab?: boolean }) => void; // File preview navigation canGoBack?: boolean; canGoForward?: boolean;