Commit 6a7d5ae
authored
feat(files): extract PDF viewer behind SSR boundary and polish file preview (#4316)
* feat(files): extract PDF viewer behind SSR boundary and polish file preview
## Core architectural fix
Move all react-pdf / pdfjs-dist code into a new pdf-viewer.tsx module and
import it exclusively via next/dynamic({ ssr: false }). pdfjs-dist v5
references DOMMatrix at module evaluation time, which crashed SSR. The
previous workaround (a DOMMatrix polyfill in instrumentation.ts) is removed
in favour of this proper hard module boundary.
## PDF viewer improvements
- Cursor-anchored zoom: Ctrl/⌘+wheel and trackpad-pinch now zoom toward the
cursor instead of the top-left corner. Toolbar ± buttons anchor to the
viewport centre. Uses the canonical scroll-adjust formula used by map and
canvas viewers.
- Horizontal scroll: dropping flex-col from the scroll container lets the
zoomed pages wrapper overflow naturally and produces a horizontal scrollbar
at zoom > 1×.
- Loading skeleton: replaced the conditional inline skeleton with an
absolute inset-0 overlay so it fills the scroll container correctly in all
layout contexts.
- Shadow tokens: fixed shadow-[var(--shadow-medium)] and
shadow-[var(--shadow-card)] to use the Tailwind utility classes
shadow-medium and shadow-card directly.
## File viewer cleanup
- data-table.tsx: wrap setInputRef in useCallback([]) so the ref callback
has a stable identity across renders. Previously the inline function got a
new identity on every keystroke (because editValue state changed), causing
React to teardown/remount the ref and re-run node.select() on every
character typed.
- preview-panel.tsx: keep useMemo on ctxValue passed to Context.Provider —
Context uses Object.is, so a new object every render causes unnecessary
consumer re-renders.
- resource-content.tsx: remove unnecessary useCallback/useMemo wrappers on
handlers and derived values that have no memoization observers.
## API route
- Wrap content route with withRouteHandler for automatic request-ID tracking
via AsyncLocalStorage; remove manual generateRequestId() calls.
- Add resourceName to audit record; add encoding param support (base64 /
utf-8).
## Query hooks
- Include key (storage object key) in both useWorkspaceFileContent and
useWorkspaceFileBinary query key tuples so the cache is correctly busted
when a file is re-uploaded with a new storage key.
## Other
- Add Suspense boundaries to files/page.tsx and files/[fileId]/page.tsx
(required for useSearchParams inside the Files component).
- Add mmd to SUPPORTED_CODE_EXTENSIONS (Mermaid diagrams).
- Add https: to CSP img-src.
- Remove ==== separator comments from lib/copilot/constants.ts.
- New dependencies: pdfjs-dist 5.4.296, mermaid 11.14.0,
monaco-editor 0.55.1, @monaco-editor/react 4.7.0.
* fix(files): replace instanceof Error checks with toError() and fix skeleton tokens
- Use toError() from @sim/utils/errors across all catch blocks in
file-viewer.tsx, preview-panel.tsx, and route.ts instead of the
prohibited `err instanceof Error ? err.message : fallback` pattern
- Fix loading skeleton in files.tsx: bg-white → bg-[var(--surface-2)]
and shadow-[var(--shadow-medium)] → shadow-medium
* fix(files): address PR review findings
- csp.ts: revert bare https: from img-src — it defeats the existing
domain allowlist and opens info-leakage vectors
- files/page.tsx + files/[fileId]/page.tsx: add explicit fallback={null}
to <Suspense> to make intent clear (React defaults to null, but
omitting it looks like an oversight)
- preview-panel.tsx: restore pre passthrough in STATIC_MARKDOWN_COMPONENTS
so Streamdown's wrapping <pre> doesn't nest inside the custom code
block <div>, which produced invalid HTML and broken styling
- file-viewer.tsx: add 'webm' to VIDEO_PREVIEWABLE_EXTENSIONS to match
'video/webm' in VIDEO_PREVIEWABLE_MIME_TYPES
* chore(files): revert accidental pptxgenjs.cjs re-minification
The bundle was regenerated non-deterministically during development (same
pptxgenjs 4.0.1, different variable names in minifier output). No functional
change — restore the prior version to keep the diff clean.
* fix(files): fix Monaco stale closure, XLSX Ctrl+S data loss, and async workbook mutation
Three bugs from Cursor Bugbot follow-up review:
1. Stale closure in handleEditorMount (Medium): useCallback([], []) captured
content='' at first render. When Monaco mounts after content loads (e.g.
switching from preview to editor mode), lastSyncedContentRef was never
initialized and external content changes stopped syncing. Fixed by keeping
a contentRef updated on every render and reading it inside handleEditorMount.
2. XLSX Ctrl+S discards active cell edit (Medium): handleSave read from
workbookRef.current before DataTable's in-progress editValue was committed.
Fixed by exposing commitEdit() from DataTable via useImperativeHandle
(using an always-current editStateRef so the handle stays stable) and
calling it at the top of handleSave.
3. Async workbook mutation fragility (Low): handleCellChange / handleHeaderChange
updated the workbook inside import('xlsx').then(), creating microtask-order
coupling with handleSave. Fixed by caching the xlsx module in xlsxModuleRef
on first parse and using it synchronously in both handlers.
* refactor(files): cleanup anti-patterns across file viewer components
Six-pass cleanup over the file-viewer directory:
Effects (you-might-not-need-an-effect):
- AudioPreview, VideoPreview: replace reset useEffect with key={file.id} so
the component remounts on file change — React's canonical solution
- DocxPreview: same key-prop fix; removes a 5-setState reset effect that was
also clearing containerRef.current.innerHTML unnecessarily
Callbacks (you-might-not-need-a-callback):
- handleEditorMount, handleEditorChange: remove useCallback — MonacoEditor is
dynamic(), not React.memo, so reference stability has no observer
- markSavedContent: remove useCallback — called only through an onSaveRef,
never directly observed
- DataTable.setInputRef: remove useCallback — callback refs on native elements
are called regardless of reference identity
Design tokens (emcn-design-review):
- VideoPreview: bg-black → bg-[var(--surface-inverted)]
- HtmlPreview iframe: bg-white → bg-[var(--surface-2)]
useMemo, useState, and react-query passes found no issues.
* improvement(files): replace stock Monaco theme with Sim design system theme
Define sim-dark and sim-light Monaco themes using Sim's exact design tokens
instead of the default vs/vs-dark which looked identical to stock VSCode.
Chrome changes (both themes):
- Background, gutter use --bg (not VSCode's near-black / pure-white defaults)
- Line numbers use --text-muted instead of VSCode gray
- Cursor switches to --brand-secondary (#33b4ff)
- Selection highlight is brand blue at 15% opacity
- Scrollbar shadow removed, track uses surface tokens
- Bracket match, word highlight, find match all keyed to brand blue
- Suggestion/hover widgets use --surface-2 / --border tokens
- All hardcoded shadows removed (scrollbar.shadow = transparent)
Syntax token changes (inherit: true — base handles unlisted tokens):
- Comments: muted gray + italic (vs VSCode's bright green)
- Strings: #3ab872 dark / #16825d light (vs VSCode orange-red)
- Numbers: warm amber / warm orange (both readable on their backgrounds)
- Keywords: #33b4ff dark / #0078d4 light (brand blue family)
- Types: complementary blue-gray / purple
* fix(files): bump light theme comment color to #888888 for WCAG contrast
* fix(files): fix dark mode comment contrast #4a4a4a → #606060 (~1.9:1 → ~2.9:1)
* improvement(files): cursor to default color, video background to surface-1
- Monaco cursor: #33b4ff (brand blue) → #e6e6e6 dark / #1a1a1a light
(text cursor should be neutral, not loud)
- VideoPreview background: var(--surface-inverted) → var(--surface-1)
(consistent with PDF viewer, fits workspace context over cinema-black)
* fix(files): stabilize setInputRef callback and guard against double-commit in DataTable
Wrap setInputRef in useCallback([], []) so React doesn't tear down and
re-mount the input ref on every keystroke. Without stable identity, every
editValue state change caused node.focus()/node.select() to fire, resetting
the cursor selection to "select all" on each character typed.
Add isCommittedRef to guard both the imperative commitEdit handle and the
inline commitEdit (called by onBlur) against double-application. The ref is
cleared in startEdit and set to true on the first commit, so a concurrent
onBlur cannot re-apply the same edit.
* fix(files): preserve scroll position during Mothership streaming edits
Two fixes to the Monaco auto-scroll logic:
1. At streaming start, initialize textareaStuckRef from the editor's actual
scroll position (isAtBottom check) instead of unconditionally setting true.
Previously every streaming session jumped the viewport to the last line on
the very first content update, even when the user was reading mid-file.
2. Replace the wheel-only DOM listener with editor.onDidScrollChange(), the
proper Monaco API. This covers trackpad, scrollbar drag, and keyboard scroll
— not just mouse wheel. As a bonus, scrolling back to the bottom during
streaming now re-engages follow mode (matching iTerm2/xterm.js behavior).
3. Save and restore view state around model.setValue() during streaming when
the user has scrolled away from the bottom. This prevents Monaco from
resetting the viewport on each content replacement. When the user is at
the bottom, view state is not saved so Effect 3 can scroll to the new bottom.
* fix(files): fix two scroll logic bugs introduced in previous streaming scroll fix
The prior fix introduced a regression for the "user was at bottom" case and
a false-disengagement bug from programmatic scroll events.
Bug 1 — Effect ordering: all three effects fire on the same render when
isStreamInteractionLocked flips true. Effect 2 called isAtBottom() AFTER
Effect 1 had already called model.setValue(), which grew scrollHeight. The
old "at bottom" scroll position was now 200px short of the new bottom, so
isAtBottom() returned false, textareaStuckRef was set false, and Effect 3
never called revealLine. Users at the bottom stopped following the stream.
Fix: measure isAtBottom() in Effect 1 BEFORE setValue, while scrollHeight
is still accurate. Set textareaStuckRef = true only (never false here).
Effect 2 no longer initializes the ref — only the listener disengages it.
Bug 2 — onDidScrollChange fires during model.setValue: Monaco fires
onDidScrollChange when scroll dimensions change, including when setValue
grows the document. This caused the listener to disengage auto-scroll on
every content update even with no user interaction.
Fix: add suppressScrollListenerRef, set true before setValue/restoreViewState
and false after. The listener exits early when suppressed, so only genuine
user scroll events (wheel, trackpad, keyboard, scrollbar) can disengage.
Both refs moved to the component's ref block for conventional placement.
* chore(files): remove extraneous comments from file viewer and data table
* refactor(files): split 2281-line file-viewer.tsx into focused modules
TextEditor, DocxPreview, PptxPreview, XlsxPreview, ImagePreview each moved
to their own files. Shared utilities (PreviewError, resolvePreviewError,
shouldSuppressStreamingDocumentError, PDF_PAGE_SKELETON) extracted to
preview-shared.tsx. file-viewer.tsx is now the orchestrator + MIME constants
+ small stateless previews (~495 lines).
* fix(files): remove unnecessary TextEditorProps export
* refactor(files): four stellar-quality improvements to file-viewer split
- Extract useBlobUrl hook shared by AudioPreview and VideoPreview,
eliminating ~30 lines of duplicated state/effect logic
- Stabilize markSavedContent with useCallback (matches setDraftContent)
- Stabilize handleEditorChange with useCallback([setDraftContent])
- Fix pptx static render effect deps: drop redundant dataUpdatedAt
(already encoded in cacheKey) and unused workspaceId
* test(files): extract pure modules and add 122-test suite for file viewer logic
Extract TextEditorContentState machine and file category resolution into
plain .ts modules (text-editor-state.ts, file-category.ts) so they can
be unit-tested without React or Next.js overhead. Update component files
to import from the extracted modules, eliminating code duplication.
Add two test files:
- text-editor-state.test.ts: 32 tests covering resolveStreamingEditorContent,
the reducer (edit / save-success), and syncTextEditorContentState across
all phases (uninitialized, ready, streaming, reconciling) including
reference-equality short-circuit checks for zero-allocation paths
- file-category.test.ts: 90 tests covering MIME-type routing for all 8
categories, extension fallback, MIME-priority-over-extension, and
case-insensitive extension handling
* fix(files): add key to IframePreview and use monotonic seq for streaming PDF key
- Add key={file.id} to IframePreview so React remounts on file switch,
preventing stale renderError from persisting across different files
- Replace key={streamingBuffer.byteLength} with a monotonic sequence
counter so same-size successive PDF compilations still trigger a remount
* fix(files): restore getFileExtension import dropped during refactor
* fix(files): clear loadError on PDF success and fix streaming null-flash
- pdf-viewer: add setLoadError(null) in onLoadSuccess so the toolbar
is not permanently hidden after a failed-then-successful PDF load
- file-viewer: consolidate streaming-mode rendering so the debounce
period (before rendering=true) shows a skeleton instead of null
* refactor(files): cleanup pass — effect, callback, state, and design fixes
- text-editor: replace sync-external useEffect with "adjust during render"
pattern so the state machine advances immediately instead of after a paint
- text-editor: remove unnecessary useCallback from markSavedContent (no observer)
- files: narrow deleteTargetFile state to {id, name} — only those fields are used
- files: remove uploadFile (mutation object) from useCallback deps — .mutateAsync is stable
- files: remove unnecessary useCallback from handleNavigateToFiles (no observer)
- files: replace raw <button> with emcn Button for "Clear all filters" action1 parent dc20229 commit 6a7d5ae
25 files changed
Lines changed: 3862 additions & 2139 deletions
File tree
- apps/sim
- app
- api/workspaces/[id]/files/[fileId]/content
- workspace/[workspaceId]
- files
- [fileId]
- components/file-viewer
- home/components/mothership-view/components/resource-content
- hooks/queries
- lib
- copilot
- uploads/utils
Lines changed: 9 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | | - | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
21 | 20 | | |
22 | 21 | | |
23 | 22 | | |
| |||
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
35 | | - | |
36 | | - | |
37 | | - | |
| 34 | + | |
38 | 35 | | |
39 | 36 | | |
40 | 37 | | |
41 | 38 | | |
42 | | - | |
| 39 | + | |
43 | 40 | | |
44 | 41 | | |
45 | 42 | | |
46 | 43 | | |
47 | 44 | | |
48 | | - | |
| 45 | + | |
| 46 | + | |
49 | 47 | | |
50 | 48 | | |
51 | 49 | | |
| |||
62 | 60 | | |
63 | 61 | | |
64 | 62 | | |
65 | | - | |
| 63 | + | |
66 | 64 | | |
67 | 65 | | |
68 | 66 | | |
| |||
83 | 81 | | |
84 | 82 | | |
85 | 83 | | |
86 | | - | |
| 84 | + | |
87 | 85 | | |
88 | 86 | | |
89 | 87 | | |
90 | 88 | | |
91 | 89 | | |
92 | | - | |
| 90 | + | |
93 | 91 | | |
94 | | - | |
| 92 | + | |
95 | 93 | | |
96 | 94 | | |
97 | 95 | | |
| |||
Lines changed: 8 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
1 | 2 | | |
2 | 3 | | |
3 | 4 | | |
| |||
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
9 | | - | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
Lines changed: 128 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
2 | 10 | | |
3 | 11 | | |
4 | 12 | | |
5 | 13 | | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
6 | 19 | | |
7 | 20 | | |
8 | | - | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
9 | 96 | | |
10 | 97 | | |
11 | 98 | | |
| |||
14 | 101 | | |
15 | 102 | | |
16 | 103 | | |
17 | | - | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
18 | 109 | | |
19 | | - | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
20 | 122 | | |
21 | 123 | | |
22 | 124 | | |
| |||
25 | 127 | | |
26 | 128 | | |
27 | 129 | | |
28 | | - | |
29 | | - | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
30 | 150 | | |
31 | 151 | | |
32 | 152 | | |
| |||
36 | 156 | | |
37 | 157 | | |
38 | 158 | | |
| 159 | + | |
| 160 | + | |
0 commit comments