From 167256b40615d0f7e6ac670ffc66ae692324167a Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 17 Mar 2026 09:36:12 -0300 Subject: [PATCH 1/2] fix(presentation-editor): revalidate scroll container after first layout (SD-1950) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arrow key navigation was not auto-scrolling to keep the cursor visible when the consumer's container had overflow:auto but no height constraint. The root cause was in #findScrollableAncestor — at setup time it picks the first ancestor with overflow-y:auto|scroll, but cannot verify the element actually constrains content height (content isn't laid out yet). When the consumer sets overflow:auto without a height constraint, the container expands to fit all content instead of scrolling, making scrollHeight equal to clientHeight. This caused three cascading failures: 1. Caret scroll-into-view: getBoundingClientRect() on the ~15000px container always contained the caret, so the scroll condition never triggered. 2. Virtualization: domPainter received the wrong scroll container, computing page visibility against the full container instead of the viewport. 3. Scroll offset calculations: all scroll-relative logic used a container that never actually scrolled. The fix adds #revalidateScrollContainer(), called once after the first layout completes. At that point content is laid out and we can check scrollHeight vs clientHeight. If the detected container isn't actually scrollable, we walk further up the DOM to find one that is, or fall back to window — then update the scroll listener and domPainter reference. --- .../presentation-editor/PresentationEditor.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 67ccc8f61b..6c36e2a388 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -315,6 +315,7 @@ export class PresentationEditor extends EventEmitter { #editorListeners: Array<{ event: string; handler: (...args: unknown[]) => void }> = []; #scrollHandler: (() => void) | null = null; #scrollContainer: Element | Window | null = null; + #scrollContainerValidated = false; #sectionMetadata: SectionMetadata[] = []; #documentMode: 'editing' | 'viewing' | 'suggesting' = 'editing'; #inputBridge: PresentationInputBridge | null = null; @@ -3064,6 +3065,53 @@ export class PresentationEditor extends EventEmitter { return win; } + /** + * Re-validates the detected scroll container after the first layout completes. + * + * At setup time, #findScrollableAncestor picks the first ancestor with + * overflow-y: auto|scroll — but it can't verify the element actually constrains + * content height (content isn't laid out yet). A consumer may set overflow:auto + * on the SuperDoc container without constraining its height, causing the element + * to expand to fit all content instead of scrolling. + * + * After layout, we can check scrollHeight vs clientHeight. If the detected + * container isn't actually scrollable, we walk further up to find one that is, + * or fall back to window. + */ + #revalidateScrollContainer(): void { + if (this.#scrollContainerValidated) return; + this.#scrollContainerValidated = true; + + if (!(this.#scrollContainer instanceof Element)) return; + if (this.#scrollContainer.scrollHeight > this.#scrollContainer.clientHeight + 1) return; + + const win = this.#scrollContainer.ownerDocument?.defaultView; + let el: Element | null = this.#scrollContainer.parentElement; + let next: Element | Window | null = win ?? null; + + while (el) { + const { overflowY } = getComputedStyle(el); + if ((overflowY === 'auto' || overflowY === 'scroll') && el.scrollHeight > el.clientHeight + 1) { + next = el; + break; + } + el = el.parentElement; + } + + if (!next || next === this.#scrollContainer) return; + + const prev = this.#scrollContainer; + prev.removeEventListener('scroll', this.#scrollHandler!); + this.#scrollContainer = next; + + if (next instanceof Element) { + next.addEventListener('scroll', this.#scrollHandler!, { passive: true }); + } + if (this.#domPainter && next instanceof HTMLElement) { + this.#domPainter.setScrollContainer(next); + } + } + /** * Sets up drag and drop handlers for field annotations and image files. */ @@ -3798,6 +3846,7 @@ export class PresentationEditor extends EventEmitter { this.#epochMapper.onLayoutComplete(layoutEpoch); this.#selectionSync.onLayoutComplete(layoutEpoch); layoutCompleted = true; + this.#revalidateScrollContainer(); this.#updatePermissionOverlay(); // Reset error state on successful layout From 5095734961af20947296663fc48c5653a084e714 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 17 Mar 2026 19:51:59 -0300 Subject: [PATCH 2/2] fix(presentation-editor): address scroll container revalidation edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes based on review feedback: 1. Avoid falsely switching away from properly constrained containers. A container with height:600px and overflow:auto may not overflow on first layout if the document is short. The previous check would switch to window in that case and never retry. Now we also verify the container grew beyond the viewport before switching — a clear signal its height is unconstrained. 2. Clear domPainter scroll container when falling back to window. When revalidation resolves to window, the domPainter was keeping the stale HTMLElement reference from #ensurePainter. Now we pass null, which makes the domPainter use viewport-based calculations. --- .../presentation-editor/PresentationEditor.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 6c36e2a388..ee6bb91e49 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -3075,8 +3075,10 @@ export class PresentationEditor extends EventEmitter { * to expand to fit all content instead of scrolling. * * After layout, we can check scrollHeight vs clientHeight. If the detected - * container isn't actually scrollable, we walk further up to find one that is, - * or fall back to window. + * container isn't actually scrollable AND it grew beyond the viewport (ruling + * out properly constrained containers that simply don't have enough content + * yet), we walk further up to find one that actually scrolls, or fall back + * to window. */ #revalidateScrollContainer(): void { if (this.#scrollContainerValidated) return; @@ -3085,7 +3087,14 @@ export class PresentationEditor extends EventEmitter { if (!(this.#scrollContainer instanceof Element)) return; if (this.#scrollContainer.scrollHeight > this.#scrollContainer.clientHeight + 1) return; + // A properly constrained container (e.g. height:600px; overflow:auto) may + // not be overflowing yet if the document is short. Its clientHeight stays + // within viewport bounds. Only switch when the container grew beyond the + // viewport — a clear sign its height is unconstrained. const win = this.#scrollContainer.ownerDocument?.defaultView; + const viewportHeight = win?.innerHeight ?? 0; + if (this.#scrollContainer.clientHeight <= viewportHeight) return; + let el: Element | null = this.#scrollContainer.parentElement; let next: Element | Window | null = win ?? null; @@ -3107,9 +3116,7 @@ export class PresentationEditor extends EventEmitter { if (next instanceof Element) { next.addEventListener('scroll', this.#scrollHandler!, { passive: true }); } - if (this.#domPainter && next instanceof HTMLElement) { - this.#domPainter.setScrollContainer(next); - } + this.#domPainter?.setScrollContainer?.(next instanceof HTMLElement ? next : null); } /**