Fix Virtualize scroll container detection with horizontal overflow#65744
Fix Virtualize scroll container detection with horizontal overflow#65744ilonatommy wants to merge 2 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Blazor Virtualize’s scroll container detection so that elements whose overflow-y is only implicitly promoted (due to non-visible overflow-x) are no longer incorrectly selected as the vertical scroll root when they don’t actually scroll vertically.
Changes:
- Update
findClosestScrollContainer()to require actual vertical scrollability (or explicitoverflow-y: scroll) before selecting an element. - Add a new BasicTestApp scenario reproducing horizontal-overflow container behavior.
- Add an E2E test asserting
Virtualizecontinues to load items when the container only scrolls horizontally and the page is the vertical scroll root.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Components/Web.JS/src/Virtualize.ts | Tightens scroll-root selection to avoid false positives caused by CSS overflow promotion. |
| src/Components/test/testassets/BasicTestApp/VirtualizationHorizontalOverflow.razor | Adds a repro component exercising horizontal overflow with vertical overflow hidden. |
| src/Components/test/testassets/BasicTestApp/Index.razor | Exposes the new repro component in the BasicTestApp selector. |
| src/Components/test/E2ETest/Tests/VirtualizationTest.cs | Adds an E2E regression test for the horizontal-overflow container scenario. |
|
|
||
| if (style.overflowY !== 'visible') { | ||
| return element; | ||
| if (element.scrollHeight > element.clientHeight || style.overflowY === 'scroll') { |
There was a problem hiding this comment.
Is there an edge case that is not covered? If you have a container with overflow-x: auto, overflow-y: hidden, and a fixed height (e.g., 400px), the content inside is taller than the container, so scrollHeight > clientHeight is true. Even with the fix we would pick this container as the scroll root. But overflow-y: hidden means the user can't actually scroll it, so virtualization would still be broken in that scenario.
This isn't a regression, though. The old code also picked this element because its
computed overflowY was hidden, which isn't visible. A stronger fix could also exclude
hidden and clip values since those never allow user scrolling:
if (style.overflowY !== 'visible') {
const canScroll = style.overflowY !== 'hidden' && style.overflowY !== 'clip';
if (canScroll && (element.scrollHeight > element.clientHeight || style.overflowY === 'scroll')) {
return element;
}
}Feel free to not handle this case in this PR.
findClosestScrollContainer()incorrectly selected elements withoverflow-xset to a non-visible value as the vertical scroll root.Per the CSS spec, setting
overflow-xto a non-visible value promotesoverflow-yfrom 'visible' to 'auto', causing the heuristic to pick containers that don't actually scroll vertically.Description
The fix adds a check that the candidate element can actually scroll vertically (
scrollHeight > clientHeight) or has overflow-y explicitly set to 'scroll' before selecting it as the scroll container.Proposal #58200 (comment) works the issue around and can be used until this fix gets in.
Fixes #58200