Skip to content

Commit 185e24d

Browse files
waleedlatif1waleedlatif1
authored
feat(files): support heading images and simplify image selection (#7597)
* feat(files): support heading images and simplify image selection * fix(files): preserve drag targets and valid collaborative selections
1 parent b3eb2ca commit 185e24d

35 files changed

Lines changed: 1275 additions & 1034 deletions

‎apps/realtime/src/handlers/file-doc.join-readiness.test.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import {
1313
FILE_DOC_EVENTS,
1414
FILE_DOC_MESSAGE_TYPE,
15+
FILE_DOC_SCHEMA_VERSION,
1516
FILE_DOC_SEED,
1617
} from '@sim/realtime-protocol/file-doc'
1718
import * as decoding from 'lib0/decoding'
@@ -272,7 +273,15 @@ describe('file-doc join readiness (shared store enabled)', () => {
272273
backing.readDelayTicks = 6
273274
const { socket, handlers } = setup('socket-1', sockets)
274275

275-
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: FILE_ID, clientId: 1 })
276+
await handlers[FILE_DOC_EVENTS.JOIN]({
277+
fileId: FILE_ID,
278+
clientId: 1,
279+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
280+
})
281+
expect(socket.emit).toHaveBeenCalledWith(
282+
FILE_DOC_EVENTS.JOIN_SUCCESS,
283+
expect.objectContaining({ fileId: FILE_ID, schemaVersion: FILE_DOC_SCHEMA_VERSION })
284+
)
276285
requestSyncStep2(handlers)
277286
await flushPendingWork()
278287

@@ -283,9 +292,17 @@ describe('file-doc join readiness (shared store enabled)', () => {
283292

284293
it('does not fetch a seed for a room the stream can already reconstruct', async () => {
285294
seedWarmStreamHistory()
286-
const { handlers } = setup('socket-1', sockets)
295+
const { socket, handlers } = setup('socket-1', sockets)
287296

288-
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: FILE_ID, clientId: 1 })
297+
await handlers[FILE_DOC_EVENTS.JOIN]({
298+
fileId: FILE_ID,
299+
clientId: 1,
300+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
301+
})
302+
expect(socket.emit).toHaveBeenCalledWith(
303+
FILE_DOC_EVENTS.JOIN_SUCCESS,
304+
expect.objectContaining({ fileId: FILE_ID, schemaVersion: FILE_DOC_SCHEMA_VERSION })
305+
)
289306

290307
expect(mockFetchFileDocSeed).not.toHaveBeenCalled()
291308
})
@@ -302,7 +319,15 @@ describe('file-doc join readiness (shared store enabled)', () => {
302319
doc.destroy()
303320

304321
const { socket, handlers } = setup('socket-1', sockets)
305-
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: FILE_ID, clientId: 1 })
322+
await handlers[FILE_DOC_EVENTS.JOIN]({
323+
fileId: FILE_ID,
324+
clientId: 1,
325+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
326+
})
327+
expect(socket.emit).toHaveBeenCalledWith(
328+
FILE_DOC_EVENTS.JOIN_SUCCESS,
329+
expect.objectContaining({ fileId: FILE_ID, schemaVersion: FILE_DOC_SCHEMA_VERSION })
330+
)
306331
requestSyncStep2(handlers)
307332
await flushPendingWork()
308333

‎apps/realtime/src/handlers/file-doc.test.ts‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
FILE_DOC_EVENTS,
66
FILE_DOC_LIMITS,
77
FILE_DOC_MESSAGE_TYPE,
8+
FILE_DOC_SCHEMA_VERSION,
89
FILE_DOC_SEED,
910
} from '@sim/realtime-protocol/file-doc'
1011
import { ROOM_TYPES } from '@sim/realtime-protocol/rooms'
@@ -103,7 +104,14 @@ function createSocket(id: string, overrides?: Record<string, unknown>) {
103104
userImage: 'avatar.png',
104105
disconnected: false,
105106
on: vi.fn((event: string, handler: Handler) => {
106-
handlers[event] = handler
107+
handlers[event] =
108+
event === FILE_DOC_EVENTS.JOIN
109+
? (payload) =>
110+
handler({
111+
schemaVersion: FILE_DOC_SCHEMA_VERSION,
112+
...(payload as Record<string, unknown>),
113+
})
114+
: handler
107115
}),
108116
emit: vi.fn(),
109117
join: vi.fn(),
@@ -281,22 +289,25 @@ describe('setupWorkspaceFileDocHandlers', () => {
281289
expect(mockAuthorizeRoom).not.toHaveBeenCalled()
282290
})
283291

284-
it('rejects an incompatible collaborative-document schema before authorizing', async () => {
285-
const { io } = createIo()
286-
const { socket, handlers } = setup('socket-schema', io)
292+
it.each([undefined, 1, 99])(
293+
'rejects incompatible schema %s before authorizing',
294+
async (schemaVersion) => {
295+
const { io } = createIo()
296+
const { socket, handlers } = setup('socket-schema', io)
287297

288-
await handlers[FILE_DOC_EVENTS.JOIN]({
289-
fileId: 'file-1',
290-
clientId: 1,
291-
schemaVersion: 99,
292-
})
298+
await handlers[FILE_DOC_EVENTS.JOIN]({
299+
fileId: 'file-1',
300+
clientId: 1,
301+
schemaVersion,
302+
})
293303

294-
expect(socket.emit).toHaveBeenCalledWith(
295-
FILE_DOC_EVENTS.JOIN_ERROR,
296-
expect.objectContaining({ code: 'SCHEMA_VERSION_MISMATCH', retryable: false })
297-
)
298-
expect(mockAuthorizeRoom).not.toHaveBeenCalled()
299-
})
304+
expect(socket.emit).toHaveBeenCalledWith(
305+
FILE_DOC_EVENTS.JOIN_ERROR,
306+
expect.objectContaining({ code: 'SCHEMA_VERSION_MISMATCH', retryable: false })
307+
)
308+
expect(mockAuthorizeRoom).not.toHaveBeenCalled()
309+
}
310+
)
300311

301312
it('acknowledges user updates only after applying them to the joined document', async () => {
302313
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Original', 'doc-1'))

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/block-image-paragraph.test.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,22 @@ const cleanups: Array<() => void> = []
3636
afterEach(() => cleanups.splice(0).forEach((cleanup) => cleanup()))
3737

3838
describe('block images within Markdown paragraphs', () => {
39+
it('retains a whitespace-only code span beside an image', () => {
40+
const paragraph: JSONContent = {
41+
type: 'paragraph',
42+
content: [
43+
{ type: 'image', attrs: { src: '/image.png' } },
44+
{ type: 'text', text: ' ', marks: [{ type: 'code' }] },
45+
],
46+
}
47+
expect(splitBlockImageParagraph(paragraph)).toEqual([
48+
{
49+
...paragraph,
50+
content: [{ type: 'inlineImage', attrs: { src: '/image.png' } }, paragraph.content![1]],
51+
},
52+
])
53+
})
54+
3955
it('does not mutate parsed nodes or trim meaningful code-span whitespace', () => {
4056
const paragraph: JSONContent = {
4157
type: 'paragraph',
@@ -50,22 +66,37 @@ describe('block images within Markdown paragraphs', () => {
5066
const original = structuredClone(paragraph)
5167
const blocks = splitBlockImageParagraph(paragraph)
5268
expect(paragraph).toEqual(original)
53-
expect(blocks[0].content).toEqual([{ type: 'text', text: 'Before', marks: [{ type: 'bold' }] }])
54-
expect(blocks[1]).toBe(paragraph.content?.[1])
55-
expect(blocks[2].content).toEqual([{ type: 'text', text: ' code ', marks: [{ type: 'code' }] }])
69+
expect(blocks).toHaveLength(1)
70+
expect(blocks[0].content).toEqual(
71+
paragraph.content?.map((child) =>
72+
child.type === 'image' ? { ...child, type: 'inlineImage' } : child
73+
)
74+
)
5675
})
5776

5877
it('preserves source order, text marks, and linked image dimensions and titles', () => {
5978
const markdown =
6079
'**Before** [<img src="https://example.test/sized.png" alt="Sized preview" width="320" height="180" title="Image title">](https://example.test/target "Link title") *after* ![Second](https://example.test/second.png) ` done `'
61-
const doc = schema.nodeFromJSON(parseMarkdownToDoc(markdown))
80+
const root = schema.nodeFromJSON(parseMarkdownToDoc(markdown))
81+
expect(root.childCount).toBe(1)
82+
const doc = root.child(0)
6283
expect(() => doc.check()).not.toThrow()
6384
expect(
6485
Array.from({ length: doc.childCount }, (_, index) => doc.child(index).type.name)
65-
).toEqual(['paragraph', 'image', 'paragraph', 'image', 'paragraph'])
86+
).toEqual([
87+
'text',
88+
'text',
89+
'inlineImage',
90+
'text',
91+
'text',
92+
'text',
93+
'inlineImage',
94+
'text',
95+
'text',
96+
])
6697
expect(doc.child(0).textContent).toBe('Before')
67-
expect(doc.child(0).firstChild?.marks.map((mark) => mark.type.name)).toEqual(['bold'])
68-
expect(doc.child(1).attrs).toMatchObject({
98+
expect(doc.child(0).marks.map((mark) => mark.type.name)).toEqual(['bold'])
99+
expect(doc.child(2).attrs).toMatchObject({
69100
src: 'https://example.test/sized.png',
70101
alt: 'Sized preview',
71102
width: '320',
@@ -74,23 +105,25 @@ describe('block images within Markdown paragraphs', () => {
74105
href: 'https://example.test/target',
75106
hrefTitle: 'Link title',
76107
})
77-
expect(doc.child(2).textContent).toBe('after')
78-
expect(doc.child(2).firstChild?.marks.map((mark) => mark.type.name)).toEqual(['italic'])
79-
expect(doc.child(3).attrs.src).toBe('https://example.test/second.png')
80-
expect(doc.child(4).textContent).toBe('done')
81-
expect(doc.child(4).firstChild?.marks.map((mark) => mark.type.name)).toEqual(['code'])
108+
expect(doc.child(4).textContent).toBe('after')
109+
expect(doc.child(4).marks.map((mark) => mark.type.name)).toEqual(['italic'])
110+
expect(doc.child(6).attrs.src).toBe('https://example.test/second.png')
111+
expect(doc.child(8).textContent).toBe('done')
112+
expect(doc.child(8).marks.map((mark) => mark.type.name)).toEqual(['code'])
82113
const serialized = serializeMarkdownBody(markdown)
83-
expect(schema.nodeFromJSON(parseMarkdownToDoc(serialized)).toJSON()).toEqual(doc.toJSON())
114+
expect(schema.nodeFromJSON(parseMarkdownToDoc(serialized)).toJSON()).toEqual(root.toJSON())
84115
expect(serializeMarkdownBody(serialized)).toBe(serialized)
85116
})
86117

87-
it.each(CASES)('keeps %s schema-valid and stable through save and reopen', (_label, markdown) => {
118+
it.each(CASES)('keeps %s schema-valid and stable through save and reopen', (label, markdown) => {
88119
const parsed = schema.nodeFromJSON(parseMarkdownToDoc(markdown))
89120
expect(() => parsed.check()).not.toThrow()
90121
const serialized = serializeMarkdownBody(markdown)
91122
const reparsed = schema.nodeFromJSON(parseMarkdownToDoc(serialized))
92123
expect(() => reparsed.check()).not.toThrow()
93-
expect(reparsed.toJSON()).toEqual(parsed.toJSON())
124+
if (label !== 'marks spanning an image') expect(reparsed.toJSON()).toEqual(parsed.toJSON())
125+
else expect(serialized).toBe(`**Before** ${IMAGE} **after**`)
126+
expect(reparsed.textContent).toBe(parsed.textContent)
94127
expect(serializeMarkdownBody(serialized)).toBe(serialized)
95128
expect(isRoundTripSafe(markdown)).toBe(true)
96129
})

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/block-image-paragraph.ts‎

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,25 @@
11
import type { JSONContent } from '@tiptap/core'
22

3-
/** Images are block nodes in the shared schema, even when Markdown places them beside text. */
3+
/** Image-only paragraphs remain blocks; images beside text need an inline representation. */
44
export function splitBlockImageParagraph(node: JSONContent): JSONContent[] {
55
if (node.type !== 'paragraph' || !node.content?.some((child) => child.type === 'image')) {
66
return [node]
77
}
8-
const blocks: JSONContent[] = []
9-
let inline: JSONContent[] = []
10-
const flush = () => {
11-
/** Whitespace beside a block image becomes paragraph padding, not visible inline content. */
12-
let start = 0
13-
let end = inline.length - 1
14-
for (const leading of [true, false]) {
15-
while (start <= end) {
16-
const index = leading ? start : end
17-
const child = inline[index]
18-
if (child.type !== 'text' || child.marks?.some((mark) => mark.type === 'code')) break
19-
const text = (child.text ?? '').replace(leading ? /^[ \t\r\n]+/ : /[ \t\r\n]+$/, '')
20-
if (text) {
21-
inline[index] = { ...child, text }
22-
break
23-
}
24-
if (leading) start++
25-
else end--
26-
}
27-
}
28-
if (start <= end) blocks.push({ ...node, content: inline.slice(start, end + 1) })
29-
inline = []
8+
if (
9+
node.content.some(
10+
(child) =>
11+
child.type !== 'image' &&
12+
(child.type !== 'text' || child.text?.trim() || child.marks?.length)
13+
)
14+
) {
15+
return [
16+
{
17+
...node,
18+
content: node.content.map((child) =>
19+
child.type === 'image' ? { ...child, type: 'inlineImage' } : child
20+
),
21+
},
22+
]
3023
}
31-
for (const child of node.content) {
32-
if (child.type === 'image') {
33-
flush()
34-
blocks.push(child)
35-
} else {
36-
inline.push(child)
37-
}
38-
}
39-
flush()
40-
return blocks
24+
return node.content.filter((child) => child.type === 'image')
4125
}

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/chrome-scope.test.ts‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*/
1414
import { readFileSync } from 'node:fs'
1515
import path from 'node:path'
16-
import { beforeAll, describe, expect, it } from 'vitest'
16+
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
1717

1818
const EDITOR_CSS_PATH = path.join(__dirname, 'rich-markdown-editor.css')
1919

@@ -33,9 +33,10 @@ const CHROME_MARKERS = [
3333
] as const
3434

3535
let selectors: string[] = []
36+
let style: HTMLStyleElement
3637

3738
beforeAll(() => {
38-
const style = document.createElement('style')
39+
style = document.createElement('style')
3940
style.textContent = readFileSync(EDITOR_CSS_PATH, 'utf-8')
4041
document.head.appendChild(style)
4142
if (!style.sheet) throw new Error('rich-markdown-editor.css did not parse')
@@ -44,6 +45,8 @@ beforeAll(() => {
4445
.map((rule) => rule.selectorText)
4546
})
4647

48+
afterAll(() => style.remove())
49+
4750
describe('rich markdown chrome scoping', () => {
4851
it.each(CHROME_MARKERS)('scopes every %s rule to the shared node class', (marker) => {
4952
const matching = selectors.filter((selector) => selector.includes(marker))
@@ -53,4 +56,30 @@ describe('rich markdown chrome scoping', () => {
5356
expect(selector).not.toContain('.rich-markdown-prose')
5457
}
5558
})
59+
60+
it.each(['div', 'span'].flatMap((tag) => [false, true].map((linked) => ({ tag, linked }))))(
61+
'keeps the $tag image selection ring inside the image (linked: $linked)',
62+
({ tag, linked }) => {
63+
const root = document.createElement('div')
64+
root.className = 'rich-markdown-nodes'
65+
const wrapper = document.createElement(tag)
66+
wrapper.className = 'ProseMirror-selectednode'
67+
const image = document.createElement('img')
68+
if (linked) {
69+
const link = document.createElement('a')
70+
link.append(image)
71+
wrapper.append(link)
72+
} else wrapper.append(image)
73+
root.append(wrapper)
74+
document.body.append(root)
75+
try {
76+
expect(getComputedStyle(image).outlineOffset).toBe('-2px')
77+
expect(getComputedStyle(wrapper).outline).toBe('none')
78+
wrapper.classList.remove('ProseMirror-selectednode')
79+
expect(getComputedStyle(image).outlineOffset).toBe('')
80+
} finally {
81+
root.remove()
82+
}
83+
}
84+
)
5685
})

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-collaboration.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import Collaboration from '@tiptap/extension-collaboration'
2+
import { NodeSelection, Plugin, Selection } from '@tiptap/pm/state'
3+
4+
/**
5+
* Yjs can resolve a restored node selection into text after a structural edit.
6+
* Normalize that invalid selection before ProseMirror renders or scrolls it.
7+
* @see https://github.com/ueberdosis/y-tiptap/blob/main/src/plugins/sync-plugin.js
8+
*/
9+
export const FileCollaboration = Collaboration.extend({
10+
addProseMirrorPlugins() {
11+
return [
12+
...(this.parent?.() ?? []),
13+
new Plugin({
14+
appendTransaction: (_transactions, _oldState, state) => {
15+
const { selection } = state
16+
if (selection instanceof NodeSelection && !NodeSelection.isSelectable(selection.node)) {
17+
return state.tr.setSelection(Selection.near(selection.$from))
18+
}
19+
return null
20+
},
21+
}),
22+
]
23+
},
24+
})

0 commit comments

Comments
 (0)