Skip to content

Commit 5e66227

Browse files
bjohasclaude
andcommitted
test(superdoc): cover scrollToComment delegation + scrollToHeading
Updates the two existing `scrollToComment` tests to reflect the new delegation-via-`scrollToElement` behaviour (no more bespoke `querySelector('[data-comment-ids*=...]')` lookup), and adds: - `scrollToElement` falls back to the body editor when the presentation path returns false (flow-layout case): uses `commands.setCursorById` to resolve the id and `getElementAtPos` for the DOM target. - `scrollToHeading` walks the doc, counts paragraphs whose `paragraphProperties.styleId = "HeadingN"`, picks the 1-based ordinal-th, and passes a text-inside-content position (not the doc-level boundary) to `scrollToPositionAsync`. - `scrollToHeading` rejects out-of-range levels (0, 7), non-positive ordinals, and non-integer arguments. All 84 `SuperDoc.test.js` cases pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e44c627 commit 5e66227

1 file changed

Lines changed: 141 additions & 13 deletions

File tree

packages/superdoc/src/core/SuperDoc.test.js

Lines changed: 141 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,15 @@ describe('SuperDoc core', () => {
276276
expect(instance.user).toEqual(expect.objectContaining({ name: 'Default SuperDoc user', email: null }));
277277
});
278278

279-
it('scrolls to a comment and sets it active', async () => {
280-
const { commentsStore } = createAppHarness();
279+
it('delegates scrollToComment to scrollToElement and marks the thread active', async () => {
280+
const { superdocStore, commentsStore } = createAppHarness();
281+
const scrollToElement = vi.fn(async () => true);
282+
superdocStore.documents = [
283+
{
284+
getPresentationEditor: vi.fn(() => ({ scrollToElement })),
285+
},
286+
];
287+
281288
const instance = new SuperDoc({
282289
selector: '#host',
283290
document: 'https://example.com/doc.docx',
@@ -289,19 +296,21 @@ describe('SuperDoc core', () => {
289296
});
290297
await flushMicrotasks();
291298

292-
const target = document.createElement('div');
293-
target.setAttribute('data-comment-ids', 'comment-1');
294-
target.scrollIntoView = vi.fn();
295-
document.querySelector('#host').appendChild(target);
296-
297-
const result = instance.scrollToComment('comment-1');
298-
expect(result).toBe(true);
299-
expect(target.scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth', block: 'start' });
299+
await expect(instance.scrollToComment('comment-1')).resolves.toBe(true);
300+
expect(scrollToElement).toHaveBeenCalledWith('comment-1');
300301
expect(commentsStore.setActiveComment).toHaveBeenCalledWith(instance, 'comment-1');
301302
});
302303

303-
it('returns false when comment element is not found', async () => {
304-
createAppHarness();
304+
it('scrollToComment resolves false when neither presentation nor body editor can locate the id', async () => {
305+
const { superdocStore } = createAppHarness();
306+
superdocStore.documents = [
307+
{
308+
// Presentation editor present but returns false (e.g. mark on an unmounted page,
309+
// or no such id in the doc).
310+
getPresentationEditor: vi.fn(() => ({ scrollToElement: vi.fn(async () => false) })),
311+
},
312+
];
313+
305314
const instance = new SuperDoc({
306315
selector: '#host',
307316
document: 'https://example.com/doc.docx',
@@ -311,7 +320,23 @@ describe('SuperDoc core', () => {
311320
});
312321
await flushMicrotasks();
313322

314-
expect(instance.scrollToComment('nonexistent-id')).toBe(false);
323+
// No activeEditor on the instance → body-editor fallback also returns false.
324+
await expect(instance.scrollToComment('nonexistent-id')).resolves.toBe(false);
325+
});
326+
327+
it('scrollToComment returns false when comments module is disabled', async () => {
328+
createAppHarness();
329+
const instance = new SuperDoc({
330+
selector: '#host',
331+
document: 'https://example.com/doc.docx',
332+
documents: [],
333+
// no `modules.comments`
334+
modules: { toolbar: {} },
335+
onException: vi.fn(),
336+
});
337+
await flushMicrotasks();
338+
339+
await expect(instance.scrollToComment('any-id')).resolves.toBe(false);
315340
});
316341

317342
it('forwards navigateTo to the first presentation editor', async () => {
@@ -407,6 +432,109 @@ describe('SuperDoc core', () => {
407432
await expect(instance.scrollToElement('element-1')).resolves.toBe(false);
408433
});
409434

435+
it('scrollToElement falls back to body editor when presentation returns false', async () => {
436+
const { superdocStore } = createAppHarness();
437+
superdocStore.documents = [
438+
{
439+
// Presentation editor is present but cannot scroll (e.g. flow layout).
440+
getPresentationEditor: vi.fn(() => ({ scrollToElement: vi.fn(async () => false) })),
441+
},
442+
];
443+
444+
const scrollIntoView = vi.fn();
445+
const targetEl = { scrollIntoView };
446+
const setCursorById = vi.fn(() => true);
447+
448+
const instance = new SuperDoc({
449+
selector: '#host',
450+
document: 'https://example.com/doc.docx',
451+
documents: [],
452+
modules: { comments: {}, toolbar: {} },
453+
onException: vi.fn(),
454+
});
455+
await flushMicrotasks();
456+
457+
// Inject a minimal activeEditor stub. The body-editor fallback uses
458+
// `setCursorById` to resolve the id and `getElementAtPos` to find the DOM.
459+
Object.defineProperty(instance, 'activeEditor', {
460+
configurable: true,
461+
get: () => ({
462+
state: {
463+
doc: { content: { size: 100 }, descendants: vi.fn() },
464+
selection: { from: 42 },
465+
},
466+
commands: { setCursorById },
467+
getElementAtPos: vi.fn(() => targetEl),
468+
}),
469+
});
470+
471+
await expect(instance.scrollToElement('comment-1')).resolves.toBe(true);
472+
expect(setCursorById).toHaveBeenCalledWith('comment-1', { preferredActiveThreadId: 'comment-1' });
473+
expect(scrollIntoView).toHaveBeenCalledWith(
474+
expect.objectContaining({ block: expect.any(String), inline: 'nearest' }),
475+
);
476+
});
477+
478+
it('scrollToHeading walks for the Nth heading at the given level and scrolls', async () => {
479+
const { superdocStore } = createAppHarness();
480+
// Mock doc with three Heading1 paragraphs at known positions.
481+
const headings = [
482+
{ pos: 10, text: 'first', styleId: 'Heading1' },
483+
{ pos: 50, text: 'second', styleId: 'Heading1' },
484+
{ pos: 200, text: 'third', styleId: 'Heading1' },
485+
];
486+
const makeNode = (h) => ({
487+
type: { name: 'paragraph' },
488+
attrs: { paragraphProperties: { styleId: h.styleId } },
489+
content: { size: 5 },
490+
descendants: (cb) => cb({ isText: true, text: h.text }, 0),
491+
});
492+
const descendants = (cb) => {
493+
for (const h of headings) {
494+
if (cb(makeNode(h), h.pos) === false) return;
495+
}
496+
};
497+
498+
const scrollToPositionAsync = vi.fn(async () => true);
499+
superdocStore.documents = [{ getPresentationEditor: vi.fn(() => ({ scrollToPositionAsync })) }];
500+
501+
const instance = new SuperDoc({
502+
selector: '#host',
503+
document: 'https://example.com/doc.docx',
504+
documents: [],
505+
modules: { comments: {}, toolbar: {} },
506+
onException: vi.fn(),
507+
});
508+
await flushMicrotasks();
509+
510+
Object.defineProperty(instance, 'activeEditor', {
511+
configurable: true,
512+
get: () => ({ state: { doc: { descendants, content: { size: 1000 } } } }),
513+
});
514+
515+
await expect(instance.scrollToHeading(1, 2)).resolves.toBe(true);
516+
// The 2nd Heading1 starts at pos=50; the text-inside-content fix should
517+
// shift the target one position into the paragraph's content.
518+
expect(scrollToPositionAsync).toHaveBeenCalledWith(51, expect.any(Object));
519+
});
520+
521+
it('scrollToHeading rejects out-of-range levels and non-positive ordinals', async () => {
522+
createAppHarness();
523+
const instance = new SuperDoc({
524+
selector: '#host',
525+
document: 'https://example.com/doc.docx',
526+
documents: [],
527+
modules: { comments: {}, toolbar: {} },
528+
onException: vi.fn(),
529+
});
530+
await flushMicrotasks();
531+
532+
await expect(instance.scrollToHeading(0, 1)).resolves.toBe(false);
533+
await expect(instance.scrollToHeading(7, 1)).resolves.toBe(false);
534+
await expect(instance.scrollToHeading(1, 0)).resolves.toBe(false);
535+
await expect(instance.scrollToHeading(1.5, 1)).resolves.toBe(false);
536+
});
537+
410538
it('warns when both document object and documents list provided', async () => {
411539
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
412540
createAppHarness();

0 commit comments

Comments
 (0)