From 6b0291f5f81f9d9636d3fcef168df8b9307b2bf5 Mon Sep 17 00:00:00 2001 From: hieptl Date: Sat, 30 May 2026 11:21:13 +0700 Subject: [PATCH 1/3] fix: show clear preview unavailable message for office documents --- .../files-tab/file-content-viewer.test.tsx | 116 ++++++++++++++++++ .../files-tab/file-content-viewer.tsx | 63 ++++++---- src/i18n/translation.json | 17 +++ 3 files changed, 172 insertions(+), 24 deletions(-) create mode 100644 __tests__/components/features/files-tab/file-content-viewer.test.tsx diff --git a/__tests__/components/features/files-tab/file-content-viewer.test.tsx b/__tests__/components/features/files-tab/file-content-viewer.test.tsx new file mode 100644 index 000000000..eb1f6ca01 --- /dev/null +++ b/__tests__/components/features/files-tab/file-content-viewer.test.tsx @@ -0,0 +1,116 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { FileContentViewer } from "#/components/features/files-tab/file-content-viewer"; +import { useWorkspaceMutationCounter } from "#/stores/use-workspace-mutation-counter"; + +// Mock the *services* the file-content hook depends on — not the hook itself — +// so the real classification (text decoded, then flipped to binary on a NUL +// sniff) runs end to end through the viewer. +const useWorkspaceSessionMock = vi.fn(); +vi.mock("#/hooks/query/use-workspace-session", async (importOriginal) => { + const real = + await importOriginal< + typeof import("#/hooks/query/use-workspace-session") + >(); + return { + ...real, // keep the real joinWorkspaceUrl the hook builds its fetch URL with + useWorkspaceSession: () => useWorkspaceSessionMock(), + }; +}); + +const useActiveConversationMock = vi.fn(); +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => useActiveConversationMock(), +})); + +const useRuntimeIsReadyMock = vi.fn(); +vi.mock("#/hooks/use-runtime-is-ready", () => ({ + useRuntimeIsReady: () => useRuntimeIsReadyMock(), +})); + +const getActiveBackendMock = vi.fn(); +vi.mock("#/api/backend-registry/active-store", () => ({ + getActiveBackend: () => getActiveBackendMock(), +})); + +const downloadFileMock = vi.fn(); +vi.mock("#/api/runtime-service/agent-server-runtime-service", () => ({ + default: { + downloadFile: (...args: unknown[]) => downloadFileMock(...args), + }, +})); + +const fetchMock = vi.fn(); + +const BASE_URL = + "https://agent.example.com/api/conversations/conv-1/workspace/"; + +function renderViewer(path: string) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + , + ); +} + +describe("FileContentViewer", () => { + beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + fetchMock.mockReset(); + useWorkspaceSessionMock.mockReset(); + useActiveConversationMock.mockReset(); + useRuntimeIsReadyMock.mockReset(); + getActiveBackendMock.mockReset(); + downloadFileMock.mockReset(); + + useRuntimeIsReadyMock.mockReturnValue(true); + useActiveConversationMock.mockReturnValue({ + data: { + id: "conv-1", + conversation_url: "https://agent.example.com/api/conversations/conv-1", + session_api_key: "session-key", + }, + }); + useWorkspaceSessionMock.mockReturnValue({ + data: { baseUrl: BASE_URL }, + isLoading: false, + isError: false, + error: null, + }); + getActiveBackendMock.mockReturnValue({ + backend: { id: "local-1", kind: "local", host: "http://localhost:8000" }, + orgId: null, + }); + useWorkspaceMutationCounter.setState({ count: 0 }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("shows a clear unsupported-document message when previewing an Office file (.pptx)", async () => { + // Arrange: the workspace fileserver returns real .pptx bytes — a ZIP whose + // header carries a NUL, so the hook classifies the file as binary. + fetchMock.mockResolvedValue({ + ok: true, + status: 200, + arrayBuffer: () => + Promise.resolve(new Uint8Array([0x50, 0x4b, 0x03, 0x04, 0x00]).buffer), + }); + + // Act + renderViewer("demo.pptx"); + + // Assert: the format-aware "no preview" message replaces the generic binary + // fallback, so the pane is never blank and the limitation is explicit. + expect( + await screen.findByTestId("file-content-viewer-unsupported-document"), + ).toBeInTheDocument(); + }); +}); diff --git a/src/components/features/files-tab/file-content-viewer.tsx b/src/components/features/files-tab/file-content-viewer.tsx index 147218bb1..2a15f1135 100644 --- a/src/components/features/files-tab/file-content-viewer.tsx +++ b/src/components/features/files-tab/file-content-viewer.tsx @@ -18,11 +18,47 @@ interface FileContentViewerProps { const HTML_LIKE_EXTS = new Set(["html", "htm", "svg"]); const MARKDOWN_EXTS = new Set(["md", "markdown", "mdx"]); +// Office/document formats we can't preview inline. The label doubles as the +// allow-list (a present entry => Office doc) and feeds a clear, format-named +// "no preview" message instead of the generic binary fallback. +const OFFICE_DOCUMENT_LABELS: Record = { + pptx: "PowerPoint", + ppt: "PowerPoint", + docx: "Word", + doc: "Word", + xlsx: "Excel", + xls: "Excel", +}; + function getExtension(path: string): string { const idx = path.lastIndexOf("."); return idx === -1 ? "" : path.slice(idx + 1).toLowerCase(); } +/** + * Fallback shown when a file's bytes aren't previewable. Office documents + * (.pptx / .docx / .xlsx …) get a clear, format-named message; every other + * binary keeps the generic "binary file" string so the pane is never blank. + */ +function UnpreviewableFallback({ path }: { path: string }) { + const { t } = useTranslation("openhands"); + const documentLabel = OFFICE_DOCUMENT_LABELS[getExtension(path)]; + return ( +
+ {documentLabel + ? t(I18nKey.FILES$UNSUPPORTED_DOCUMENT, { type: documentLabel }) + : t(I18nKey.FILES$BINARY_FALLBACK)} +
+ ); +} + /** * Renders the contents of a single workspace file. In `rich` mode we point * an iframe / straight at the agent server's static workspace @@ -82,14 +118,7 @@ export function FileContentViewer({ path, viewMode }: FileContentViewerProps) { /> ); } - return ( -
- {t(I18nKey.FILES$BINARY_FALLBACK)} -
- ); + return ; } // ----- Rich mode: render HTML, markdown, images, PDFs from staticUrl. ---- @@ -129,14 +158,7 @@ export function FileContentViewer({ path, viewMode }: FileContentViewerProps) { } if (kind === "binary") { - return ( -
- {t(I18nKey.FILES$BINARY_FALLBACK)} -
- ); + return ; } // Text-like content. @@ -202,12 +224,5 @@ export function FileContentViewer({ path, viewMode }: FileContentViewerProps) { // Truly unknown / empty payload — show a fallback so the pane is never // blank. - return ( -
- {t(I18nKey.FILES$BINARY_FALLBACK)} -
- ); + return ; } diff --git a/src/i18n/translation.json b/src/i18n/translation.json index 53ad6c2ac..0aa0ad9f8 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -21606,6 +21606,23 @@ "uk": "Двійковий файл – попередній перегляд недоступний", "ca": "Fitxer binari – previsualització no disponible" }, + "FILES$UNSUPPORTED_DOCUMENT": { + "en": "Preview isn't available for {{type}} files.", + "ja": "{{type}} ファイルのプレビューは利用できません。", + "zh-CN": "{{type}} 文件无法预览。", + "zh-TW": "{{type}} 檔案無法預覽。", + "ko-KR": "{{type}} 파일은 미리 보기를 사용할 수 없습니다.", + "no": "Forhåndsvisning er ikke tilgjengelig for {{type}}-filer.", + "it": "L'anteprima non è disponibile per i file {{type}}.", + "pt": "A visualização não está disponível para arquivos {{type}}.", + "es": "La vista previa no está disponible para archivos {{type}}.", + "ar": "المعاينة غير متاحة لملفات {{type}}.", + "fr": "L'aperçu n'est pas disponible pour les fichiers {{type}}.", + "tr": "{{type}} dosyaları için önizleme kullanılamıyor.", + "de": "Für {{type}}-Dateien ist keine Vorschau verfügbar.", + "uk": "Попередній перегляд недоступний для файлів {{type}}.", + "ca": "La previsualització no està disponible per als fitxers {{type}}." + }, "FILES$LOADING_FILES": { "en": "Loading files…", "ja": "ファイルを読み込んでいます…", From b778b6e2e692b3975757fa67052757ebe6761f4d Mon Sep 17 00:00:00 2001 From: hieptl Date: Sat, 30 May 2026 11:51:26 +0700 Subject: [PATCH 2/3] refactor: update the code based on feedback --- .../files-tab/file-content-viewer.test.tsx | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/__tests__/components/features/files-tab/file-content-viewer.test.tsx b/__tests__/components/features/files-tab/file-content-viewer.test.tsx index eb1f6ca01..2cbb2362d 100644 --- a/__tests__/components/features/files-tab/file-content-viewer.test.tsx +++ b/__tests__/components/features/files-tab/file-content-viewer.test.tsx @@ -4,6 +4,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { FileContentViewer } from "#/components/features/files-tab/file-content-viewer"; +import type { ViewMode } from "#/components/features/files-tab/view-mode"; import { useWorkspaceMutationCounter } from "#/stores/use-workspace-mutation-counter"; // Mock the *services* the file-content hook depends on — not the hook itself — @@ -48,13 +49,13 @@ const fetchMock = vi.fn(); const BASE_URL = "https://agent.example.com/api/conversations/conv-1/workspace/"; -function renderViewer(path: string) { +function renderViewer(path: string, viewMode: ViewMode = "rich") { const client = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); return render( - + , ); } @@ -94,23 +95,31 @@ describe("FileContentViewer", () => { vi.unstubAllGlobals(); }); - it("shows a clear unsupported-document message when previewing an Office file (.pptx)", async () => { - // Arrange: the workspace fileserver returns real .pptx bytes — a ZIP whose - // header carries a NUL, so the hook classifies the file as binary. - fetchMock.mockResolvedValue({ - ok: true, - status: 200, - arrayBuffer: () => - Promise.resolve(new Uint8Array([0x50, 0x4b, 0x03, 0x04, 0x00]).buffer), - }); + // The acceptance criteria require the clear message in BOTH view modes. The + // plain-mode fallback and the rich-mode binary branch both route through + // UnpreviewableFallback, so one parametrized spec covers both code paths. + it.each(["rich", "plain"] as const)( + "shows a clear unsupported-document message for an Office file (.pptx) in %s mode", + async (viewMode) => { + // Arrange: the workspace fileserver returns real .pptx bytes — a ZIP whose + // header carries a NUL, so the hook classifies the file as binary. + fetchMock.mockResolvedValue({ + ok: true, + status: 200, + arrayBuffer: () => + Promise.resolve( + new Uint8Array([0x50, 0x4b, 0x03, 0x04, 0x00]).buffer, + ), + }); - // Act - renderViewer("demo.pptx"); + // Act + renderViewer("demo.pptx", viewMode); - // Assert: the format-aware "no preview" message replaces the generic binary - // fallback, so the pane is never blank and the limitation is explicit. - expect( - await screen.findByTestId("file-content-viewer-unsupported-document"), - ).toBeInTheDocument(); - }); + // Assert: the format-aware "no preview" message replaces the generic + // binary fallback in both modes, so the pane is never blank. + expect( + await screen.findByTestId("file-content-viewer-unsupported-document"), + ).toBeInTheDocument(); + }, + ); }); From a47b8ec1f19a7e7bb4ffa287170025fcc50db1ff Mon Sep 17 00:00:00 2001 From: hieptl Date: Sat, 30 May 2026 12:09:18 +0700 Subject: [PATCH 3/3] refactor: update the code based on feedback --- .../features/files-tab/file-content-viewer.test.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/__tests__/components/features/files-tab/file-content-viewer.test.tsx b/__tests__/components/features/files-tab/file-content-viewer.test.tsx index 2cbb2362d..734dbb2dc 100644 --- a/__tests__/components/features/files-tab/file-content-viewer.test.tsx +++ b/__tests__/components/features/files-tab/file-content-viewer.test.tsx @@ -37,11 +37,11 @@ vi.mock("#/api/backend-registry/active-store", () => ({ getActiveBackend: () => getActiveBackendMock(), })); -const downloadFileMock = vi.fn(); +// The hook statically imports the cloud runtime service; stub the module so +// this local-path test never loads the real cloud/proxy machinery. The test +// uses the fetch (local) path, so downloadFile is never called or asserted. vi.mock("#/api/runtime-service/agent-server-runtime-service", () => ({ - default: { - downloadFile: (...args: unknown[]) => downloadFileMock(...args), - }, + default: { downloadFile: vi.fn() }, })); const fetchMock = vi.fn(); @@ -68,7 +68,6 @@ describe("FileContentViewer", () => { useActiveConversationMock.mockReset(); useRuntimeIsReadyMock.mockReset(); getActiveBackendMock.mockReset(); - downloadFileMock.mockReset(); useRuntimeIsReadyMock.mockReturnValue(true); useActiveConversationMock.mockReturnValue({