Skip to content

Commit 0add889

Browse files
committed
fix(search): center virtualized matches after mount in find nav (SD-3315)
scrollToPositionAsync forwarded ifNeeded:true into its post-mount retry. A match that was off-screen at call time (hence the async/mount path) could end up edge-visible after the page scrolls into view, then downgrade to 'nearest' and skip the centering it should get. Force ifNeeded:false on the retry (keeping suppressSelectionSyncScroll); the fast path keeps ifNeeded for already-mounted matches. Flagged in review by Codex and cubic.
1 parent a7ccb7b commit 0add889

2 files changed

Lines changed: 54 additions & 3 deletions

File tree

packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3884,8 +3884,12 @@ export class PresentationEditor extends EventEmitter {
38843884
return false;
38853885
}
38863886

3887-
// Retry now that page is mounted
3888-
return this.scrollToPosition(pos, options);
3887+
// Retry now that page is mounted. Reaching this path means the target was on an unmounted
3888+
// (off-screen) page at call time, and #scrollPageIntoView above only scrolled the page into
3889+
// view — not the specific match, which can now sit at a viewport edge. Force ifNeeded:false so
3890+
// the match centers, instead of letting the now-edge-visible match downgrade to 'nearest' and
3891+
// skip centering (SD-3315 review). suppressSelectionSyncScroll is preserved via the spread.
3892+
return this.scrollToPosition(pos, { ...options, ifNeeded: false });
38893893
}
38903894

38913895
/**
@@ -3989,7 +3993,9 @@ export class PresentationEditor extends EventEmitter {
39893993
async focusContentControl(
39903994
entityId: string,
39913995
options: { block?: 'start' | 'center' | 'end' | 'nearest'; behavior?: ScrollBehavior } = {},
3992-
): Promise<{ success: true } | { success: false; reason: 'not-ready' | 'invalid-id' | 'not-found' | 'not-reachable' }> {
3996+
): Promise<
3997+
{ success: true } | { success: false; reason: 'not-ready' | 'invalid-id' | 'not-found' | 'not-reachable' }
3998+
> {
39933999
const editor = this.#editor;
39944000
if (!editor) return { success: false, reason: 'not-ready' };
39954001
if (typeof entityId !== 'string' || entityId.length === 0) return { success: false, reason: 'invalid-id' };

packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,51 @@ describe('PresentationEditor - scrollToPosition', () => {
649649
expect(result).toBe(true);
650650
});
651651

652+
it('centers a virtualized match after mount even with ifNeeded (SD-3315)', async () => {
653+
// Reaching the async/mount path means the target was off-screen at call time, so it should
654+
// center once mounted. The post-mount retry forces ifNeeded:false; without that, a match
655+
// that ends up edge-visible after the page scrolls into view would downgrade to 'nearest'
656+
// and skip centering. Pin innerHeight so the span counts as "visible" (where the old
657+
// forwarded ifNeeded:true would have downgraded).
658+
const originalInnerHeight = window.innerHeight;
659+
Object.defineProperty(window, 'innerHeight', { value: 800, configurable: true });
660+
try {
661+
editor = new PresentationEditor({ element: container, documentId: 'test-doc' });
662+
await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled());
663+
await new Promise((resolve) => setTimeout(resolve, 50));
664+
665+
const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement;
666+
const span = document.createElement('span');
667+
span.dataset.pmStart = '140';
668+
span.dataset.pmEnd = '160';
669+
span.scrollIntoView = vi.fn();
670+
span.getBoundingClientRect = vi.fn(
671+
() => ({ top: 100, bottom: 120, left: 0, right: 0, width: 0, height: 20 }) as DOMRect,
672+
);
673+
// Mount page index 1 (covers pos 150) after the async call starts.
674+
setTimeout(() => {
675+
const mockPage = document.createElement('div');
676+
mockPage.setAttribute('data-page-index', '1');
677+
mockPage.scrollIntoView = vi.fn();
678+
mockPage.appendChild(span);
679+
pagesHost.appendChild(mockPage);
680+
}, 100);
681+
682+
const result = await editor.scrollToPositionAsync(150, {
683+
block: 'center',
684+
ifNeeded: true,
685+
suppressSelectionSyncScroll: true,
686+
});
687+
688+
expect(result).toBe(true);
689+
// Post-mount retry must center the now-visible match, not downgrade to 'nearest'.
690+
expect(span.scrollIntoView).toHaveBeenCalledWith({ block: 'center', inline: 'nearest', behavior: 'auto' });
691+
expect(span.scrollIntoView).not.toHaveBeenCalledWith(expect.objectContaining({ block: 'nearest' }));
692+
} finally {
693+
Object.defineProperty(window, 'innerHeight', { value: originalInnerHeight, configurable: true });
694+
}
695+
});
696+
652697
it('should return false and warn when page fails to mount within timeout', async () => {
653698
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
654699

0 commit comments

Comments
 (0)