fix(painter-dom): skip non-scrollable scroll container in virtualization (SD-2199)#2383
Merged
harbournick merged 1 commit intomainfrom Mar 12, 2026
Merged
Conversation
…ion (SD-2199) Only first 7 pages rendered on documents with 8+ pages when SuperDoc was mounted inside an unconstrained flex layout. The scroll container detection found an ancestor with overflow:auto CSS but that element was never actually scrollable because its parent only had min-height (no height constraint). The element grew to fit content instead of constraining it, so scrollTop stayed at 0 and the virtual window never advanced beyond pages 0-6. The fix adds a runtime check in updateVirtualWindow() that verifies the scroll container is actually scrollable (scrollHeight > clientHeight) before using its scrollTop. When the container is not scrollable, it falls through to the viewport-based getBoundingClientRect calculation which works correctly with window-level scrolling.
Contributor
|
🎉 This PR is included in superdoc v1.18.0-next.55 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-cli v0.2.0-next.130 The release is available on GitHub release |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Demo
CleanShot.2026-03-12.at.19.38.35.mp4
Summary
Fixes SD-2199: only the first 7 pages rendered on documents with 8+ pages when SuperDoc was embedded in certain layouts (version tester, manual testing site).
Root Cause
In v1.18.0 we added scroll container detection (
#findScrollableAncestorin PresentationEditor,setScrollContaineron DomPainter) to fix virtualization offsets when SuperDoc is mounted inside a scrollable wrapper div.The detection walks up the DOM tree and picks the first ancestor with
overflow: autooroverflow: scrollCSS.updateVirtualWindow()then reads that element'sscrollTopto compute which pages to render.The problem: having
overflow: autoin CSS does not mean the element actually scrolls. In flex layouts where the parent only hasmin-height(notheight), the child grows to fit its content instead of constraining it. The element never scrolls,scrollTopstays at 0, and the virtual window is permanently stuck at pages 0-6 (window=5, overscan=1).This is the exact layout in both affected environments:
The local dev app works because
.dev-app__layouthasheight: 100vh(fixed), so.dev-app__mainis properly height-constrained and actually scrolls.The Fix
Added a runtime check in
updateVirtualWindow()that verifies the scroll container is actually scrollable (scrollHeight > clientHeight) before using itsscrollTop. When the container hasoverflow: autobut isn't actually overflowing, the code falls through to the viewport-basedgetBoundingClientRectcalculation, which works correctly with window-level scrolling.Why this approach
Alternative considered: fix
#findScrollableAncestorto check if the element is height-constrained. This is fragile because at setup time content may not be laid out yet, and determining whether a flex child will be constrained requires walking the entire ancestor chain analyzing flex/grid properties, explicit heights, and viewport units. The detection would need to be repeated on every layout change.Why the runtime check is better:
scrollHeight > clientHeightis the ground truth. It directly answers "is this element scrollable right now?" on every scroll event, with zero overhead (two property reads). It self-heals if the layout changes dynamically (e.g., window resize causes the container to become constrained). No assumptions about CSS layout models needed.Test Plan