Skip to content

Commit f87438b

Browse files
committed
update ScrollView to fix ComboBox tests
the issue was that updateSize was being called before the listboxref was attached due to it being immediatelly called in the test. The change makes it so the updateSize is called in the next render instead
1 parent 5258d8b commit f87438b

File tree

5 files changed

+163
-3
lines changed

5 files changed

+163
-3
lines changed

packages/@react-aria/virtualizer/src/ScrollView.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
9898
// Prevent rubber band scrolling from shaking when scrolling out of bounds
9999
state.scrollTop = Math.max(0, Math.min(scrollTop, contentSize.height - state.height));
100100
state.scrollLeft = Math.max(0, Math.min(scrollLeft, contentSize.width - state.width));
101-
102101
onVisibleRectChange(new Rect(state.scrollLeft, state.scrollTop, state.width, state.height));
103102

104103
if (!state.isScrolling) {
@@ -199,6 +198,8 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
199198

200199
// Update visible rect when the content size changes, in case scrollbars need to appear or disappear.
201200
let lastContentSize = useRef<Size | null>(null);
201+
let [, setUpdate] = useState({});
202+
let queuedUpdateSize = useRef(false);
202203
useLayoutEffect(() => {
203204
if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) {
204205
// React doesn't allow flushSync inside effects, so queue a microtask.
@@ -209,12 +210,22 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
209210
// https://github.com/reactwg/react-18/discussions/102
210211
// @ts-ignore
211212
if (typeof IS_REACT_ACT_ENVIRONMENT === 'boolean' ? IS_REACT_ACT_ENVIRONMENT : typeof jest !== 'undefined') {
212-
updateSize(fn => fn());
213+
// Queue call of updateSize to happen in a separate render but within the same act so that RAC virtualized ComboBoxes and Selects
214+
// work properly
215+
queuedUpdateSize.current = true;
216+
setUpdate({});
217+
lastContentSize.current = contentSize;
218+
return;
213219
} else {
214220
queueMicrotask(() => updateSize(flushSync));
215221
}
216222
}
217223

224+
if (queuedUpdateSize.current) {
225+
queuedUpdateSize.current = false;
226+
updateSize(fn => fn());
227+
}
228+
218229
lastContentSize.current = contentSize;
219230
});
220231

packages/@react-spectrum/picker/test/Picker.test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,9 @@ describe('Picker', function () {
10251025
expect(document.activeElement).toBe(picker);
10261026
expect(picker).toHaveTextContent('Empty');
10271027

1028+
// TODO: this test (along with others in this suite) fails because we seem to be detecting that the button is being focused after the
1029+
// dropdown is opened, resulting in the dropdown closing due to useOverlay interactOutside logic
1030+
// Seems to specifically happen if the Picker has a selected item and the user tries to open the Picker
10281031
await selectTester.selectOption({option: 'Zero'});
10291032
expect(onSelectionChange).toHaveBeenCalledTimes(2);
10301033
expect(onSelectionChange).toHaveBeenLastCalledWith('0');

packages/@react-spectrum/s2/chromatic/Combobox.stories.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,5 +142,9 @@ export const Filtering = {
142142
await waitFor(() => {
143143
expect(within(body).getByRole('progressbar', {hidden: true})).toBeInTheDocument();
144144
}, {timeout: 5000});
145+
146+
await waitFor(() => {
147+
expect(within(listbox).queryByRole('progressbar', {hidden: true})).toBeFalsy();
148+
}, {timeout: 5000});
145149
}
146150
};
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright 2025 Adobe. All rights reserved.
3+
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License. You may obtain a copy
5+
* of the License at http://www.apache.org/licenses/LICENSE-2.0
6+
*
7+
* Unless required by applicable law or agreed to in writing, software distributed under
8+
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
9+
* OF ANY KIND, either express or implied. See the License for the specific language
10+
* governing permissions and limitations under the License.
11+
*/
12+
13+
import {act, render, setupIntersectionObserverMock, within} from '@react-spectrum/test-utils-internal';
14+
import {ComboBox, ComboBoxItem} from '../src';
15+
import React from 'react';
16+
import {User} from '@react-aria/test-utils';
17+
18+
describe('Combobox', () => {
19+
let testUtilUser = new User();
20+
21+
beforeAll(function () {
22+
jest.useFakeTimers();
23+
jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
24+
jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
25+
});
26+
27+
afterEach(() => {
28+
jest.clearAllMocks();
29+
act(() => jest.runAllTimers());
30+
});
31+
32+
afterAll(function () {
33+
jest.restoreAllMocks();
34+
});
35+
36+
it('should render the sentinel when the combobox is empty', async () => {
37+
let tree = render(
38+
<ComboBox label="test">
39+
{[]}
40+
</ComboBox>
41+
);
42+
43+
let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container});
44+
expect(comboboxTester.listbox).toBeFalsy();
45+
comboboxTester.setInteractionType('mouse');
46+
await comboboxTester.open();
47+
48+
expect(comboboxTester.options()).toHaveLength(1);
49+
expect(within(comboboxTester.listbox!).getByTestId('loadMoreSentinel')).toBeInTheDocument();
50+
});
51+
52+
// TODO: will need to update these tests when I replace isLoading with colection in useLoadMoreSentinel
53+
it('should only call loadMore if loading is false', async () => {
54+
let onLoadMore = jest.fn();
55+
let observe = jest.fn();
56+
let observer = setupIntersectionObserverMock({
57+
observe
58+
});
59+
60+
let tree = render(
61+
<ComboBox label="test" loadingState="loadingMore" onLoadMore={onLoadMore}>
62+
<ComboBoxItem>Chocolate</ComboBoxItem>
63+
<ComboBoxItem>Mint</ComboBoxItem>
64+
<ComboBoxItem>Strawberry</ComboBoxItem>
65+
<ComboBoxItem>Vanilla</ComboBoxItem>
66+
<ComboBoxItem>Chocolate Chip Cookie Dough</ComboBoxItem>
67+
</ComboBox>
68+
);
69+
70+
let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container});
71+
expect(comboboxTester.listbox).toBeFalsy();
72+
comboboxTester.setInteractionType('mouse');
73+
await comboboxTester.open();
74+
75+
expect(onLoadMore).toHaveBeenCalledTimes(0);
76+
let sentinel = tree.getByTestId('loadMoreSentinel');
77+
expect(observe).toHaveBeenLastCalledWith(sentinel);
78+
79+
80+
act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
81+
act(() => {jest.runAllTimers();});
82+
83+
tree.rerender(
84+
<ComboBox label="test" loadingState="idle" onLoadMore={onLoadMore}>
85+
<ComboBoxItem>Chocolate</ComboBoxItem>
86+
<ComboBoxItem>Mint</ComboBoxItem>
87+
<ComboBoxItem>Strawberry</ComboBoxItem>
88+
<ComboBoxItem>Vanilla</ComboBoxItem>
89+
<ComboBoxItem>Chocolate Chip Cookie Dough</ComboBoxItem>
90+
</ComboBox>
91+
);
92+
93+
act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
94+
act(() => {jest.runAllTimers();});
95+
expect(onLoadMore).toHaveBeenCalledTimes(1);
96+
});
97+
});

packages/react-aria-components/test/ComboBox.test.js

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*/
1212

1313
import {act} from '@testing-library/react';
14-
import {Button, ComboBox, ComboBoxContext, FieldError, Header, Input, Label, ListBox, ListBoxItem, ListBoxSection, Popover, Text} from '../';
14+
import {Button, ComboBox, ComboBoxContext, FieldError, Header, Input, Label, ListBox, ListBoxItem, ListBoxSection, ListLayout, Popover, Text, Virtualizer} from '../';
1515
import {fireEvent, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
1616
import React from 'react';
1717
import {User} from '@react-aria/test-utils';
@@ -38,8 +38,15 @@ describe('ComboBox', () => {
3838
let user;
3939
let testUtilUser = new User();
4040
beforeAll(() => {
41+
jest.useFakeTimers();
4142
user = userEvent.setup({delay: null, pointerMap});
4243
});
44+
45+
afterEach(() => {
46+
jest.clearAllMocks();
47+
act(() => jest.runAllTimers());
48+
});
49+
4350
it('provides slots', async () => {
4451
let {getByRole} = render(<TestComboBox />);
4552

@@ -295,4 +302,42 @@ describe('ComboBox', () => {
295302

296303
expect(queryByRole('listbox')).not.toBeInTheDocument();
297304
});
305+
306+
it('should support virtualizer', async () => {
307+
let items = [];
308+
for (let i = 0; i < 50; i++) {
309+
items.push({id: i, name: 'Item ' + i});
310+
}
311+
312+
jest.restoreAllMocks(); // don't mock scrollTop for this test
313+
jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
314+
jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
315+
316+
let tree = render(
317+
<ComboBox >
318+
<Label style={{display: 'block'}}>Test</Label>
319+
<div style={{display: 'flex'}}>
320+
<Input />
321+
<Button>
322+
<span aria-hidden="true" style={{padding: '0 2px'}}></span>
323+
</Button>
324+
</div>
325+
<Popover>
326+
<Virtualizer layout={ListLayout} layoutOptions={{rowHeight: 25}}>
327+
<ListBox items={items}>
328+
{(item) => <ListBoxItem>{item.name}</ListBoxItem>}
329+
</ListBox>
330+
</Virtualizer>
331+
</Popover>
332+
</ComboBox>
333+
);
334+
335+
336+
let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container});
337+
expect(comboboxTester.listbox).toBeFalsy();
338+
comboboxTester.setInteractionType('mouse');
339+
await comboboxTester.open();
340+
341+
expect(comboboxTester.options()).toHaveLength(7);
342+
});
298343
});

0 commit comments

Comments
 (0)