Skip to content

Commit ba2bf7a

Browse files
committed
fix(files): reserve image layout space so images stop reflowing on load
A markdown image with no stored dimensions reserved zero vertical space until it downloaded, then snapped to its natural height and pushed content below it down (cumulative layout shift). Reserve the box up front from the image's intrinsic aspect ratio instead. Store intrinsic width/height as workspace_file metadata (not in the markdown — it stays clean `![](src)`), read it synchronously from the already-loaded file list to reserve a responsive aspect-ratio box on first render, and lazily backfill it once per image on first view via a write-gated, idempotent PATCH. The node view falls back to on-load measurement for the first-ever view and for external images. Images stay fluid (max-width:100%, height:auto).
1 parent 2ba2484 commit ba2bf7a

14 files changed

Lines changed: 18812 additions & 17 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { authMockFns, permissionsMock, permissionsMockFns } from '@sim/testing'
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockUpdateWorkspaceFileDimensions } = vi.hoisted(() => ({
9+
mockUpdateWorkspaceFileDimensions: vi.fn(),
10+
}))
11+
12+
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
13+
updateWorkspaceFileDimensions: mockUpdateWorkspaceFileDimensions,
14+
}))
15+
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
16+
17+
const WS = '7727ef3f-8cf6-4686-b063-2bb006a10785'
18+
const FILE = 'wf_abc123'
19+
20+
import { PATCH } from '@/app/api/workspaces/[id]/files/[fileId]/dimensions/route'
21+
22+
const routeContext = { params: Promise.resolve({ id: WS, fileId: FILE }) }
23+
24+
function buildRequest(body: unknown): NextRequest {
25+
return new NextRequest(`http://localhost/api/workspaces/${WS}/files/${FILE}/dimensions`, {
26+
method: 'PATCH',
27+
headers: { 'content-type': 'application/json' },
28+
body: JSON.stringify(body),
29+
})
30+
}
31+
32+
describe('PATCH /api/workspaces/[id]/files/[fileId]/dimensions', () => {
33+
beforeEach(() => {
34+
vi.clearAllMocks()
35+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
36+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
37+
mockUpdateWorkspaceFileDimensions.mockResolvedValue(true)
38+
})
39+
40+
it('stores dimensions for a writer', async () => {
41+
const res = await PATCH(buildRequest({ width: 1600, height: 900 }), routeContext)
42+
expect(res.status).toBe(200)
43+
expect(await res.json()).toEqual({ success: true })
44+
expect(mockUpdateWorkspaceFileDimensions).toHaveBeenCalledWith(WS, FILE, {
45+
width: 1600,
46+
height: 900,
47+
})
48+
})
49+
50+
it('allows an admin', async () => {
51+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin')
52+
const res = await PATCH(buildRequest({ width: 10, height: 20 }), routeContext)
53+
expect(res.status).toBe(200)
54+
expect(mockUpdateWorkspaceFileDimensions).toHaveBeenCalledOnce()
55+
})
56+
57+
it('rejects an unauthenticated caller before touching the DB', async () => {
58+
authMockFns.mockGetSession.mockResolvedValue(null)
59+
const res = await PATCH(buildRequest({ width: 10, height: 10 }), routeContext)
60+
expect(res.status).toBe(401)
61+
expect(mockUpdateWorkspaceFileDimensions).not.toHaveBeenCalled()
62+
})
63+
64+
it('rejects a read-only member (backfill requires write)', async () => {
65+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read')
66+
const res = await PATCH(buildRequest({ width: 10, height: 10 }), routeContext)
67+
expect(res.status).toBe(403)
68+
expect(mockUpdateWorkspaceFileDimensions).not.toHaveBeenCalled()
69+
})
70+
71+
it('rejects non-positive / non-integer dimensions', async () => {
72+
for (const body of [
73+
{ width: 0, height: 10 },
74+
{ width: 10, height: -5 },
75+
{ width: 10.5, height: 10 },
76+
{ width: 10 },
77+
]) {
78+
const res = await PATCH(buildRequest(body), routeContext)
79+
expect(res.status).toBe(400)
80+
}
81+
expect(mockUpdateWorkspaceFileDimensions).not.toHaveBeenCalled()
82+
})
83+
})
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { createLogger } from '@sim/logger'
2+
import { getErrorMessage } from '@sim/utils/errors'
3+
import { type NextRequest, NextResponse } from 'next/server'
4+
import { updateWorkspaceFileDimensionsContract } from '@/lib/api/contracts/workspace-files'
5+
import { parseRequest } from '@/lib/api/server'
6+
import { getSession } from '@/lib/auth'
7+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8+
import { updateWorkspaceFileDimensions } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
9+
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
10+
11+
const logger = createLogger('WorkspaceFileDimensionsAPI')
12+
13+
/**
14+
* PATCH /api/workspaces/[id]/files/[fileId]/dimensions
15+
*
16+
* Backfill an image file's intrinsic pixel dimensions — a pure rendering hint the editor uses to reserve
17+
* layout space before the image loads. Requires write permission and is idempotent (a no-op once the
18+
* dimensions are already stored), so the client can fire it once per image without coordination.
19+
*/
20+
export const PATCH = withRouteHandler(
21+
async (request: NextRequest, context: { params: Promise<{ id: string; fileId: string }> }) => {
22+
const session = await getSession()
23+
if (!session?.user?.id) {
24+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
25+
}
26+
27+
const parsed = await parseRequest(updateWorkspaceFileDimensionsContract, request, context)
28+
if (!parsed.success) return parsed.response
29+
const { id: workspaceId, fileId } = parsed.data.params
30+
const { width, height } = parsed.data.body
31+
32+
const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
33+
if (permission !== 'admin' && permission !== 'write') {
34+
return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
35+
}
36+
37+
try {
38+
await updateWorkspaceFileDimensions(workspaceId, fileId, { width, height })
39+
return NextResponse.json({ success: true as const })
40+
} catch (error) {
41+
logger.error('Failed to backfill workspace file dimensions', {
42+
workspaceId,
43+
fileId,
44+
error: getErrorMessage(error),
45+
})
46+
return NextResponse.json({ error: 'Failed to update dimensions' }, { status: 500 })
47+
}
48+
}
49+
)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { Music } from '@sim/emcn/icons'
55
import dynamic from 'next/dynamic'
66
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
77
import { getFileExtension } from '@/lib/uploads/utils/file-utils'
8-
import { useWorkspaceFileBinary, useWorkspaceFileContent } from '@/hooks/queries/workspace-files'
8+
import {
9+
useWorkspaceFileBinary,
10+
useWorkspaceFileContent,
11+
useWorkspaceImageDimensionsAdapter,
12+
} from '@/hooks/queries/workspace-files'
913
import {
1014
createWorkspaceFileContentSource,
1115
type FileContentSource,
@@ -126,9 +130,10 @@ interface FileViewerProps {
126130

127131
export function FileViewer(props: FileViewerProps) {
128132
const { contentSource, workspaceId } = props
133+
const imageDimensions = useWorkspaceImageDimensionsAdapter(workspaceId)
129134
const source = useMemo(
130-
() => contentSource ?? createWorkspaceFileContentSource(workspaceId),
131-
[contentSource, workspaceId]
135+
() => contentSource ?? createWorkspaceFileContentSource(workspaceId, imageDimensions),
136+
[contentSource, workspaceId, imageDimensions]
132137
)
133138
return (
134139
<FileContentSourceProvider value={source}>

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.tsx

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { useEffect, useRef, useState } from 'react'
1+
import { type CSSProperties, useEffect, useMemo, useRef, useState } from 'react'
22
import { cn } from '@sim/emcn'
33
import { NodeSelection, Plugin } from '@tiptap/pm/state'
44
import type { ReactNodeViewProps } from '@tiptap/react'
55
import { NodeViewWrapper, ReactNodeViewRenderer } from '@tiptap/react'
6-
import { useFileContentSource } from '@/hooks/use-file-content-source'
6+
import { type ImageDimensions, useFileContentSource } from '@/hooks/use-file-content-source'
77
import { MarkdownImage } from './image-schema'
88
import { normalizeLinkHref } from './markdown-fidelity'
99
import { useEditorEditable } from './use-editor-editable'
@@ -24,6 +24,11 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
2424
const [dragWidth, setDragWidth] = useState<number | null>(null)
2525
/** Whether the current src failed to load; reset on src change so a retried/edited src can load. */
2626
const [failed, setFailed] = useState(false)
27+
/**
28+
* Intrinsic dimensions measured from the loaded image — holds the aspect-ratio box for THIS view when
29+
* the content source has no stored dimensions yet (the first-ever view of an image). Reset on src change.
30+
*/
31+
const [measuredDimensions, setMeasuredDimensions] = useState<ImageDimensions | null>(null)
2732
const attrs = node.attrs as {
2833
src?: string
2934
alt?: string
@@ -33,7 +38,16 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
3338
}
3439

3540
useEffect(() => () => dragAbortRef.current?.abort(), [])
36-
useEffect(() => setFailed(false), [attrs.src])
41+
42+
// Reset the load-failure flag and this-session measurement when the src changes — adjusted during
43+
// render (not in an effect) so the previous image's aspect-ratio box never paints for a frame. A `key`
44+
// remount isn't available here: TipTap owns this node view's instantiation.
45+
const [prevSrc, setPrevSrc] = useState(attrs.src)
46+
if (prevSrc !== attrs.src) {
47+
setPrevSrc(attrs.src)
48+
setFailed(false)
49+
setMeasuredDimensions(null)
50+
}
3751

3852
const startResize = (event: React.PointerEvent) => {
3953
event.preventDefault()
@@ -73,12 +87,27 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
7387
? `${attrs.width}px`
7488
: attrs.width
7589
: undefined
76-
const widthStyle =
90+
// Stored intrinsic dimensions reserve the box on the very first render. Memoized on the src (not the
91+
// live drag width) so a resize drag never re-scans the file list. Falls back to what we measured on
92+
// load this session for a first-ever view the metadata hasn't caught up on.
93+
const storedDimensions = useMemo(
94+
() => source.getImageDimensions?.(attrs.src) ?? null,
95+
[source, attrs.src]
96+
)
97+
const intrinsicDimensions = storedDimensions ?? measuredDimensions
98+
const displayWidth =
7799
dragWidth !== null
78-
? { width: `${dragWidth}px` }
79-
: committedWidth
80-
? { width: committedWidth }
81-
: undefined
100+
? `${dragWidth}px`
101+
: (committedWidth ?? (intrinsicDimensions ? `${intrinsicDimensions.width}px` : undefined))
102+
// width + aspect-ratio (with `max-w-full`/`h-auto` from the class list) reserves a responsive box the
103+
// image can't reflow into, per the CLS-avoidance pattern for known-ratio responsive images. React drops
104+
// the undefined keys, so an unmeasured image simply gets no reservation (its prior behavior).
105+
const imageStyle: CSSProperties = {
106+
width: displayWidth,
107+
aspectRatio: intrinsicDimensions
108+
? `${intrinsicDimensions.width} / ${intrinsicDimensions.height}`
109+
: undefined,
110+
}
82111

83112
// Sanitize the linked-image target before rendering the anchor — a parsed markdown href is
84113
// untrusted and could be `javascript:`/`data:`; an unsafe value drops the link (image only).
@@ -99,11 +128,21 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
99128
// the resize button sits outside this element, so it keeps its own pointer behavior.)
100129
draggable={editable}
101130
data-drag-handle={editable ? '' : undefined}
102-
style={widthStyle}
131+
style={imageStyle}
103132
onError={() => setFailed(true)}
104-
onLoad={() => setFailed(false)}
133+
onLoad={(event) => {
134+
setFailed(false)
135+
const { naturalWidth, naturalHeight } = event.currentTarget
136+
if (naturalWidth <= 0 || naturalHeight <= 0) return
137+
// Already reserved from stored metadata (possibly just backfilled by a sibling view) — done.
138+
if (source.getImageDimensions?.(attrs.src)) return
139+
// Hold the box for this first-ever view, and persist so later views reserve before load. The
140+
// report is idempotent and de-duped downstream (stored-check + server `width IS NULL`).
141+
setMeasuredDimensions({ width: naturalWidth, height: naturalHeight })
142+
source.reportImageDimensions?.(attrs.src, { width: naturalWidth, height: naturalHeight })
143+
}}
105144
className={cn(
106-
'block max-w-full rounded-lg border border-[var(--border)]',
145+
'block h-auto max-w-full rounded-lg border border-[var(--border)]',
107146
editable && 'cursor-grab',
108147
failed &&
109148
'min-h-[72px] min-w-[140px] bg-[var(--surface-5)] p-3 text-[var(--text-muted)] text-caption'
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
6+
import { findWorkspaceFileBySrc } from './find-workspace-file-by-src'
7+
8+
function record(over: Partial<WorkspaceFileRecord>): WorkspaceFileRecord {
9+
return { id: 'wf_x', key: 'workspace/ws1/x.png', ...over } as WorkspaceFileRecord
10+
}
11+
12+
const records = [
13+
record({ id: 'wf_a', key: 'workspace/ws1/a.png' }),
14+
record({ id: 'wf_b', key: 'workspace/ws1/b.png' }),
15+
]
16+
17+
const serveUrl = (key: string) => `/api/files/serve/${encodeURIComponent(key)}?context=workspace`
18+
19+
describe('findWorkspaceFileBySrc', () => {
20+
it('matches a serve URL by storage key', () => {
21+
expect(findWorkspaceFileBySrc(records, serveUrl('workspace/ws1/b.png'))?.id).toBe('wf_b')
22+
})
23+
24+
it('matches a /api/files/view/<id> URL by file id', () => {
25+
expect(findWorkspaceFileBySrc(records, '/api/files/view/wf_a')?.id).toBe('wf_a')
26+
})
27+
28+
it('matches a /workspace/<ws>/files/<id> URL by file id', () => {
29+
expect(findWorkspaceFileBySrc(records, '/workspace/ws1/files/wf_b')?.id).toBe('wf_b')
30+
})
31+
32+
it('returns undefined for a serve URL whose key is not in the list', () => {
33+
expect(findWorkspaceFileBySrc(records, serveUrl('workspace/ws1/missing.png'))).toBeUndefined()
34+
})
35+
36+
it('returns undefined for external, data:, and undefined srcs', () => {
37+
expect(findWorkspaceFileBySrc(records, 'https://example.com/x.png')).toBeUndefined()
38+
expect(findWorkspaceFileBySrc(records, 'data:image/png;base64,AAAA')).toBeUndefined()
39+
expect(findWorkspaceFileBySrc(records, undefined)).toBeUndefined()
40+
})
41+
42+
it('returns undefined when the file list has not loaded yet', () => {
43+
expect(findWorkspaceFileBySrc(undefined, '/api/files/view/wf_a')).toBeUndefined()
44+
})
45+
})
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
2+
import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref'
3+
4+
/**
5+
* Resolve the workspace file record an embedded image `src` points at, matching the persisted serve-URL
6+
* shape by storage key or file id. Returns `undefined` for external / `data:` / unrecognized srcs, and
7+
* when the file list isn't loaded — callers then fall back to on-load measurement rather than reserving
8+
* from metadata.
9+
*/
10+
export function findWorkspaceFileBySrc(
11+
records: WorkspaceFileRecord[] | undefined,
12+
src: string | undefined
13+
): WorkspaceFileRecord | undefined {
14+
const ref = src ? extractEmbeddedFileRef(src) : null
15+
if (!ref || !records) return undefined
16+
return 'key' in ref
17+
? records.find((record) => record.key === ref.key)
18+
: records.find((record) => record.id === ref.fileId)
19+
}

apps/sim/hooks/queries/workspace-files.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useMemo } from 'react'
12
import { toast } from '@sim/emcn'
23
import { createLogger } from '@sim/logger'
34
import { toError } from '@sim/utils/errors'
@@ -15,6 +16,7 @@ import {
1516
renameWorkspaceFileContract,
1617
restoreWorkspaceFileContract,
1718
updateWorkspaceFileContentContract,
19+
updateWorkspaceFileDimensionsContract,
1820
} from '@/lib/api/contracts/workspace-files'
1921
import {
2022
DirectUploadError,
@@ -23,7 +25,8 @@ import {
2325
} from '@/lib/uploads/client/direct-upload'
2426
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
2527
import type { UserFile } from '@/executor/types'
26-
import { useFileContentSource } from '@/hooks/use-file-content-source'
28+
import { findWorkspaceFileBySrc } from '@/hooks/queries/utils/find-workspace-file-by-src'
29+
import { type ImageDimensionsSource, useFileContentSource } from '@/hooks/use-file-content-source'
2730

2831
const logger = createLogger('WorkspaceFilesQuery')
2932

@@ -123,6 +126,47 @@ export function useWorkspaceFiles(
123126
})
124127
}
125128

129+
/**
130+
* Back the file content source's image-dimension capability with workspace file metadata. Reads intrinsic
131+
* dimensions straight from the already-loaded active file list (synchronous, so a stored image reserves
132+
* its box on the first render), and lazily backfills them once per image on first measurement. The
133+
* backfill is fire-and-forget and idempotent: the optimistic cache patch makes a second measurement of
134+
* the same file a no-op locally, and the server ignores an already-populated row — so it never storms,
135+
* never blocks render, and never touches the collaborative document.
136+
*/
137+
export function useWorkspaceImageDimensionsAdapter(workspaceId: string): ImageDimensionsSource {
138+
const queryClient = useQueryClient()
139+
return useMemo<ImageDimensionsSource>(() => {
140+
const listKey = workspaceFilesKeys.list(workspaceId, 'active')
141+
const findRecord = (src: string | undefined): WorkspaceFileRecord | undefined =>
142+
findWorkspaceFileBySrc(queryClient.getQueryData<WorkspaceFileRecord[]>(listKey), src)
143+
return {
144+
getImageDimensions: (src) => {
145+
const record = findRecord(src)
146+
return record?.width != null && record.height != null
147+
? { width: record.width, height: record.height }
148+
: null
149+
},
150+
reportImageDimensions: (src, dimensions) => {
151+
const record = findRecord(src)
152+
// Skip when the file isn't ours to key (external/unlisted) or its dimensions are already stored.
153+
if (!record || (record.width != null && record.height != null)) return
154+
const patch = (width: number | null, height: number | null) =>
155+
queryClient.setQueryData<WorkspaceFileRecord[]>(listKey, (previous) =>
156+
previous?.map((entry) => (entry.id === record.id ? { ...entry, width, height } : entry))
157+
)
158+
// Optimistically populate the cache so this and sibling views reserve space immediately and a
159+
// concurrent measurement of the same file short-circuits above.
160+
patch(dimensions.width, dimensions.height)
161+
void requestJson(updateWorkspaceFileDimensionsContract, {
162+
params: { id: workspaceId, fileId: record.id },
163+
body: dimensions,
164+
}).catch(() => patch(null, null))
165+
},
166+
}
167+
}, [queryClient, workspaceId])
168+
}
169+
126170
/**
127171
* Fetch file content as text via a content-source URL
128172
*/

0 commit comments

Comments
 (0)