improvement(files): fit-width previews and chip-chrome viewer controls#5002
Conversation
- PDF and DOCX previews now treat 100% zoom as fit-to-width instead of capping at the page's natural print size, removing the dead gutters in wide panels (pdf.js re-renders the canvas at the target width and DOCX uses CSS zoom, so both stay crisp) - PreviewToolbar page/zoom controls move from 24px ghost Buttons with off-token labels to canonical emcn chips (icon-only Chip pills, text-sm --text-body value labels) - XLSX sheet tabs move from underline-style ghost Buttons to a chip cluster using the active pill state - Audio preview swaps the music emoji for the lucide Music icon on design-system tokens
|
@greptile |
|
@cursor review |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Both viewers debounce resize handling (~150ms) so dragging a panel divider does not refit/re-raster on every frame; PDF still applies the first non-zero width immediately. Preview chrome moves to design-system Reviewed by Cursor Bugbot for commit 6a33064. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 62d12a0. Configure here.
Greptile SummaryThis PR removes the fixed-width cap from PDF and DOCX previews so 100% zoom fills the panel width, debounces the DOCX
Confidence Score: 5/5Safe to merge — all changes are UI-layer improvements with no data-path or auth impact. The fit-to-width changes are straightforward math (removing a cap), the debounce mirrors a pattern already reviewed and merged, the MediaPreview consolidation is a pure code-quality refactor with identical runtime behaviour, and the Chip migration follows the design-system conventions correctly. No regressions or logic gaps were found across the five changed files. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
FV[FileViewer] -->|kind='audio'| MP[MediaPreview]
FV -->|kind='video'| MP
MP --> BU[useBlobUrl]
BU --> UE[useEffect: createObjectURL]
UE -->|kind='audio'| AP[Audio player + Music icon]
UE -->|kind='video'| VP[Video player]
PDF[PdfViewerCore] --> RO_PDF[ResizeObserver]
RO_PDF -->|first non-zero| IMM[setContainerWidth immediately]
RO_PDF -->|subsequent| DB_PDF[debounce 150ms → setContainerWidth]
DB_PDF --> PW[pageWidth = containerWidth - 2×padding no max-width cap]
DOCX[DocxPreview] --> RO_DOCX[ResizeObserver]
RO_DOCX --> DB_DOCX[debounce 150ms → applyPostRenderStyling]
DB_DOCX --> FS[fitScale = available / naturalWrapperWidth no Math.min cap]
PT[PreviewToolbar] -->|leftIcon chips| CHP1[Chip: prev / next / zoom-in / zoom-out / reset]
XLSX[XlsxPreview] -->|active prop| CHP2[Chip per sheet name]
Reviews (5): Last reviewed commit: "improvement(files): module cleanup — ded..." | Re-trigger Greptile |
Greptile SummaryThis PR updates the file-viewer feature in two ways: it changes the zoom baseline for PDF and DOCX previews from "cap at natural print size" to "fit-to-width", and it migrates toolbar/tab controls from legacy
Confidence Score: 4/5The changes are well-scoped UI improvements with correct component API usage throughout; safe to merge. All five files have clean, intentional changes. The Chip migrations follow the emcn API correctly. The one thing worth watching is the PDF ResizeObserver: removing the old width cap means pdf.js now re-renders the canvas on every panel resize, whereas before resizes above ~864 px were no-ops for the renderer. This won't cause incorrect behaviour but can feel sluggish on large multi-page PDFs. apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/pdf-viewer.tsx — the ResizeObserver path now triggers pdf.js canvas repaints on every resize event in wide panels. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Container ResizeObserver] --> B{containerWidth > 0?}
B -- yes --> C[pageWidth = containerWidth - 2×padding]
B -- no --> D[pageWidth = undefined]
C --> E[ReactPdfPage width=pageWidth]
E --> F[pdf.js re-renders canvas at new width]
G[User zoom in/out] --> H[applyZoomAt]
H --> I[wrapper.style.zoom = next]
I --> J[CSS visual scale only - no canvas re-render]
subgraph DOCX
K[fitDocxToContainer] --> L[fitScale = available / naturalWrapperWidth]
L --> M[scale = fitScale × zoom%/100]
M --> N[wrapper.style.zoom = scale]
end
Reviews (2): Last reviewed commit: "improvement(files): fit-width previews a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
With fit-to-width every pageWidth change re-rasterises all page canvases, so per-tick updates during a panel-divider drag re-rendered the document continuously. First measurement still applies immediately.
|
@greptile |
|
@cursor review |
…diate resize slot A hidden container reports zero width from the ResizeObserver; treating that as the initial measurement pushed the real first width onto the debounce path and delayed initial render.
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8aea876. Configure here.
…docx refits - Merge the near-identical AudioPreview/VideoPreview into one MediaPreview (shared fetch/blob-URL/error/loading path; only the player differs) - Debounce docx resize refits the same way the PDF preview debounces width measurements (the initial fit comes from the render path, not the observer) - Document the load-bearing buffer copy in pdf-viewer (pdf.js transfers and detaches the ArrayBuffer it receives)
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a33064. Configure here.
1 participant
Summary
PreviewToolbarpage/zoom controls migrate from 24px ghostButtons with off-token labels (text-[12px]--text-secondary) to canonical emcn chips — icon-onlyChippills withtext-sm--text-bodyvalue labelsButtons to a chip cluster using theactivepill stateMusicicon on design-system tokensType of Change
Testing
Mounted the real
FileVieweragainst sample files for every preview type (pdf wide + narrow, docx, xlsx multi-sheet, image, audio) in a headless browser with the app's color tokens — all render clean with zero console errors; screenshots verified fit-width behavior and chip chrome. 138 file-viewer tests pass;tsc --noEmitandbiome checkclean.Checklist