feat(web): render HTML and PDF files in the file viewer - #9143
juliusmarminge merged 2 commits into
UI Consistency: All clear
All clear
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
🔍 Agent activity — 13 tool calls
💭 I should open the file itself to see these changes in their full context.
1. browse_code — apps/web/src/components/files/FilePreviewPanel.tsx
2. browse_code — apps/web/src/components/files/FilePreviewPanel.tsx
💭 I'm now examining the header toggle area and remaining render logic, checking against UI consistency concerns like shared primitives, Tailwind ownership, theming, and scroll behavior. One issue I'm flagging is a hardcoded
bg-whiteon the preview iframe, which could conflict with dark theme support. Everything else checks out—the revision suffix computation, PDF/HTML detection, and environment routing all look consistent. I want to confirm that PDFs correctly skip the rendered/source toggle sincecanToggleRenderedonly applies to markdown and HTML, then move on to examining the header region around lines 1090-1320.
3. browse_code — apps/web/src/components/files/FilePreviewPanel.tsx
4. browse_code — apps/web/src/components/files/FilePreviewPanel.tsx
💭 I'm checking UI consistency now, comparing how the browser, video, and image previews key their components and whether they refresh properly after workspace mutations. I notice WorkspaceBrowserPreview appends a revision suffix as a query param to bust cache rather than using the mutation refresh hook, which raises a question about whether adding that param could break a signed URL's HMAC verification -- I want to compare against how the image preview handles this. I'm also checking the toggle logic for rendered views and the reveal handling to make sure resetting state on toggle-off doesn't cause inconsistent behavior, and looking at how PDF files are routed into the browser file renderer. The bg-white hardcoded on the iframe seems fine since it's owned by this component and HTML pages generally assume a white background. Now I want to verify which extensions
isBrowserPreviewFileactually covers, since if it includes markdown that could create inconsistency with how HTML files are treated.
5. browse_code — apps/web/src/browser/openFileInPreview.ts
6. browse_code — apps/web/src/components/files/FilePreviewPanel.tsx
💭 I confirm no overlap between browser-preview and markdown file types, and that browser opening is desktop-only while the render toggle works on both web and desktop. Now I'm checking the flex layout to make sure the iframe correctly fills the available height within its flex column parent. Since the container defaults to stretch alignment, the iframe should stretch to full width even without an explicit class for it, so I want to verify there's no hardcoded background color inconsistency by searching for existing bg-white usage elsewhere in the codebase, and I should also check whether the inline preview panel mode or scroll area behavior could interfere with the iframe.
7. git_tools — bg-white
8. git_tools — iframe
💭 The
bg-whiteon the iframe seems reasonable for preventing dark theme bleed on documents without their own background, so I won't flag it. Now I'm shifting to look for genuine UI-consistency issues, starting with the mutation refresh path where image and video previews register their own hooks. Video refreshes its signed URL viauseWorkspaceMutationRefresh, but image just appends a revision suffix, and WorkspaceBrowserPreview follows the image pattern. The panel-level refresh hook is disabled for PDFs but not HTML, since HTML still needs the file query for source view, which seems fine. Thekey={src}remount on mutation change works correctly, and hiding the truncation banner for rendered browser files makes sense too. I'm also noting the "rendered" toggle for HTML defaults to true while markdown's defaults to false — a different default despite documentation claiming the choice persists similarly to the markdown toggle. Nothing else jumps out as a real UI consistency issue: the sandbox attribute concern is more of a security question than a consistency one, MediaActions wrapping isn't required for HTML/PDF, and the loader markup duplication across three components is a pre-existing pattern rather than something the new component introduces. For HTML, the branch order puts renderBrowserFile before loading, so the iframe shows immediately - good. Checking the Toggle's pressed state next: for markdown it's renderMarkdown, for HTML it's renderBrowserFile, both of which factor in revealHandled, meaning the toggle appears unpressed during a pending reveal even if the preference is true — this mirrors existing markdown behavior, so it's consistent rather than a new issue. But I'm sensing there might be an actual discrepancy worth digging into here. Environment routing looks fine since WorkspaceBrowserPreview gets an explicit environmentId, and the case where isHtml is true but absolutePath can't resolve seems handled too. Thinking about theme/contrast now — the iframe uses a white background with no dark mode handling, so an HTML preview would flash white in dark mode, though that's likely intentional since it's rendering arbitrary web content. I'm also considering the iframe's border styling. Theborder-0class properly overrides the default inset border on iframes. Checking whether the panel's inline mode could cause issues with iframes, but that doesn't seem like a real finding, and the toggle logic already correctly hides itself when a file is neither markdown nor HTML. Now I'm looking at the toggle's pressed state and tooltip behavior. That looks fine. I'm also checking whether omittingfile.refreshfor PDFs is intentional -- it is, since the query is disabled in that case. Now I'm considering whether enabling the project file query for HTML files even when rendering as a page causes unnecessary downloads, though it may still serve some purpose. Checking a few more edge cases: the browser render branch takes precedence so error display isn't an issue, the iframe replaces the editable surface without conflicting with editing state, and keys on the preview components look consistent with their src values. I'm also noting that signed URLs expire after an hour, and while video previews register a refresh mechanism, image previews don't -- but that seems acceptable rather than a bug. Now I'm wondering whether the iframe's width could actually be a problem. The container uses a flex-col layout, and the iframe sits as a flex item with flex-1 and min-h-0 set. Since iframes are replaced elements, stretch behavior on the cross axis only kicks in when width is auto, and I need to check whether the iframe has an implicit default width attribute that could interfere with that stretching. Good, the stretch condition holds since margins aren't auto. Checking the toggle variant and icon semantics—pressed state shows Code2, unpressed shows Eye, consistent with markdown behavior. The Globe2 open-in-browser button remains a distinct, non-redundant control, so that's fine too. Now I want to verify whether the new localisPdfPreviewFileduplicates existing logic infilePreview.ts, particularly since the sharedhasPreviewExtensionhelper was flagged—checking if there's already a pdf-specific helper there I should reuse instead.
9. browse_code — In packages/shared (or wherever @t3tools/shared/filePreview lives), show the full contents of the filePreview module: all exported helpers like hasPreviewExtension, isWorkspaceImagePreviewPath, isWorkspaceVideoPreviewPath, and whether there is any existing PDF or HTML path helper.
💭 I notice there's already a local pattern for browser preview files, so adding another local regex for PDFs follows existing precedent in that file rather than introducing new inconsistency—this seems like the smallest durable change given the guidance to avoid unrelated cleanup. Now I want to shift focus to actual UI behavior: whether FilePreviewPanel rendered inline (via the preview-panel-mode=inline data attribute) has its content height properly constrained, since an iframe in that context could break the layout if unbounded.
10. git_tools — preview-panel-mode
11. browse_code — apps/web/src/components/preview/PreviewPanelShell.tsx
💭 The shell has proper height flowing down to the iframe. Now I'm thinking about a potential interaction bug: dragging the resize handle over an iframe can lose pointer capture since the iframe swallows mouse events, breaking the drag. I should check whether
useResizableWidth/RightPanelResizeHandle uses pointer capture or window mousemove listeners — since PreviewView normally uses a webview on desktop and has no iframe on web, this would be the first in-panel iframe, making it a genuine regression risk if the handle relies on window listeners rather than pointer capture.
12. browse_code — apps/web/src/hooks/useResizableWidth.ts
13. browse_code — apps/web/src/components/preview/RightPanelResizeHandle.tsx
💭 Pointer capture handles the drag-over-iframe case fine, so that's not an issue. The file explorer aside and scroll area aren't resizable, and drag/drop and text-selection across panels seem fine too, so I'm now checking whether the migration preserved accessibility and interaction semantics. I'm also considering the sandboxed iframe missing
allow-same-origin, which could breaklocalStorageaccess on the page, but that's not a UI concern. Looking through the render toggle logic for markdown, HTML, and PDF files, everything seems handled correctly since PDFs skip the toggle and unused setters just go unused for non-applicable file types. I'm checking the truncation banner logic and the read-only Virtualizer condition — both behave correctly for HTML source versus rendered mode. Now I'm examining whetherprops.environmentIdis guaranteed to match the environment embedded inprops.threadRef, sinceuseAssetUrlStaterelies on both being consistent for correct environment routing in shared renderers. Actually, the iframe white background is just the correct canvas for arbitrary HTML/PDF content, so that's fine even without a comment explaining it. I'm also checking the loading/error states and messaging conventions, which stay consistent with sibling components, and noting that disabling the file query for PDFs means refresh behavior doesn't trigger there. I'm now looking at focus handling with iframes—when focus lands inside the iframe,document.activeElementis still the iframe element itself, so the preview-focus check correctly reports the panel as focused. Keyboard events like Escape won't reach the app while focus is inside the sandboxed iframe document, but that's just an inherent limitation of iframes rather than a bug. I'm circling back to whether hiding the source/page toggle for PDFs is worth flagging as a real finding. Tracing through the terminal-link scenario confirms a link to a line always forces source view, matching the documented behavior. I've checked the Toggle component reuse and I'm now satisfied the reveal logic is sound overall — with no actual findings from this review, I should wrap up and give a clean final verdict. Verifying the flexbox stretch behavior applies to replaced elements like iframes in a column flex container—checking the spec's treatment of cross-axis sizing for auto-width items to confirm whether the missingw-fullclass actually causes a rendering issue. The comment placement inside the ternary's parenthesized expression is valid JS comment syntax, not JSX children, so that's fine. I'm satisfied everything checks out here—light/dark contrast is consistent with how the desktop browser would render an HTML file with no background, and the security concern is already being handled by another review track. I'll mark this as all clear.
Agent Credits: 119 credits
Token usage: 26 uncached input · 413,823 cache-read input · 82,309 cache-write input · 16,418 output
Agent Credits may also include non-token charges from external tools such as web research.