fix(files): give the file preview a working detach button

The button next to the file preview's close icon was Copy Content, whose
overlapping-pages glyph reads as a pop-out control - and for a PDF or any
media/binary preview it was completely dead: those branches never fill
filePreviewContent, so the click hit an empty-content guard and did nothing,
with no feedback.

There is now a real detach button that opens the previewed file in a browser
tab (raw route for PDFs/images/media/text, the server-converted PDF preview
for docx/pptx), severs window.opener by hand so a blocked pop-up stays
detectable, closes the overlay on success (which also stops any playing
media), and disarms on close so it can never open a stale file. The copy
button now toasts 'Nothing to copy in this preview' instead of staying
silent.

Verified live with Playwright against an isolated instance: button visible
and armed on a PDF preview, file-raw answers 200, clicking opens the URL and
tears the overlay down, text previews keep a working copy buffer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-24 00:59:27 +02:00
parent 82f81d21c4
commit f3c615b669
3 changed files with 203 additions and 0 deletions
+1
View File
@@ -567,6 +567,7 @@
<div class="file-preview-actions">
<button class="btn-icon-sm file-preview-edit-btn" id="filePreviewEditBtn" onclick="app.enterFilePreviewEdit()" title="Edit file" aria-label="Edit file" hidden><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M17 3a2.85 2.83 0 1 1 4 4L7.5 20.5 2 22l1.5-5.5z"/></svg></button>
<button class="btn-icon-sm" onclick="app.copyFilePreviewContent()" title="Copy content">&#x2398;</button>
<button class="btn-icon-sm" id="filePreviewDetachBtn" onclick="app.detachFilePreview()" title="Open in new tab" aria-label="Open in new tab" hidden><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"/><polyline points="15 3 21 3 21 9"/><line x1="10" y1="14" x2="21" y2="3"/></svg></button>
<button class="btn-icon-sm" onclick="app.closeFilePreview()" title="Close">&times;</button>
</div>
</div>
+45
View File
@@ -3313,6 +3313,11 @@ Object.assign(CodemanApp.prototype, {
// Stop whatever the previous preview was playing. Overwriting innerHTML
// only DETACHES a <video>/<audio>; a detached media element keeps playing.
this._stopFilePreviewMedia();
// Disarm detach until this load has a URL of its own: an early error return
// must not leave the button opening the PREVIOUS file in a new tab.
this.filePreviewDetachUrl = '';
const detachBtn = this.$('filePreviewDetachBtn');
if (detachBtn) detachBtn.hidden = true;
// Show overlay with loading state
overlay.classList.add('visible');
@@ -3340,6 +3345,19 @@ Object.assign(CodemanApp.prototype, {
return;
}
// Every branch below renders from one of these routes, so the detach button
// can always offer the same bytes in a browser tab: docx/pptx through the
// server-converted PDF preview, everything else through the raw route.
// (html/htm arrive as a download there by design — file-raw serves them
// attachment-only so widening READ never widens RUN.)
const officeDoc = ext === 'docx' || ext === 'pptx';
this.filePreviewDetachUrl = attachmentId
? `/api/sessions/${sessionId}/attachments/${encodeURIComponent(attachmentId)}/${officeDoc ? 'preview' : 'raw'}`
: officeDoc
? `/api/sessions/${sessionId}/file-preview?path=${encodeURIComponent(filePath)}`
: `/api/sessions/${sessionId}/file-raw?path=${encodeURIComponent(filePath)}`;
if (detachBtn) detachBtn.hidden = false;
// Registered attachment: render straight from its by-id routes — images and
// PDFs inline, Office docs via the server-converted PDF preview, text fetched
// raw. (Workspace-path previews fall through to the file-content endpoint.)
@@ -3489,6 +3507,29 @@ Object.assign(CodemanApp.prototype, {
// audible and keeps streaming from the server. Closing has to stop it.
this._stopFilePreviewMedia();
this.filePreviewContent = '';
this.filePreviewDetachUrl = '';
const detachBtn = this.$('filePreviewDetachBtn');
if (detachBtn) detachBtn.hidden = true;
},
/**
* Open the previewed file in a browser tab and close the overlay.
*
* window.open is called WITHOUT the 'noopener' feature string: with it the
* call returns null even on success, which would make a blocked pop-up
* indistinguishable from a working one. The opener link is severed by hand
* instead, and a null return then reliably means the browser blocked it, in
* which case the overlay stays up so the user has not lost the file.
*/
detachFilePreview() {
if (!this.filePreviewDetachUrl) return;
const win = window.open(this.filePreviewDetachUrl, '_blank');
if (!win) {
this.showToast('Pop-up blocked: allow pop-ups for this site to detach previews', 'error');
return;
}
win.opener = null;
this.closeFilePreview();
},
/**
@@ -4127,6 +4168,10 @@ Object.assign(CodemanApp.prototype, {
}).catch(() => {
this.showToast('Failed to copy', 'error');
});
} else {
// Media/PDF/binary previews have no text buffer. Saying so beats the
// dead-button silence this used to be.
this.showToast('Nothing to copy in this preview', 'info');
}
},
+157
View File
@@ -0,0 +1,157 @@
/**
* @fileoverview File viewer detach button: pop the previewed file into a browser tab.
*
* The header used to end in [copy ⎘] [close ×], and for a PDF/media preview the
* copy button was completely dead: `filePreviewContent` stays empty for those
* branches, the `if (content)` guard swallowed the click, and the ⎘ glyph reads
* as a pop-out icon — so the visible symptom was "the detach button next to the
* X does nothing". There is now a real detach button (`filePreviewDetachBtn`)
* that opens the preview's own raw/preview route in a new tab, and the copy
* button toasts instead of silently doing nothing.
*
* What is pinned here:
* 1. opening a workspace PDF arms the detach URL (file-raw) and reveals the button,
* 2. an attachment docx/pptx detaches through the converted-PDF /preview route,
* other attachments through /raw,
* 3. detach opens the URL, severs opener, and closes the overlay,
* 4. a blocked pop-up (window.open → null) keeps the overlay up and toasts,
* 5. closing the preview disarms the button (no stale URL for the next file),
* 6. copy with no text buffer toasts instead of the old dead-button silence.
*
* Loaded via `vm` against a stub app, same harness style as
* file-preview-media.test.ts (no jsdom).
*/
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
import vm from 'node:vm';
import { describe, expect, it, vi } from 'vitest';
const PUBLIC = resolve(import.meta.dirname, '../src/web/public');
const panelsJs = readFileSync(resolve(PUBLIC, 'panels-ui.js'), 'utf8');
function loadApp() {
const CodemanApp = function CodemanApp(this: unknown) {} as unknown as new () => Record<string, unknown>;
const windowStub: Record<string, unknown> = { addEventListener: vi.fn(), open: vi.fn() };
const context = vm.createContext({
CodemanApp,
console: { ...console, warn: vi.fn(), error: vi.fn() },
localStorage: { getItem: () => null, setItem: () => {}, removeItem: () => {} },
escapeHtml: (s: string) => String(s),
document: { getElementById: () => null, addEventListener: vi.fn() },
window: windowStub,
setTimeout,
clearTimeout,
confirm: () => true,
fetch: () => {
throw new Error('fetch not stubbed');
},
});
vm.runInContext(panelsJs, context, { filename: 'panels-ui.js' });
const body = {
innerHTML: '',
querySelectorAll: () => [] as unknown[],
querySelector: () => null,
};
const overlay = {
classes: new Set<string>(['visible']),
classList: {
add: (c: string) => overlay.classes.add(c),
remove: (c: string) => overlay.classes.delete(c),
contains: (c: string) => overlay.classes.has(c),
},
};
const detachBtn = { hidden: true };
const elements: Record<string, unknown> = {
filePreviewBody: body,
filePreviewOverlay: overlay,
filePreviewTitle: { textContent: '' },
filePreviewFooter: { textContent: '' },
filePreviewDetachBtn: detachBtn,
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const app = new CodemanApp() as Record<string, any>;
app.$ = (id: string) => elements[id] ?? null;
app._resetFilePreviewEdit = () => {};
app._isExternalPreviewPath = () => false;
app.showToast = vi.fn();
app.filePreviewContent = '';
return { app, body, overlay, detachBtn, windowStub };
}
describe('file viewer detach button', () => {
it('arms the detach URL and reveals the button for a workspace PDF', async () => {
const { app, detachBtn, body } = loadApp();
await app.openFilePreview('/ws/report.pdf', 's1');
expect(app.filePreviewDetachUrl).toBe('/api/sessions/s1/file-raw?path=%2Fws%2Freport.pdf');
expect(detachBtn.hidden).toBe(false);
expect(body.innerHTML).toContain('<iframe');
});
it('routes attachment office docs through /preview and other attachments through /raw', async () => {
const { app } = loadApp();
await app.openFilePreview('/tmp/deck.pptx', 's1', 'att-1');
expect(app.filePreviewDetachUrl).toBe('/api/sessions/s1/attachments/att-1/preview');
await app.openFilePreview('/tmp/scan.pdf', 's1', 'att-2');
expect(app.filePreviewDetachUrl).toBe('/api/sessions/s1/attachments/att-2/raw');
});
it('opens the URL, severs opener, and closes the overlay on detach', () => {
const { app, overlay, windowStub } = loadApp();
const win: Record<string, unknown> = { opener: {} };
(windowStub.open as ReturnType<typeof vi.fn>).mockReturnValue(win);
app.filePreviewDetachUrl = '/api/sessions/s1/file-raw?path=doc.pdf';
app.detachFilePreview();
expect(windowStub.open).toHaveBeenCalledWith('/api/sessions/s1/file-raw?path=doc.pdf', '_blank');
expect(win.opener).toBeNull();
expect(overlay.classList.contains('visible')).toBe(false);
});
it('keeps the overlay and toasts when the pop-up is blocked', () => {
const { app, overlay, windowStub } = loadApp();
(windowStub.open as ReturnType<typeof vi.fn>).mockReturnValue(null);
app.filePreviewDetachUrl = '/api/sessions/s1/file-raw?path=doc.pdf';
app.detachFilePreview();
expect(overlay.classList.contains('visible')).toBe(true);
expect(app.showToast).toHaveBeenCalledWith(expect.stringContaining('Pop-up blocked'), 'error');
});
it('does nothing when no preview is armed', () => {
const { app, windowStub } = loadApp();
app.filePreviewDetachUrl = '';
app.detachFilePreview();
expect(windowStub.open).not.toHaveBeenCalled();
});
it('disarms the button when the preview closes', () => {
const { app, detachBtn } = loadApp();
app.filePreviewDetachUrl = '/api/sessions/s1/file-raw?path=doc.pdf';
detachBtn.hidden = false;
app.closeFilePreview();
expect(app.filePreviewDetachUrl).toBe('');
expect(detachBtn.hidden).toBe(true);
});
it('copy with no text buffer toasts instead of staying silent', () => {
const { app } = loadApp();
app.filePreviewContent = '';
app.copyFilePreviewContent();
expect(app.showToast).toHaveBeenCalledWith('Nothing to copy in this preview', 'info');
});
});