From 96999b116b5efcbdfbe4e84b79ef28b4fccd045a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 17:55:13 -0500 Subject: [PATCH] refactor(desktop): put every preview on one rail tab list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The right rail held two things at once: a list of file tabs, and a privileged "live preview" slot with a hardcoded `preview` tab id backed by a separate session-keyed registry. The two were written under different session-id rules and reconciled against each other, so an `open_preview` from a session whose stored id hadn't landed yet was set and then immediately cleared — the pane flashed and vanished. Artifacts arrived as a third list with their own pane and renderers. Now everything the rail can show is a `PreviewTarget` in `$previewTabs`, and `openPreview` is the only way in. `$previewTarget` is a computed read of the active tab, the session registry and its reconciler are gone, and artifacts render in the real preview pane through the shared mode switcher and source view instead of a parallel one. Artifact tabs stay memory-only since the registry rebuilds from the transcript. --- apps/desktop/src/app/chat/close-tab.test.ts | 41 +- apps/desktop/src/app/chat/close-tab.ts | 13 +- .../composer/status-stack/preview-row.tsx | 10 +- .../src/app/chat/right-rail/artifact-pane.tsx | 222 ------- .../chat/right-rail/artifact-renderers.tsx | 107 --- .../chat/right-rail/preview-artifact.test.tsx | 112 ++++ .../app/chat/right-rail/preview-artifact.tsx | 263 ++++++++ .../src/app/chat/right-rail/preview-file.tsx | 28 +- .../src/app/chat/right-rail/preview-pane.tsx | 16 +- .../src/app/chat/right-rail/preview.tsx | 93 +-- apps/desktop/src/app/contrib/controller.tsx | 27 +- apps/desktop/src/app/contrib/panes.tsx | 9 +- apps/desktop/src/app/contrib/wiring.tsx | 11 +- apps/desktop/src/app/right-sidebar/index.tsx | 4 +- .../app/right-sidebar/review/file-tree.tsx | 4 +- .../terminal/use-terminal-session.ts | 4 +- .../app/session/hooks/preview-open.test.tsx | 177 +++++ .../hooks/use-preview-routing.test.tsx | 184 ------ .../app/session/hooks/use-preview-routing.ts | 75 +-- .../components/assistant-ui/artifact-card.tsx | 17 +- .../markdown-text.artifacts.test.tsx | 5 +- apps/desktop/src/i18n/ar.ts | 5 +- apps/desktop/src/i18n/en.ts | 5 +- apps/desktop/src/i18n/ja.ts | 5 +- apps/desktop/src/i18n/types.ts | 5 +- apps/desktop/src/i18n/zh-hant.ts | 5 +- apps/desktop/src/i18n/zh.ts | 5 +- apps/desktop/src/store/artifacts.test.ts | 78 ++- apps/desktop/src/store/artifacts.ts | 227 +------ apps/desktop/src/store/gateway-switch.ts | 8 +- apps/desktop/src/store/layout.ts | 20 +- apps/desktop/src/store/preview.test.ts | 162 +++-- apps/desktop/src/store/preview.ts | 622 +++++------------- 33 files changed, 1000 insertions(+), 1569 deletions(-) delete mode 100644 apps/desktop/src/app/chat/right-rail/artifact-pane.tsx delete mode 100644 apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx create mode 100644 apps/desktop/src/app/chat/right-rail/preview-artifact.test.tsx create mode 100644 apps/desktop/src/app/chat/right-rail/preview-artifact.tsx create mode 100644 apps/desktop/src/app/session/hooks/preview-open.test.tsx delete mode 100644 apps/desktop/src/app/session/hooks/use-preview-routing.test.tsx diff --git a/apps/desktop/src/app/chat/close-tab.test.ts b/apps/desktop/src/app/chat/close-tab.test.ts index 95847fd925c..44368c80403 100644 --- a/apps/desktop/src/app/chat/close-tab.test.ts +++ b/apps/desktop/src/app/chat/close-tab.test.ts @@ -1,14 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { $rightRailActiveTabId, RIGHT_RAIL_PREVIEW_TAB_ID } from '@/store/layout' -import { - $filePreviewTabs, - $previewTarget, - clearSessionPreviewRegistry, - type PreviewTarget, - setCurrentSessionPreviewTarget -} from '@/store/preview' -import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' +import { $rightRailActiveTabId } from '@/store/layout' +import { $previewTabs, closeRightRail, openPreview, type PreviewTarget } from '@/store/preview' import { closeActiveTab } from './close-tab' @@ -26,40 +19,34 @@ function fileTarget(path: string): PreviewTarget { describe('closeActiveTab', () => { beforeEach(() => { vi.stubGlobal('document', { activeElement: null }) - $activeSessionId.set('session-1') - $selectedStoredSessionId.set(null) + closeRightRail() window.localStorage.clear() - clearSessionPreviewRegistry() }) afterEach(() => { vi.unstubAllGlobals() - $activeSessionId.set(null) - $selectedStoredSessionId.set(null) - clearSessionPreviewRegistry() + closeRightRail() window.localStorage.clear() }) it('closes the active file preview tab (⌘W happy path)', () => { - setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') + openPreview(fileTarget('/work/notes.md'), 'manual') - expect($filePreviewTabs.get()).toHaveLength(1) + expect($previewTabs.get()).toHaveLength(1) expect($rightRailActiveTabId.get()).toBe('file:file:///work/notes.md') expect(closeActiveTab()).toBe(true) - expect($filePreviewTabs.get()).toHaveLength(0) + expect($previewTabs.get()).toHaveLength(0) }) - it('closes the visible file tab when active selection is a ghost preview', () => { - // Active tab id stuck on live-preview after that target was cleared, while - // file tabs remain (UI falls back to tabs[0] until React syncs). ⌘W must - // close the visible file tab instead of no-op'ing via closeWorkspaceTab(). - setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') - $previewTarget.set(null) - $rightRailActiveTabId.set(RIGHT_RAIL_PREVIEW_TAB_ID) + it('closes the visible tab when the active selection points at a tab that is gone', () => { + // The rail falls back to tabs[0] until React syncs the selection, so ⌘W has + // to act on what is actually on screen rather than no-op'ing. + openPreview(fileTarget('/work/notes.md'), 'manual') + $rightRailActiveTabId.set('file:file:///work/stale.md') - expect($filePreviewTabs.get()).toHaveLength(1) + expect($previewTabs.get()).toHaveLength(1) expect(closeActiveTab()).toBe(true) - expect($filePreviewTabs.get()).toHaveLength(0) + expect($previewTabs.get()).toHaveLength(0) }) }) diff --git a/apps/desktop/src/app/chat/close-tab.ts b/apps/desktop/src/app/chat/close-tab.ts index b5e15dd30cf..bf8b2899870 100644 --- a/apps/desktop/src/app/chat/close-tab.ts +++ b/apps/desktop/src/app/chat/close-tab.ts @@ -1,8 +1,7 @@ import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals' import { closeWorkspaceTab } from '@/components/pane-shell/tree/store' import { isFocusWithin } from '@/lib/keybinds/combo' -import { $artifactTabs } from '@/store/artifacts' -import { $filePreviewTabs, $previewTarget, closeActiveRightRailTab } from '@/store/preview' +import { $previewTabs, closeActiveRightRailTab } from '@/store/preview' import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-states' /** @@ -28,12 +27,10 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri return true } - // Prefer tab *presence* over the derived active file target. After the live - // preview is cleared, `$rightRailActiveTabId` can stay on `preview` while - // file tabs remain (the rail UI falls back to tabs[0]). Gating only on - // `$filePreviewTarget` made ⌘W fall through to closeWorkspaceTab() and look - // broken with a file tab still on screen. - if ($previewTarget.get() || $filePreviewTabs.get().length > 0 || $artifactTabs.get().length > 0) { + // Gate on tab *presence*, not on the selection: a stale `$rightRailActiveTabId` + // would otherwise make ⌘W fall through to closeWorkspaceTab() and look broken + // with a tab still on screen. The store resolves which tab that is. + if ($previewTabs.get().length > 0) { return closeActiveRightRailTab() } diff --git a/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx b/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx index cf721d2ae93..dc40c31de2e 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx @@ -11,7 +11,7 @@ import { cn } from '@/lib/utils' import { PREVIEW_PANE_ID } from '@/store/layout' import { notifyError } from '@/store/notifications' import { $paneOpen } from '@/store/panes' -import { $previewTarget, dismissPreviewTarget, setCurrentSessionPreviewTarget } from '@/store/preview' +import { $previewTabSources, closePreviewForSource, openPreview } from '@/store/preview' import { type PreviewArtifact } from '@/store/preview-status' interface PreviewStatusRowProps { @@ -22,10 +22,10 @@ interface PreviewStatusRowProps { /** One detected artifact, single line, always visible: filename + open + close. */ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss }: PreviewStatusRowProps) { const { t } = useI18n() - const activePreview = useStore($previewTarget) + const openSources = useStore($previewTabSources) const previewPaneOpen = useStore($paneOpen(PREVIEW_PANE_ID)) const [opening, setOpening] = useState(false) - const isOpen = activePreview?.source === item.target && previewPaneOpen + const isOpen = openSources.includes(item.target) && previewPaneOpen const resolveTarget = async () => { const target = await normalizeOrLocalPreviewTarget(item.target, item.cwd || undefined) @@ -43,7 +43,7 @@ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss } if (isOpen) { - dismissPreviewTarget() + closePreviewForSource(item.target) return } @@ -51,7 +51,7 @@ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss setOpening(true) try { - setCurrentSessionPreviewTarget(await resolveTarget(), 'tool-result', item.target) + openPreview(await resolveTarget(), 'tool-result') } catch (error) { notifyError(error, t.preview.unavailable) } finally { diff --git a/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx b/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx deleted file mode 100644 index c463ee84c41..00000000000 --- a/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx +++ /dev/null @@ -1,222 +0,0 @@ -import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useState } from 'react' - -import { CopyButton } from '@/components/ui/copy-button' -import { Tip } from '@/components/ui/tooltip' -import { useI18n } from '@/i18n' -import { artifactDownloadName } from '@/lib/artifact-detect' -import { downloadTextFile } from '@/lib/download-text' -import { ChevronLeft, ChevronRight, Download, ExternalLink } from '@/lib/icons' -import { cn } from '@/lib/utils' -import { - $artifactRegistry, - $artifactVersionSelection, - type ArtifactRecord, - selectArtifactVersion -} from '@/store/artifacts' -import { notifyError } from '@/store/notifications' - -import { ArtifactLivePreview, ArtifactSourceView, composeArtifactHtml } from './artifact-renderers' -import { PreviewEmptyState } from './preview-file' - -type ArtifactViewMode = 'preview' | 'source' - -const MIME_BY_KIND = { code: 'text/plain', html: 'text/html', svg: 'image/svg+xml' } as const - -const HEADER_BUTTON_CLASS = - 'flex h-5 items-center gap-1 rounded-md px-1 text-[0.625rem] font-bold text-muted-foreground transition-colors hover:bg-accent hover:text-foreground disabled:pointer-events-none disabled:opacity-40' - -/** Write the composed document to a real temp file through the existing - * buffer-save IPC, then hand it to the OS browser. A blob/data URL can't - * cross into the OS default browser, so a file on disk is the honest path. */ -async function openHtmlInBrowser(content: string): Promise { - const bridge = window.hermesDesktop - - if (!bridge?.saveImageBuffer || !bridge.openExternal) { - throw new Error('Desktop bridge unavailable') - } - - const bytes = new TextEncoder().encode(composeArtifactHtml(content)) - const path = await bridge.saveImageBuffer(bytes, '.html') - - if (!path) { - throw new Error('Could not write artifact file') - } - - const fileUrl = `file://${path.startsWith('/') ? '' : '/'}${path.replace(/\\/g, '/')}` - - if (bridge.openPreviewInBrowser) { - await bridge.openPreviewInBrowser(fileUrl) - - return - } - - await bridge.openExternal(fileUrl) -} - -function VersionStepper({ - current, - onSelect, - total -}: { - current: number - onSelect: (index: number) => void - total: number -}) { - const { t } = useI18n() - const copy = t.artifactPane - - if (total < 2) { - return null - } - - return ( -
- - - - {copy.versionOf(current + 1, total)} - - - -
- ) -} - -export function ArtifactPane({ artifactId }: { artifactId: string }) { - const { t } = useI18n() - const copy = t.artifactPane - const registry = useStore($artifactRegistry) - const versionSelection = useStore($artifactVersionSelection) - // View mode is per-pane, ephemeral: renderable artifacts open in preview. - const [userMode, setUserMode] = useState(null) - - // Reset the explicit mode when the pane is reused for another artifact. - useEffect(() => { - setUserMode(null) - }, [artifactId]) - - const record = useMemo(() => { - for (const records of Object.values(registry)) { - const found = records.find(candidate => candidate.id === artifactId) - - if (found) { - return found - } - } - - return null - }, [artifactId, registry]) - - if (!record) { - return - } - - const isRenderable = record.kind === 'html' || record.kind === 'svg' - const versionIndex = Math.min(versionSelection[artifactId] ?? record.versions.length - 1, record.versions.length - 1) - const version = record.versions[versionIndex]! - const isCurrentVersion = versionIndex >= record.versions.length - 1 - const mode: ArtifactViewMode = isRenderable ? (userMode ?? 'preview') : 'source' - const downloadName = artifactDownloadName(record.kind, record.language, record.title) - - const modeLabel: Record = { - preview: copy.modePreview, - source: copy.modeSource - } - - return ( -
-
-
- selectArtifactVersion(artifactId, index)} - total={record.versions.length} - /> - {!isCurrentVersion && ( - - )} -
- {isRenderable && - (['preview', 'source'] as const).map(candidate => ( - - ))} -
- - - - - {record.kind === 'html' && window.hermesDesktop && ( - - - - )} -
-
-
- {mode === 'preview' && isRenderable ? ( - - ) : ( - - )} -
-
- ) -} diff --git a/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx b/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx deleted file mode 100644 index 00ed9a58a58..00000000000 --- a/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx +++ /dev/null @@ -1,107 +0,0 @@ -import DOMPurify from 'dompurify' -import { useMemo } from 'react' -import ShikiHighlighter from 'react-shiki' - -import { chunkTextLines, useFixedRowWindow } from '@/components/chat/fixed-row-window' -import type { ArtifactKind } from '@/lib/artifact-detect' - -const SHIKI_THEME = { dark: 'github-dark-default', light: 'github-light-default' } as const -const SOURCE_CHUNK_LINES = 200 -const SOURCE_LINE_PX = 20 -const SOURCE_OVERSCAN_LINES = 400 - -/** Windowed, Shiki-highlighted source view for artifact content. Same fixed-row - * windowing as the file preview's SourceView so a 5k-line artifact scrolls - * smoothly, minus the gutter drag/selection machinery (artifact content has no - * on-disk path to reference lines against). */ -export function ArtifactSourceView({ language, text }: { language: string; text: string }) { - const chunks = useMemo(() => chunkTextLines(text, SOURCE_CHUNK_LINES), [text]) - const lastChunk = chunks.at(-1) - const totalLines = lastChunk ? lastChunk.start + lastChunk.lines.length : 0 - - const { afterRows, beforeRows, endChunk, onScroll, scrollerRef, startChunk } = useFixedRowWindow({ - overscanRows: SOURCE_OVERSCAN_LINES, - rowPx: SOURCE_LINE_PX, - rowsPerChunk: SOURCE_CHUNK_LINES, - totalRows: totalLines - }) - - const visibleChunks = chunks.slice(startChunk, endChunk + 1) - - return ( -
-
- {beforeRows > 0 &&
} - {visibleChunks.map(chunk => ( -
- - {chunk.text} - -
- ))} - {afterRows > 0 &&
} -
-
- ) -} - -/** Wrap an HTML fragment in a minimal document shell; full documents pass - * through untouched. Keeps generated fragments (no /) rendering - * with sane defaults instead of quirks-mode soup. */ -export function composeArtifactHtml(content: string): string { - if (/]|', - '', - '', - content, - '' - ].join('\n') -} - -/** - * Sandboxed live renderer for html/svg artifact content. - * - * HTML runs in an `