fix(desktop): give preview error states a Close that closes the tab
A preview tab whose file was moved or deleted rendered "Preview unavailable" with nothing to click, so the only way out was the strip's close glyph. The file read/PDF error states and the missing-artifact state now offer a Close button wired to the tile's own closeTabPane, the same close the strip and Cmd+W run. Adapts the onClose threading from #93194. Co-authored-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
This commit is contained in:
74
apps/desktop/src/app/chat/preview-error-close.test.tsx
Normal file
74
apps/desktop/src/app/chat/preview-error-close.test.tsx
Normal file
@@ -0,0 +1,74 @@
|
||||
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { registry } from '@/contrib/registry'
|
||||
import { $previewTabs, closeRightRail, openPreview, previewTabId } from '@/store/preview'
|
||||
import { $connection } from '@/store/session'
|
||||
|
||||
import { watchPreviewTiles } from './preview-tile'
|
||||
|
||||
vi.mock('./right-rail/real-profile-consent-dialog', () => ({
|
||||
RealProfileConsentDialog: () => null
|
||||
}))
|
||||
|
||||
// #87411: a persisted preview tab pointing at a deleted file rendered
|
||||
// "Preview unavailable" with nothing to click. The body's Close must be the
|
||||
// tab's own Close — the same verb the strip's ✕ and ⌘W run.
|
||||
|
||||
const target = {
|
||||
kind: 'file' as const,
|
||||
label: 'report.csv',
|
||||
path: '/tmp/gone/report.csv',
|
||||
previewKind: 'text' as const,
|
||||
source: '/tmp/gone/report.csv',
|
||||
url: 'file:///tmp/gone/report.csv'
|
||||
}
|
||||
|
||||
const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] }
|
||||
|
||||
beforeAll(() => {
|
||||
watchPreviewTiles()
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
$connection.set({ mode: 'local' } as never)
|
||||
desktopWindow.hermesDesktop = {
|
||||
readFileText: vi.fn(async () => {
|
||||
throw new Error('Text preview failed: file does not exist.')
|
||||
})
|
||||
} as unknown as Window['hermesDesktop']
|
||||
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) =>
|
||||
window.setTimeout(() => callback(Date.now()), 0)
|
||||
)
|
||||
vi.stubGlobal('cancelAnimationFrame', (id: number) => window.clearTimeout(id))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
closeRightRail()
|
||||
$connection.set(null)
|
||||
delete desktopWindow.hermesDesktop
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
describe('a preview whose file is gone', () => {
|
||||
it('offers Close in the error state, and it closes the tab', async () => {
|
||||
openPreview(target)
|
||||
|
||||
const tabId = previewTabId(target)
|
||||
const pane = registry.getArea('panes').find(contribution => contribution.id === `preview-tile:${tabId}`)
|
||||
|
||||
expect(pane?.render).toBeTypeOf('function')
|
||||
|
||||
await act(async () => {
|
||||
render(<>{pane!.render!()}</>)
|
||||
})
|
||||
|
||||
expect(await screen.findByText('Preview unavailable')).toBeTruthy()
|
||||
expect($previewTabs.get().map(tab => tab.id)).toEqual([tabId])
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Close' }))
|
||||
|
||||
expect($previewTabs.get()).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -13,7 +13,13 @@
|
||||
import { useStore } from '@nanostores/react'
|
||||
|
||||
import { findGroup } from '@/components/pane-shell/tree/model'
|
||||
import { $activeTreeGroup, $layoutTree, revealTreePane, treePanesWithPrefix } from '@/components/pane-shell/tree/store'
|
||||
import {
|
||||
$activeTreeGroup,
|
||||
$layoutTree,
|
||||
closeTabPane,
|
||||
revealTreePane,
|
||||
treePanesWithPrefix
|
||||
} from '@/components/pane-shell/tree/store'
|
||||
import { type MenuKit, renderActionItem } from '@/components/ui/actions-menu'
|
||||
import { FileTypeIcon } from '@/components/ui/file-type-icon'
|
||||
import { ToolIcon } from '@/components/ui/tool-icon'
|
||||
@@ -282,7 +288,8 @@ const watchPreviewTileMirror = paneMirror<{ id: string }>({
|
||||
|
||||
return target?.kind === 'url' || target?.previewKind === 'html'
|
||||
},
|
||||
render: tabId => <PreviewTilePane tabId={tabId} />,
|
||||
// The body's own Close (an error state's way out) is the tab's ✕, verbatim.
|
||||
render: tabId => <PreviewTilePane onClose={() => closeTabPane(previewPaneId(tabId))} tabId={tabId} />,
|
||||
close: tabId => {
|
||||
forgetBrowserPage(tabId)
|
||||
forgetPreviewConsole(tabId)
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { act, cleanup, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { $artifactRegistry, $artifactVersionSelection, artifactPreviewTarget, upsertArtifact } from '@/store/artifacts'
|
||||
|
||||
@@ -109,4 +109,21 @@ describe('ArtifactPreview', () => {
|
||||
|
||||
expect(screen.queryByTitle('Dashboard')).toBeNull()
|
||||
})
|
||||
|
||||
it('gives the missing-artifact state a Close that closes its tab', async () => {
|
||||
const { artifactId } = register('Dashboard', 'html', '<h1>gone</h1>')
|
||||
const record = $artifactRegistry.get()['session-1']!.find(item => item.id === artifactId)!
|
||||
const target = artifactPreviewTarget(record)
|
||||
const onClose = vi.fn()
|
||||
|
||||
$artifactRegistry.set({})
|
||||
|
||||
await act(async () => {
|
||||
render(<ArtifactPreview onClose={onClose} target={target} />)
|
||||
})
|
||||
|
||||
expect(screen.getByText('Artifact unavailable')).toBeTruthy()
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Close' }))
|
||||
expect(onClose).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -172,7 +172,7 @@ function VersionStepper({
|
||||
* The target only carries the artifact id — content is read live from the
|
||||
* registry, so an open tab picks up new versions as the model iterates.
|
||||
*/
|
||||
export function ArtifactPreview({ target }: { target: PreviewTarget }) {
|
||||
export function ArtifactPreview({ onClose, target }: { onClose?: () => void; target: PreviewTarget }) {
|
||||
const { t } = useI18n()
|
||||
const copy = t.artifactPreview
|
||||
const artifactId = target.url
|
||||
@@ -188,7 +188,13 @@ export function ArtifactPreview({ target }: { target: PreviewTarget }) {
|
||||
const record = useMemo(() => findArtifact(registry, artifactId), [artifactId, registry])
|
||||
|
||||
if (!record) {
|
||||
return <PreviewEmptyState body={copy.missingBody} title={copy.missingTitle} />
|
||||
return (
|
||||
<PreviewEmptyState
|
||||
body={copy.missingBody}
|
||||
primaryAction={onClose ? { label: t.common.close, onClick: onClose } : undefined}
|
||||
title={copy.missingTitle}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
const renderable = record.kind === 'html' || record.kind === 'svg'
|
||||
|
||||
@@ -729,10 +729,13 @@ export function SourceView({ filePath, language, text }: { filePath?: string; la
|
||||
export type PreviewViewMode = 'diff' | 'rendered' | 'source'
|
||||
|
||||
export function LocalFilePreview({
|
||||
onClose,
|
||||
onSelectRendered,
|
||||
reloadKey,
|
||||
target
|
||||
}: {
|
||||
/** Closes the preview's tab; offered when the file can't be shown. */
|
||||
onClose?: () => void
|
||||
/** Present when the pane can render this file live (HTML). Adds the
|
||||
* `rendered` mode to the switcher and routes its selection to the pane. */
|
||||
onSelectRendered?: () => void
|
||||
@@ -1095,12 +1098,16 @@ export function LocalFilePreview({
|
||||
return <PageLoader label={t.preview.loading} />
|
||||
}
|
||||
|
||||
// A preview that can't load (the file was moved or deleted) is a dead end,
|
||||
// so it carries its own way out rather than leaving it to the tab strip.
|
||||
const closeAction = onClose ? { label: t.common.close, onClick: onClose } : undefined
|
||||
|
||||
if (state.error) {
|
||||
return <PreviewEmptyState body={state.error} title={t.preview.unavailable} />
|
||||
return <PreviewEmptyState body={state.error} primaryAction={closeAction} title={t.preview.unavailable} />
|
||||
}
|
||||
|
||||
if (pdfError) {
|
||||
return <PreviewEmptyState body={pdfError} title={t.preview.unavailable} />
|
||||
return <PreviewEmptyState body={pdfError} primaryAction={closeAction} title={t.preview.unavailable} />
|
||||
}
|
||||
|
||||
if (
|
||||
|
||||
@@ -135,6 +135,9 @@ interface GuestContextMenuParams {
|
||||
|
||||
interface PreviewPaneProps {
|
||||
embedded?: boolean
|
||||
/** Closes this preview's tab. Offered by body states that are a dead end
|
||||
* (a file that no longer exists) so the way out is not only the strip. */
|
||||
onClose?: () => void
|
||||
onRestartServer?: (url: string, context?: string) => Promise<string>
|
||||
reloadRequest?: number
|
||||
/** The preview tab this pane renders. Keys the per-tab console store the
|
||||
@@ -237,7 +240,14 @@ function PreviewLoadError({
|
||||
)
|
||||
}
|
||||
|
||||
export function PreviewPane({ embedded = false, onRestartServer, reloadRequest = 0, tabId, target }: PreviewPaneProps) {
|
||||
export function PreviewPane({
|
||||
embedded = false,
|
||||
onClose,
|
||||
onRestartServer,
|
||||
reloadRequest = 0,
|
||||
tabId,
|
||||
target
|
||||
}: PreviewPaneProps) {
|
||||
const { t } = useI18n()
|
||||
const copy = t.preview.web
|
||||
// The console store belongs to the TAB, not this render: the toggles live on
|
||||
@@ -1391,9 +1401,10 @@ export function PreviewPane({ embedded = false, onRestartServer, reloadRequest =
|
||||
)}
|
||||
{!isWebPreview &&
|
||||
(target.kind === 'artifact' ? (
|
||||
<ArtifactPreview target={target} />
|
||||
<ArtifactPreview onClose={onClose} target={target} />
|
||||
) : (
|
||||
<LocalFilePreview
|
||||
onClose={onClose}
|
||||
onSelectRendered={canRenderHtmlFile ? () => selectRenderMode('preview') : undefined}
|
||||
reloadKey={localReloadKey}
|
||||
target={target}
|
||||
|
||||
@@ -6,6 +6,8 @@ import { $previewReloadRequest, $previewTabs } from '@/store/preview'
|
||||
import { PreviewPane } from './preview-pane'
|
||||
|
||||
interface PreviewTilePaneProps {
|
||||
/** The tab's own Close, for body states that offer one (a failed load). */
|
||||
onClose?: () => void
|
||||
/** The `$previewTabs` id this pane renders. */
|
||||
tabId: string
|
||||
}
|
||||
@@ -20,7 +22,7 @@ interface PreviewTilePaneProps {
|
||||
* bridge the old rail wrapper used, since the mirror renders this pane with no
|
||||
* props to thread.
|
||||
*/
|
||||
export function PreviewTilePane({ tabId }: PreviewTilePaneProps) {
|
||||
export function PreviewTilePane({ onClose, tabId }: PreviewTilePaneProps) {
|
||||
const previewReloadRequest = useStore($previewReloadRequest)
|
||||
const previewTabs = useStore($previewTabs)
|
||||
const restartPreviewServer = useStore($restartPreviewServer)
|
||||
@@ -35,6 +37,7 @@ export function PreviewTilePane({ tabId }: PreviewTilePaneProps) {
|
||||
return (
|
||||
<PreviewPane
|
||||
embedded
|
||||
onClose={onClose}
|
||||
onRestartServer={target.kind === 'url' ? (restartPreviewServer ?? undefined) : undefined}
|
||||
reloadRequest={previewReloadRequest}
|
||||
tabId={tabId}
|
||||
|
||||
Reference in New Issue
Block a user