Skip to content

Commit 8a2b0d5

Browse files
committed
fix(ui): report 'zoom' not 'mixed' for a zoom repaint (SD-3311)
layoutUpdated and paginationUpdate are emitted back-to-back with the same payload for the same paint (PresentationEditor.ts:6491-6492), so subscribing geometry invalidation to both double-counted one repaint: a zoom coalesced to 'mixed' and the 'zoom' reason was unreachable. Subscribe to layoutUpdated only - it covers every repaint. Adds regression coverage for the zoom and plain layout reasons.
1 parent 20f6849 commit 8a2b0d5

2 files changed

Lines changed: 89 additions & 4 deletions

File tree

packages/super-editor/src/ui/create-super-doc-ui.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,15 +1169,18 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI {
11691169
PRESENTATION_EVENTS.forEach((name) => {
11701170
next.on?.(name, onPresentationChange);
11711171
});
1172-
// Geometry-only: layout/pagination repaints move painted rects without a
1173-
// body `transaction`. Drive the viewport geometry signal, NOT the slice
1172+
// Geometry-only: layout repaints move painted rects without a body
1173+
// `transaction`. Drive the viewport geometry signal, NOT the slice
11741174
// recompute (which would re-attach editor listeners on every repaint).
1175+
// Listen to `layoutUpdated` only: `paginationUpdate` is emitted
1176+
// back-to-back with the same payload for the same paint
1177+
// (PresentationEditor.ts:6491-6492), so subscribing to both would
1178+
// double-count one repaint — a zoom would coalesce to 'mixed' instead of
1179+
// 'zoom'. `layoutUpdated` alone covers every repaint.
11751180
next.on?.('layoutUpdated', onGeometryLayout);
1176-
next.on?.('paginationUpdate', onGeometryLayout);
11771181
currentPresentationTeardown = () => {
11781182
PRESENTATION_EVENTS.forEach((name) => next.off?.(name, onPresentationChange));
11791183
next.off?.('layoutUpdated', onGeometryLayout);
1180-
next.off?.('paginationUpdate', onGeometryLayout);
11811184
};
11821185
};
11831186

packages/super-editor/src/ui/viewport.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,3 +607,85 @@ describe('ui.viewport.observe — geometry invalidation (SD-3311)', () => {
607607
expect(events).toEqual([]);
608608
});
609609
});
610+
611+
// Stub with real event emitters so tests can drive the engine signals that
612+
// feed ui.viewport.observe: superdoc `zoomChange` and presentation
613+
// `layoutUpdated` / `paginationUpdate`.
614+
function makeEmitter() {
615+
const map = new Map<string, Set<(p?: unknown) => void>>();
616+
return {
617+
on: (e: string, h: (p?: unknown) => void) => {
618+
if (!map.has(e)) map.set(e, new Set());
619+
map.get(e)!.add(h);
620+
},
621+
off: (e: string, h: (p?: unknown) => void) => {
622+
map.get(e)?.delete(h);
623+
},
624+
emit: (e: string, p?: unknown) => [...(map.get(e) ?? [])].forEach((h) => h(p)),
625+
};
626+
}
627+
628+
function makeGeometryStub() {
629+
const sd = makeEmitter();
630+
const pres = makeEmitter();
631+
const emptyList = () => ({ evaluatedRevision: 'r1', total: 0, items: [], page: { limit: 0, offset: 0, returned: 0 } });
632+
const editor: { on: ReturnType<typeof vi.fn>; off: ReturnType<typeof vi.fn>; doc: unknown; presentationEditor: unknown } = {
633+
on: vi.fn(),
634+
off: vi.fn(),
635+
doc: {
636+
selection: { current: vi.fn(() => ({ empty: true })) },
637+
comments: { list: vi.fn(emptyList) },
638+
trackChanges: { list: vi.fn(emptyList) },
639+
contentControls: { list: vi.fn(() => ({ items: [], total: 0 })) },
640+
},
641+
presentationEditor: undefined,
642+
};
643+
editor.presentationEditor = {
644+
on: pres.on,
645+
off: pres.off,
646+
getActiveEditor: () => editor,
647+
getEntityRects: vi.fn(() => []),
648+
navigateTo: vi.fn(async () => true),
649+
};
650+
const superdoc: SuperDocLike = {
651+
activeEditor: editor as never,
652+
config: { documentMode: 'editing' },
653+
on: sd.on as never,
654+
off: sd.off as never,
655+
};
656+
return { superdoc, emitSuperdoc: sd.emit, emitPresentation: pres.emit };
657+
}
658+
659+
describe('ui.viewport.observe — repaint reason (SD-3311 regression)', () => {
660+
const nextFrame = () => new Promise((resolve) => setTimeout(resolve, 30));
661+
662+
it('reports "zoom" for a zoom repaint, not "mixed" (layoutUpdated + paginationUpdate are one paint)', async () => {
663+
const { superdoc, emitSuperdoc, emitPresentation } = makeGeometryStub();
664+
const ui = createSuperDocUI({ superdoc });
665+
const events: Array<{ reason: string }> = [];
666+
ui.viewport.observe((e) => events.push(e));
667+
668+
// zoomChange (pre-paint), then the paired post-paint repaint events.
669+
emitSuperdoc('zoomChange');
670+
emitPresentation('layoutUpdated');
671+
emitPresentation('paginationUpdate'); // same paint / payload — must not double-count
672+
await nextFrame();
673+
674+
expect(events).toEqual([{ reason: 'zoom' }]);
675+
ui.destroy();
676+
});
677+
678+
it('reports "layout" for a plain repaint (the paginationUpdate alias does not make it "mixed")', async () => {
679+
const { superdoc, emitPresentation } = makeGeometryStub();
680+
const ui = createSuperDocUI({ superdoc });
681+
const events: Array<{ reason: string }> = [];
682+
ui.viewport.observe((e) => events.push(e));
683+
684+
emitPresentation('layoutUpdated');
685+
emitPresentation('paginationUpdate');
686+
await nextFrame();
687+
688+
expect(events).toEqual([{ reason: 'layout' }]);
689+
ui.destroy();
690+
});
691+
});

0 commit comments

Comments
 (0)