Skip to content

Commit 35efcff

Browse files
authored
fix(files): preserve image selection after resizing (#7629)
1 parent c8ef4c1 commit 35efcff

3 files changed

Lines changed: 120 additions & 37 deletions

File tree

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,64 @@ async function setNestedImages(depth: number): Promise<void> {
156156
}
157157

158158
describe('image resizing during real peer Yjs updates', () => {
159+
it.each(['block', 'heading', 'paragraph'])(
160+
'keeps the resized %s image selected for deletion and undo without changing peer text',
161+
async (placement) => {
162+
const image = '<img src="/logo.png" alt="Original" width="200" height="100">'
163+
await act(async () => {
164+
local.commands.setContent(
165+
placement === 'block'
166+
? `<h2>Before</h2>${image}<p>After</p>`
167+
: `<${placement === 'heading' ? 'h2' : 'p'}>Before ${image} after</${placement === 'heading' ? 'h2' : 'p'}>`
168+
)
169+
Y.applyUpdate(peerDoc, Y.encodeStateAsUpdate(localDoc))
170+
local.commands.setNodeSelection(imagePosition(local))
171+
})
172+
const undoManager = yUndoPluginKey.getState(local.state).undoManager
173+
undoManager.clear()
174+
beginResize()
175+
peer.commands.insertContentAt(1, 'Peer ')
176+
await receivePeerUpdate()
177+
const text = local.state.doc.textContent
178+
const onUpdate = vi.fn()
179+
local.on('update', onUpdate)
180+
181+
pointer(window, 'pointerup', 160)
182+
183+
expect(onUpdate).toHaveBeenCalledOnce()
184+
expect(local.state.selection).toBeInstanceOf(NodeSelection)
185+
expect(local.state.selection.from).toBe(imagePosition(local))
186+
expect(host.querySelector('.ProseMirror-selectednode img')).not.toBeNull()
187+
expect(imageAttributes(local)).toMatchObject({ width: '260', height: null })
188+
expect(local.state.doc.textContent).toBe(text)
189+
await act(async () => {
190+
Y.applyUpdate(peerDoc, Y.encodeStateAsUpdate(localDoc))
191+
expect(local.commands.undo()).toBe(true)
192+
})
193+
expect(imageAttributes(local)).toMatchObject({ width: '200', height: '100' })
194+
expect(local.state.doc.textContent).toBe(text)
195+
expect(local.can().undo()).toBe(false)
196+
await act(async () => {
197+
expect(local.commands.redo()).toBe(true)
198+
})
199+
expect(imageAttributes(local)).toMatchObject({ width: '260', height: null })
200+
201+
undoManager.stopCapturing()
202+
await act(async () => {
203+
expect(local.commands.keyboardShortcut('Backspace')).toBe(true)
204+
})
205+
expect(imageAttributes(local)).toBeNull()
206+
expect(local.state.doc.textContent).toBe(text)
207+
await act(async () => {
208+
expect(local.commands.undo()).toBe(true)
209+
Y.applyUpdate(peerDoc, Y.encodeStateAsUpdate(localDoc))
210+
})
211+
expect(imageAttributes(local)).toMatchObject({ width: '260', height: null })
212+
expect(local.getJSON()).toEqual(peer.getJSON())
213+
expect(local.state.doc.textContent).toBe(text)
214+
}
215+
)
216+
159217
it.each(['heading', 'paragraph'])(
160218
'renders and scrolls a valid selection when undoing a move into a %s',
161219
async (target) => {

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

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
/** @vitest-environment jsdom */
22
import { act } from 'react'
3+
import { Editor } from '@tiptap/core'
34
import type { ReactNodeViewProps } from '@tiptap/react'
5+
import StarterKit from '@tiptap/starter-kit'
46
import { createRoot, type Root } from 'react-dom/client'
57
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
68

@@ -26,23 +28,24 @@ vi.mock(
2628
)
2729

2830
import { ResizableImageView } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image'
31+
import { MarkdownImage } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-schema'
2932

3033
let host: HTMLDivElement
3134
let root: Root
32-
const editor = { isEditable: true, isDestroyed: false, commands: { focus: vi.fn() } }
35+
let editor: Editor
3336

3437
beforeEach(() => {
3538
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true })
3639
vi.clearAllMocks()
37-
editor.isEditable = true
38-
editor.isDestroyed = false
40+
editor = new Editor({ extensions: [StarterKit, MarkdownImage] })
3941
host = document.createElement('div')
4042
document.body.append(host)
4143
root = createRoot(host)
4244
})
4345

4446
afterEach(() => {
4547
act(() => root.unmount())
48+
editor.destroy()
4649
host.remove()
4750
})
4851

@@ -61,22 +64,31 @@ function pointerEvent(
6164
}
6265

6366
function renderImage(
64-
updateAttributes: ReturnType<typeof vi.fn>,
65-
dimensions: { width?: string | null; height?: string | null } = {}
67+
onUpdate: ReturnType<typeof vi.fn>,
68+
dimensions: { width?: string | null; height?: string | null } = {},
69+
getPos: ReactNodeViewProps['getPos'] = () => 0
6670
): HTMLButtonElement {
67-
const props = {
68-
node: {
69-
attrs: {
70-
src: '/image.png',
71-
alt: '',
72-
title: null,
73-
width: null,
74-
height: '100',
75-
...dimensions,
76-
href: null,
71+
editor.commands.setContent({
72+
type: 'doc',
73+
content: [
74+
{
75+
type: 'image',
76+
attrs: {
77+
src: '/image.png',
78+
alt: '',
79+
title: null,
80+
width: null,
81+
height: '100',
82+
...dimensions,
83+
href: null,
84+
},
7785
},
78-
},
79-
updateAttributes,
86+
],
87+
})
88+
editor.on('update', onUpdate)
89+
const props = {
90+
node: editor.state.doc.firstChild,
91+
getPos,
8092
selected: true,
8193
editor,
8294
} as unknown as ReactNodeViewProps
@@ -150,42 +162,56 @@ describe('ResizableImageView', () => {
150162
)
151163

152164
it('commits one proportional width change and clears a stale explicit height', () => {
153-
const updateAttributes = vi.fn()
154-
const handle = renderImage(updateAttributes)
165+
const onUpdate = vi.fn()
166+
const handle = renderImage(onUpdate)
155167

156168
act(() => handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 7, clientX: 100 })))
157169
act(() => window.dispatchEvent(pointerEvent('pointermove', { pointerId: 7, clientX: 160 })))
158170
act(() => window.dispatchEvent(pointerEvent('pointerup', { pointerId: 7, clientX: 160 })))
159171

160-
expect(updateAttributes).toHaveBeenCalledOnce()
161-
expect(updateAttributes).toHaveBeenCalledWith({ width: '260', height: null })
172+
expect(onUpdate).toHaveBeenCalledOnce()
173+
expect(editor.state.doc.firstChild?.attrs).toMatchObject({ width: '260', height: null })
162174
})
163175

164176
it('ignores unrelated pointers and cancels without mutating document attributes', () => {
165-
const updateAttributes = vi.fn()
166-
const handle = renderImage(updateAttributes)
177+
const onUpdate = vi.fn()
178+
const handle = renderImage(onUpdate)
167179

168180
act(() => handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 7, clientX: 100 })))
169181
act(() => window.dispatchEvent(pointerEvent('pointermove', { pointerId: 8, clientX: 180 })))
170182
act(() => window.dispatchEvent(pointerEvent('pointerup', { pointerId: 8, clientX: 180 })))
171183
act(() => window.dispatchEvent(pointerEvent('pointercancel', { pointerId: 7 })))
172-
expect(updateAttributes).not.toHaveBeenCalled()
184+
expect(onUpdate).not.toHaveBeenCalled()
173185

174186
act(() => handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 9, clientX: 100 })))
175187
act(() => window.dispatchEvent(pointerEvent('pointermove', { pointerId: 9, clientX: 140 })))
176188
act(() => window.dispatchEvent(new Event('blur')))
177-
expect(updateAttributes).not.toHaveBeenCalled()
189+
expect(onUpdate).not.toHaveBeenCalled()
178190
})
179191

180192
it('does not commit a resize after live editing becomes unavailable', () => {
181-
const updateAttributes = vi.fn()
182-
const handle = renderImage(updateAttributes)
193+
const onUpdate = vi.fn()
194+
const handle = renderImage(onUpdate)
195+
196+
act(() => handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 7, clientX: 100 })))
197+
act(() => window.dispatchEvent(pointerEvent('pointermove', { pointerId: 7, clientX: 160 })))
198+
editor.setEditable(false, false)
199+
act(() => window.dispatchEvent(pointerEvent('pointerup', { pointerId: 7, clientX: 160 })))
200+
201+
expect(onUpdate).not.toHaveBeenCalled()
202+
})
203+
204+
it('does not commit a resize when the node view no longer has a position', () => {
205+
const onUpdate = vi.fn()
206+
const getPos = vi.fn<ReactNodeViewProps['getPos']>(() => 0)
207+
const handle = renderImage(onUpdate, {}, getPos)
183208

184209
act(() => handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 7, clientX: 100 })))
185210
act(() => window.dispatchEvent(pointerEvent('pointermove', { pointerId: 7, clientX: 160 })))
186-
editor.isEditable = false
211+
getPos.mockReturnValue(undefined)
187212
act(() => window.dispatchEvent(pointerEvent('pointerup', { pointerId: 7, clientX: 160 })))
188213

189-
expect(updateAttributes).not.toHaveBeenCalled()
214+
expect(onUpdate).not.toHaveBeenCalled()
215+
expect(editor.state.doc.firstChild?.attrs).toMatchObject({ width: null, height: '100' })
190216
})
191217
})

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,7 @@ const PIXEL_SIZE = /^\d+(?:\.\d+)?px$/
2424
* Drag-to-resize image node view (handle at the bottom-right, revealed on selection). Dragging
2525
* commits the new pixel width to the `width` attribute, which serializes to `<img width>`.
2626
*/
27-
export function ResizableImageView({
28-
node,
29-
updateAttributes,
30-
selected,
31-
editor,
32-
getPos,
33-
}: ReactNodeViewProps) {
27+
export function ResizableImageView({ node, selected, editor, getPos }: ReactNodeViewProps) {
3428
const source = useFileContentSource()
3529
const imageRef = useRef<HTMLImageElement>(null)
3630
const dragAbortRef = useRef<AbortController | null>(null)
@@ -122,7 +116,12 @@ export function ResizableImageView({
122116
!editor.isDestroyed &&
123117
isCurrentTarget()
124118
) {
125-
updateAttributes({ width: String(finalWidth), height: null })
119+
const position = getPos()
120+
if (typeof position !== 'number') return
121+
const tr = editor.state.tr
122+
.setNodeAttribute(position, 'width', String(finalWidth))
123+
.setNodeAttribute(position, 'height', null)
124+
editor.view.dispatch(tr.setSelection(NodeSelection.create(tr.doc, position)))
126125
}
127126
}
128127
if (binding) {

0 commit comments

Comments
 (0)