From 9229b8b2dc0319688fcb763d1ca5ea3abe7619da Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 17 Sep 2026 23:09:45 -0700 Subject: [PATCH] fix(editor): drop the load-error Close action; closing stays with the tab strip The Close button routed through `requestEditorFileClose`, which skips the pinned-tab and shared-reference checks the tab strip applies, has no listener outside the Terminal workbench (floating editor panels), and on the conflict-review overview could target an unrelated open tab whose id is the same absolute path as a synthesized inline row. Rather than reimplement the tab strip's close semantics in a second place, the error view keeps Retry and its copy points the user at closing the tab. --- .../editor/EditorConflictReviewSurface.tsx | 5 ---- .../src/components/editor/EditorContent.tsx | 2 -- .../editor/EditorEditFileSurface.tsx | 2 -- .../editor/EditorFileLoadErrorView.test.tsx | 21 ++++---------- .../editor/EditorFileLoadErrorView.tsx | 29 ++++++------------- src/renderer/src/i18n/locales/en.json | 3 -- 6 files changed, 14 insertions(+), 48 deletions(-) diff --git a/src/renderer/src/components/editor/EditorConflictReviewSurface.tsx b/src/renderer/src/components/editor/EditorConflictReviewSurface.tsx index 4e8ac9a2bfd..fbff9caf22e 100644 --- a/src/renderer/src/components/editor/EditorConflictReviewSurface.tsx +++ b/src/renderer/src/components/editor/EditorConflictReviewSurface.tsx @@ -7,7 +7,6 @@ import type { GitStatusEntry } from '../../../../shared/git-status-types' import { ConflictBanner, ConflictPlaceholderView, ConflictReviewPanel } from './ConflictComponents' import { ImageViewer, MonacoEditor } from './editor-lazy-views' import { EditorFileLoadErrorView } from './EditorFileLoadErrorView' -import { requestEditorFileClose } from './editor-autosave' import type { FileContent } from './editor-panel-content-types' import { translate } from '@/i18n/i18n' import type { EditorConflictNavigation } from './useEditorConflictNavigation' @@ -128,16 +127,12 @@ export function EditorConflictReviewSurface({ ) } if (fileContent.loadError) { - // Why: inline overview rows are synthesized per entry and are not tabs, so the - // close queue would drop the request; only a real open tab gets a Close action. - const isOpenTab = openFiles.some((file) => file.id === contentFile.id) return (
reloadContent(contentFile)} - onClose={isOpenTab ? () => requestEditorFileClose(contentFile.id) : undefined} />
) diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index e30db431550..86527b25f29 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -8,7 +8,6 @@ import { EditorConflictReviewSurface } from './EditorConflictReviewSurface' import { EditorDiffFileSurface } from './EditorDiffFileSurface' import { EditorEditFileSurface } from './EditorEditFileSurface' import { EditorFileLoadErrorView } from './EditorFileLoadErrorView' -import { requestEditorFileClose } from './editor-autosave' import type { FileContent } from './editor-panel-content-types' import { translate } from '@/i18n/i18n' import { useEditorConflictNavigation } from './useEditorConflictNavigation' @@ -191,7 +190,6 @@ export function EditorContent({ message={fileContent.loadError} code={fileContent.loadErrorCode} onRetry={() => reloadContent(activeFile)} - onClose={() => requestEditorFileClose(activeFile.id)} /> ) } diff --git a/src/renderer/src/components/editor/EditorEditFileSurface.tsx b/src/renderer/src/components/editor/EditorEditFileSurface.tsx index bb559532217..a9e3f8ff856 100644 --- a/src/renderer/src/components/editor/EditorEditFileSurface.tsx +++ b/src/renderer/src/components/editor/EditorEditFileSurface.tsx @@ -13,7 +13,6 @@ import { } from './editor-lazy-views' import type { EditorConflictNavigation } from './useEditorConflictNavigation' import { EditorFileLoadErrorView } from './EditorFileLoadErrorView' -import { requestEditorFileClose } from './editor-autosave' import type { FileContent } from './editor-panel-content-types' import { ExternalFileChangeBanner } from './ExternalFileChangeBanner' import type { useMarkdownDocuments } from './useMarkdownDocuments' @@ -104,7 +103,6 @@ export function EditorEditFileSurface({ message={fileContent.loadError} code={fileContent.loadErrorCode} onRetry={() => reloadContent(activeFile)} - onClose={() => requestEditorFileClose(activeFile.id)} /> ) } diff --git a/src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsx b/src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsx index 5f73a2494fa..2d0d08fdfbe 100644 --- a/src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsx +++ b/src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsx @@ -10,28 +10,17 @@ import { describe('EditorFileLoadErrorView', () => { afterEach(cleanup) - it('offers Close only when the caller can route it, and never closes on its own', () => { + it('offers Retry as its only action', () => { + // Why: closing must stay with the tab strip, whose path carries the pin, shared- + // reference, and unsaved-changes checks; a second close control here would not. const onRetry = vi.fn() - const onClose = vi.fn() - render( - - ) + render() screen.getByText('selector_not_found') + expect(screen.getAllByRole('button')).toHaveLength(1) fireEvent.click(screen.getByRole('button', { name: 'Retry' })) expect(onRetry).toHaveBeenCalledOnce() - expect(onClose).not.toHaveBeenCalled() - - fireEvent.click(screen.getByRole('button', { name: 'Close tab' })) - expect(onClose).toHaveBeenCalledOnce() - }) - - it('renders no Close action without a handler', () => { - render() - - screen.getByRole('button', { name: 'Retry' }) - expect(screen.queryByRole('button', { name: 'Close tab' })).toBeNull() }) it('localizes the host-unresolved state by its sentinel code, not by the stored text', () => { diff --git a/src/renderer/src/components/editor/EditorFileLoadErrorView.tsx b/src/renderer/src/components/editor/EditorFileLoadErrorView.tsx index 6ad418029f8..0ab91c5ebe5 100644 --- a/src/renderer/src/components/editor/EditorFileLoadErrorView.tsx +++ b/src/renderer/src/components/editor/EditorFileLoadErrorView.tsx @@ -16,19 +16,18 @@ function localizeFileLoadError(message: string, code: string | undefined): strin return message } +// Why no Close action here: this view renders for real tabs and for synthesized inline +// conflict rows alike, and only the tab strip's own close path carries the pin, shared- +// reference, and unsaved-changes semantics. The copy points the user at that path instead +// of adding a second one that would have to reimplement it (#21041). export function EditorFileLoadErrorView({ message, code, - onRetry, - onClose + onRetry }: { message: string code?: string onRetry: () => void - // Why: a tab whose read reached a terminal state must not vanish on its own — the user - // decides whether to keep retrying or close it (#21041). Callers route the close through - // the unsaved-changes queue so a dirty draft is still confirmed, never silently dropped. - onClose?: () => void }): React.JSX.Element { return (
@@ -39,20 +38,10 @@ export function EditorFileLoadErrorView({ {translate('auto.components.editor.EditorContent.39f018b052', 'Unable to load file')}
{localizeFileLoadError(message, code)}
-
- - {onClose ? ( - - ) : null} -
+ diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 201fa8add71..6292a9f74f8 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -15344,9 +15344,6 @@ "a0af0198aa": "Large diffs are not rendered by default.", "c3d9f4a712": "This diff's size isn't known yet, so it loads on request.", "f7fa7a40d0": "Load diff" - }, - "EditorFileLoadErrorView": { - "296b59cd29": "Close tab" } }, "diff": {