Skip to content

Commit 1aea458

Browse files
committed
fix: transform points through text-preserving children replacements
When the sync machine receives a block whose children share no keys with the local ones (a remote mark toggle that split spans without preserving keys), it replaces the children array wholesale with one `set` op. `transformPoint`'s `set` case only handled `_key` renames and `text` clamps, so points inside the replaced children kept referencing dead keys and the selection clamped to the block start. The reported symptom is the local caret jumping, but every key-addressed position flowing through the op stream was equally stranded: `rangeRefs`, `pointRefs`, remote presence selections, range decorations. `transformPoint` now remaps such points through their text offset within the node: the offset into the old children (the op's `inverse`, which the `_key` branch already reads) locates the same textual position in the new children, via the shared walks in `util.child-text-offset.ts`. The mapping is gated on the old and new children carrying identical concatenated span text, the condition that makes it lossless; a remote mark toggle preserves text by definition. Points whose span key survives the replacement keep their identity untouched; raw block-offset points and points deeper than a direct child pass through untransformed; text-changing replacements keep the previous behavior, offset mapping would be guesswork there. Spans are detected structurally (`text` being a string) because `transformPoint` is deliberately schema-free; a container's array field may also be named `children` and hold blocks, which the text requirement turns into a no-op by construction. Fixing at the transform layer rather than around the sync machine's `set` call (the approach explored in the earlier draft) means all point consumers are covered by the same code and the correctness precondition is enforced where the mapping happens. The sync machine is untouched. The browser scenario (ported from the earlier draft, verified failing before the fix) pins the end-to-end behavior along with a guard scenario for text-changing replacements; unit tests pin the mechanics, mid-span remaps, boundary offsets, inline objects, identity over offset, container-depth remaps, container blocks in `children`-named fields, raw block-offset points, and the no-inverse and foreign-block no-ops.
1 parent 80c325b commit 1aea458

4 files changed

Lines changed: 580 additions & 0 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@portabletext/editor': patch
3+
---
4+
5+
fix: preserve selections through remote children replacements that keep the text
6+
7+
When a remote change replaces a block's children with re-keyed spans
8+
(a collaborator toggling a mark that splits spans), the local cursor
9+
and any other tracked selection no longer jump to the start of the
10+
block. As long as the block's text is unchanged, selections keep
11+
their exact textual position; replacements that change the text keep
12+
the previous behavior.

packages/editor/src/engine/point/transform-point.test.ts

Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,309 @@ describe(transformPoint.name, () => {
217217
offset: 0,
218218
})
219219
})
220+
221+
test('set children: remaps a point through a text-preserving replacement', () => {
222+
const point = {
223+
path: [{_key: 'b1'}, 'children', {_key: 's1'}],
224+
offset: 5,
225+
}
226+
const op: EngineOperation = {
227+
type: 'set',
228+
path: [{_key: 'b1'}, 'children'],
229+
value: [
230+
{_type: 'span', _key: 'n1', text: 'foo', marks: []},
231+
{_type: 'span', _key: 'n2', text: 'bar', marks: ['strong']},
232+
{_type: 'span', _key: 'n3', text: 'baz', marks: []},
233+
],
234+
inverse: {
235+
type: 'set',
236+
path: [{_key: 'b1'}, 'children'],
237+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
238+
},
239+
}
240+
expect(transformPoint(point, op)).toEqual({
241+
path: [{_key: 'b1'}, 'children', {_key: 'n2'}],
242+
offset: 2,
243+
})
244+
})
245+
246+
test('set children: a boundary offset lands at the end of the earlier span', () => {
247+
const point = {
248+
path: [{_key: 'b1'}, 'children', {_key: 's1'}],
249+
offset: 3,
250+
}
251+
const op: EngineOperation = {
252+
type: 'set',
253+
path: [{_key: 'b1'}, 'children'],
254+
value: [
255+
{_type: 'span', _key: 'n1', text: 'foo', marks: []},
256+
{_type: 'span', _key: 'n2', text: 'barbaz', marks: []},
257+
],
258+
inverse: {
259+
type: 'set',
260+
path: [{_key: 'b1'}, 'children'],
261+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
262+
},
263+
}
264+
expect(transformPoint(point, op)).toEqual({
265+
path: [{_key: 'b1'}, 'children', {_key: 'n1'}],
266+
offset: 3,
267+
})
268+
})
269+
270+
test('set children: inline objects occupy no text offset on either side', () => {
271+
const point = {
272+
path: [{_key: 'b1'}, 'children', {_key: 's2'}],
273+
offset: 1,
274+
}
275+
const op: EngineOperation = {
276+
type: 'set',
277+
path: [{_key: 'b1'}, 'children'],
278+
value: [
279+
{_type: 'span', _key: 'n1', text: 'foo', marks: []},
280+
{_type: 'stock-ticker', _key: 'n2'},
281+
{_type: 'span', _key: 'n3', text: 'bar', marks: []},
282+
],
283+
inverse: {
284+
type: 'set',
285+
path: [{_key: 'b1'}, 'children'],
286+
value: [
287+
{_type: 'span', _key: 's1', text: 'foo', marks: []},
288+
{_type: 'stock-ticker', _key: 'o1'},
289+
{_type: 'span', _key: 's2', text: 'bar', marks: []},
290+
],
291+
},
292+
}
293+
expect(transformPoint(point, op)).toEqual({
294+
path: [{_key: 'b1'}, 'children', {_key: 'n3'}],
295+
offset: 1,
296+
})
297+
})
298+
299+
test('set children: a surviving span keeps its identity over offset mapping', () => {
300+
const point = {
301+
path: [{_key: 'b1'}, 'children', {_key: 's1'}],
302+
offset: 2,
303+
}
304+
const op: EngineOperation = {
305+
type: 'set',
306+
path: [{_key: 'b1'}, 'children'],
307+
value: [
308+
{_type: 'span', _key: 'n1', text: 'bar', marks: []},
309+
{_type: 'span', _key: 's1', text: 'foo', marks: []},
310+
],
311+
inverse: {
312+
type: 'set',
313+
path: [{_key: 'b1'}, 'children'],
314+
value: [
315+
{_type: 'span', _key: 's1', text: 'foo', marks: []},
316+
{_type: 'span', _key: 'o2', text: 'bar', marks: []},
317+
],
318+
},
319+
}
320+
expect(transformPoint(point, op)).toBe(point)
321+
})
322+
323+
test('set children: a text-changing replacement leaves the point untransformed', () => {
324+
const point = {
325+
path: [{_key: 'b1'}, 'children', {_key: 's1'}],
326+
offset: 5,
327+
}
328+
const op: EngineOperation = {
329+
type: 'set',
330+
path: [{_key: 'b1'}, 'children'],
331+
value: [{_type: 'span', _key: 'n1', text: 'hello', marks: []}],
332+
inverse: {
333+
type: 'set',
334+
path: [{_key: 'b1'}, 'children'],
335+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
336+
},
337+
}
338+
expect(transformPoint(point, op)).toBe(point)
339+
})
340+
341+
test('set children: no inverse leaves the point untransformed', () => {
342+
const point = {
343+
path: [{_key: 'b1'}, 'children', {_key: 's1'}],
344+
offset: 5,
345+
}
346+
const op: EngineOperation = {
347+
type: 'set',
348+
path: [{_key: 'b1'}, 'children'],
349+
value: [{_type: 'span', _key: 'n1', text: 'foobarbaz', marks: []}],
350+
}
351+
expect(transformPoint(point, op)).toBe(point)
352+
})
353+
354+
test('set children: a point in another block is untouched', () => {
355+
const point = {
356+
path: [{_key: 'b2'}, 'children', {_key: 's9'}],
357+
offset: 5,
358+
}
359+
const op: EngineOperation = {
360+
type: 'set',
361+
path: [{_key: 'b1'}, 'children'],
362+
value: [{_type: 'span', _key: 'n1', text: 'foobarbaz', marks: []}],
363+
inverse: {
364+
type: 'set',
365+
path: [{_key: 'b1'}, 'children'],
366+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
367+
},
368+
}
369+
expect(transformPoint(point, op)).toBe(point)
370+
})
371+
372+
test('set children: container blocks in a field named `children` are not offset-mapped', () => {
373+
// A container's array field may be named `children` and hold
374+
// blocks. Blocks carry no `text`, so a point addressing a replaced
375+
// block keeps the untransformed-point behavior.
376+
const point = {
377+
path: [{_key: 'callout1'}, 'children', {_key: 'block1'}],
378+
offset: 0,
379+
}
380+
const op: EngineOperation = {
381+
type: 'set',
382+
path: [{_key: 'callout1'}, 'children'],
383+
value: [
384+
{
385+
_type: 'block',
386+
_key: 'newBlock1',
387+
children: [{_type: 'span', _key: 'n1', text: 'foo', marks: []}],
388+
},
389+
],
390+
inverse: {
391+
type: 'set',
392+
path: [{_key: 'callout1'}, 'children'],
393+
value: [
394+
{
395+
_type: 'block',
396+
_key: 'block1',
397+
children: [{_type: 'span', _key: 's1', text: 'foo', marks: []}],
398+
},
399+
],
400+
},
401+
}
402+
expect(transformPoint(point, op)).toBe(point)
403+
})
404+
405+
test('set children: a point deeper inside replaced container blocks is untouched', () => {
406+
const point = {
407+
path: [
408+
{_key: 'callout1'},
409+
'children',
410+
{_key: 'block1'},
411+
'children',
412+
{_key: 's1'},
413+
],
414+
offset: 2,
415+
}
416+
const op: EngineOperation = {
417+
type: 'set',
418+
path: [{_key: 'callout1'}, 'children'],
419+
value: [
420+
{
421+
_type: 'block',
422+
_key: 'newBlock1',
423+
children: [{_type: 'span', _key: 'n1', text: 'foo', marks: []}],
424+
},
425+
],
426+
inverse: {
427+
type: 'set',
428+
path: [{_key: 'callout1'}, 'children'],
429+
value: [
430+
{
431+
_type: 'block',
432+
_key: 'block1',
433+
children: [{_type: 'span', _key: 's1', text: 'foo', marks: []}],
434+
},
435+
],
436+
},
437+
}
438+
expect(transformPoint(point, op)).toBe(point)
439+
})
440+
441+
test('set children: remaps span points of a text block nested in a container', () => {
442+
const point = {
443+
path: [
444+
{_key: 'table1'},
445+
'rows',
446+
{_key: 'row1'},
447+
'cells',
448+
{_key: 'cell1'},
449+
'content',
450+
{_key: 'block1'},
451+
'children',
452+
{_key: 's1'},
453+
],
454+
offset: 5,
455+
}
456+
const op: EngineOperation = {
457+
type: 'set',
458+
path: [
459+
{_key: 'table1'},
460+
'rows',
461+
{_key: 'row1'},
462+
'cells',
463+
{_key: 'cell1'},
464+
'content',
465+
{_key: 'block1'},
466+
'children',
467+
],
468+
value: [
469+
{_type: 'span', _key: 'n1', text: 'foo', marks: []},
470+
{_type: 'span', _key: 'n2', text: 'bar', marks: ['strong']},
471+
{_type: 'span', _key: 'n3', text: 'baz', marks: []},
472+
],
473+
inverse: {
474+
type: 'set',
475+
path: [
476+
{_key: 'table1'},
477+
'rows',
478+
{_key: 'row1'},
479+
'cells',
480+
{_key: 'cell1'},
481+
'content',
482+
{_key: 'block1'},
483+
'children',
484+
],
485+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
486+
},
487+
}
488+
expect(transformPoint(point, op)).toEqual({
489+
path: [
490+
{_key: 'table1'},
491+
'rows',
492+
{_key: 'row1'},
493+
'cells',
494+
{_key: 'cell1'},
495+
'content',
496+
{_key: 'block1'},
497+
'children',
498+
{_key: 'n2'},
499+
],
500+
offset: 2,
501+
})
502+
})
503+
504+
test('set children: a raw block-offset point is never offset-mapped', () => {
505+
// Selections can carry raw block-offset points ({path: [block],
506+
// offset: N}) that resolve lazily. The children remap only applies
507+
// to points addressing a direct child of the replaced array, so
508+
// block-path points pass through untouched.
509+
const point = {
510+
path: [{_key: 'b1'}],
511+
offset: 5,
512+
}
513+
const op: EngineOperation = {
514+
type: 'set',
515+
path: [{_key: 'b1'}, 'children'],
516+
value: [{_type: 'span', _key: 'n1', text: 'foobarbaz', marks: []}],
517+
inverse: {
518+
type: 'set',
519+
path: [{_key: 'b1'}, 'children'],
520+
value: [{_type: 'span', _key: 's1', text: 'foobarbaz', marks: []}],
521+
},
522+
}
523+
expect(transformPoint(point, op)).toBe(point)
524+
})
220525
})

0 commit comments

Comments
 (0)