Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions packages/react-aria-components/stories/ListBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1146,3 +1146,38 @@ export const DropOntoRoot = () => (
<DroppableListBox />
</div>
);

export const FractionalWidth: StoryFn = () => {
let items = Array.from({length: 50}, (_, i) => ({id: i, name: `Item ${i + 1}`}));
return (
<div
style={{
display: 'flex',
width: 501,
border: '1px solid gray'
}}>
<div style={{width: '50%'}}>
<Virtualizer layout={ListLayout} layoutOptions={{rowHeight: 32}}>
<ListBox
aria-label="Fractional width list 1"
className={styles.menu}
style={{height: 300, width: '100%'}}
items={items}>
{item => <MyListBoxItem>{item.name}</MyListBoxItem>}
</ListBox>
</Virtualizer>
</div>
<div style={{width: '50%'}}>
<Virtualizer layout={ListLayout} layoutOptions={{rowHeight: 32}}>
<ListBox
aria-label="Fractional width list 2"
className={styles.menu}
style={{height: 300, width: '100%'}}
items={items}>
{item => <MyListBoxItem>{item.name}</MyListBoxItem>}
</ListBox>
</Virtualizer>
</div>
</div>
);
};
30 changes: 25 additions & 5 deletions packages/react-aria/src/virtualizer/ScrollView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,22 @@ interface ScrollViewAria {
contentProps: HTMLAttributes<HTMLElement>;
}

function getClientSize(dom: HTMLElement) {
let clientWidth = dom.clientWidth;
let clientHeight = dom.clientHeight;
let doc = dom.ownerDocument;
if (!doc || dom === doc.documentElement || dom === doc.body || dom === doc.scrollingElement) {
return {clientWidth, clientHeight};
}

let rect = dom.getBoundingClientRect?.();
if (rect && rect.width > 0 && rect.height > 0) {
clientWidth = rect.width - Math.max(0, dom.offsetWidth - dom.clientWidth);
clientHeight = rect.height - Math.max(0, dom.offsetHeight - dom.clientHeight);
}
return {clientWidth, clientHeight};
Comment on lines +78 to +83

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct, because getBoundingClientRect() returns the border-box of an element, which is fine for normal elements but can easily break window scrolling, because neither border-box nor offset sizes work for the viewport. We should be using something similar to Modal.tsx instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nwidynski, thanks to pointing this out.
I traced the viewport case and confirmed the previous getClientSize() logic could use the document content height instead of the actual viewport height for root scrolling elements.

I updated it to keep the native clientWidth/clientHeight for document.documentElement, body, and document.scrollingElement, while keeping the fractional measurement for normal elements unchanged. I also added a regression test covering the document root case.

@nwidynski nwidynski Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm not sure of how pedantic we want to be about root elements but there is a lot more complexity that goes into which element is responsible for viewport overflow. For example, the body element can be an independent scroll container, so bailing out unconditionally may not be right. I will let @snowystinger weigh in there.

I also noticed that this is potentially introducing a breaking change to test environments, since the NODE_ENV flag is now required. This may be okay, but clientWidth and clientHeight are documented public API for this use case as far as i know, so I wanted to bring it up.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. I hadn't considered that body can also act as an independent scroll container, so I agree the root element handling needs to be more careful. The NODE_ENV change is also worth looking at since it changes which measurement path tests use. I'll look into both before making another change.

@lixiaoyan lixiaoyan Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC RAC's Virtualizer doesn't support window scrolling, so this may not be a big issue. The biggest risk in mixing getBoundingClientRect with clientWidth/offsetWidth is CSS transforms, though I'm not sure whether RAC cares about that case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lixiaoyan lixiaoyan Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha I missed it. Anyhow the CSS transform is still a real issue.

}

export function useScrollView(
props: ScrollViewProps,
ref: RefObject<HTMLElement | null>
Expand Down Expand Up @@ -253,15 +269,15 @@ export function useScrollView(
// content size update, causing below layout effect to fire. This avoids infinite loops.
isUpdatingSize.current = true;

let isTestEnv = process.env.NODE_ENV === 'test' && !process.env.VIRT_ON;
let isTest = process.env.NODE_ENV === 'test';
let isTestEnv = isTest && !process.env.VIRT_ON;
let isClientWidthMocked = Object.getOwnPropertyNames(window.HTMLElement.prototype).includes(
'clientWidth'
);
let isClientHeightMocked = Object.getOwnPropertyNames(window.HTMLElement.prototype).includes(
'clientHeight'
);
let clientWidth = dom.clientWidth;
let clientHeight = dom.clientHeight;
let {clientWidth, clientHeight} = isTest ? dom : getClientSize(dom);
let w = isTestEnv && !isClientWidthMocked ? Infinity : clientWidth;
let h = isTestEnv && !isClientHeightMocked ? Infinity : clientHeight;

Expand All @@ -286,8 +302,12 @@ export function useScrollView(
// adjusted space. In very specific cases this might result in the scrollbars disappearing
// again, resulting in extra padding. We stop after a maximum of two layout passes to avoid
// an infinite loop. This matches how browsers behavior with native CSS grid layout.
if ((!isTestEnv && clientWidth !== dom.clientWidth) || clientHeight !== dom.clientHeight) {
state.size = new Size(dom.clientWidth, dom.clientHeight);
let nextSize = isTest ? dom : getClientSize(dom);
if (
(!isTest && clientWidth !== nextSize.clientWidth) ||
clientHeight !== nextSize.clientHeight
) {
state.size = new Size(nextSize.clientWidth, nextSize.clientHeight);
flush(() => {
updateVisibleRect();
onSizeChange?.(state.size);
Expand Down
87 changes: 87 additions & 0 deletions packages/react-aria/test/virtualizer/ScrollView.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright 2026 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import {act, render} from '@react-spectrum/test-utils-internal';
import React, {useRef} from 'react';
import {Size} from 'react-stately/useVirtualizerState';
import {useScrollView} from '../../src/virtualizer/ScrollView';

function RootScrollView(
props: Partial<Parameters<typeof useScrollView>[0]> & {target: HTMLElement}
) {
let {target, ...otherProps} = props;
let ref = useRef(target);
let {contentProps} = useScrollView(
{
contentSize: new Size(1200, 2000),
onVisibleRectChange: jest.fn(),
allowsWindowScrolling: true,
...otherProps
},
ref
);
return <div {...contentProps} />;
}

describe('ScrollView', () => {
beforeAll(() => {
jest.useFakeTimers();
});

afterEach(() => {
act(() => {
jest.runAllTimers();
});
});

it('preserves viewport client dimensions when attached to documentElement', () => {
let origNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
Object.defineProperty(document.documentElement, 'clientWidth', {
configurable: true,
value: 1200
});
Object.defineProperty(document.documentElement, 'clientHeight', {
configurable: true,
value: 800
});
Object.defineProperty(document.documentElement, 'offsetHeight', {
configurable: true,
value: 50
});
let rectSpy = jest.spyOn(document.documentElement, 'getBoundingClientRect').mockReturnValue({
width: 1200,
height: 50,
top: 0,
left: 0,
bottom: 50,
right: 1200,
x: 0,
y: 0,
toJSON: () => {}
});

let onSizeChange = jest.fn();
render(<RootScrollView target={document.documentElement} onSizeChange={onSizeChange} />);

expect(onSizeChange).toHaveBeenCalledWith(new Size(1200, 800));

delete (document.documentElement as any).clientWidth;
delete (document.documentElement as any).clientHeight;
delete (document.documentElement as any).offsetHeight;
rectSpy.mockRestore();
} finally {
process.env.NODE_ENV = origNodeEnv;
}
});
});