Skip to content

Commit 7991fcd

Browse files
ryanrheeatomiks
andauthored
[combobox] Prevent scroll lock when controlled value triggers re-render (#4507)
Co-authored-by: atomiks <cc.glows@gmail.com>
1 parent 324369b commit 7991fcd

2 files changed

Lines changed: 61 additions & 3 deletions

File tree

packages/react/src/combobox/positioner/ComboboxPositioner.test.tsx

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,68 @@
1-
import { expect } from 'vitest';
1+
import { expect, vi } from 'vitest';
22
import * as React from 'react';
3+
import * as ReactDOMClient from 'react-dom/client';
34
import { waitFor } from '@mui/internal-test-utils';
45
import { Combobox } from '@base-ui/react/combobox';
56
import { createRenderer, describeConformance, isJSDOM } from '#test-utils';
67

78
describe('<Combobox.Positioner />', () => {
89
const { render } = createRenderer();
910

11+
it('should not lock body scroll when controlled value={[]} triggers a re-render', async () => {
12+
// Render outside of act() to match real browser behavior where
13+
// the initial render and the useEffect re-render are separate commits.
14+
// Using act() batches them, hiding the bug.
15+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
16+
const container = document.createElement('div');
17+
document.body.appendChild(container);
18+
document.body.removeAttribute('style');
19+
document.documentElement.removeAttribute('style');
20+
21+
function Test() {
22+
const [, forceRender] = React.useState(0);
23+
React.useEffect(() => {
24+
forceRender(1);
25+
}, []);
26+
27+
return (
28+
<Combobox.Root multiple value={[]}>
29+
<Combobox.Input />
30+
<Combobox.Portal>
31+
<Combobox.Positioner />
32+
</Combobox.Portal>
33+
</Combobox.Root>
34+
);
35+
}
36+
37+
const root = ReactDOMClient.createRoot(container);
38+
root.render(<Test />);
39+
40+
// Wait for mount + useEffect re-render + setTimeout(0) in ScrollLocker.acquire
41+
await new Promise((resolve) => {
42+
setTimeout(resolve, 500);
43+
});
44+
45+
// Bug: the re-render causes forceMount, mounting the Positioner.
46+
// The Positioner's `open` is `undefined` (not `false`), so
47+
// `open && modal` evaluates to `undefined`, which triggers
48+
// useScrollLock's default parameter `enabled = true`.
49+
const bodyOverflowX = document.body.style.overflowX;
50+
const bodyOverflowY = document.body.style.overflowY;
51+
const htmlOverflowX = document.documentElement.style.overflowX;
52+
const htmlOverflowY = document.documentElement.style.overflowY;
53+
54+
root.unmount();
55+
container.remove();
56+
document.body.removeAttribute('style');
57+
document.documentElement.removeAttribute('style');
58+
consoleSpy.mockRestore();
59+
60+
expect(bodyOverflowX).not.toBe('hidden');
61+
expect(bodyOverflowY).not.toBe('hidden');
62+
expect(htmlOverflowX).not.toBe('hidden');
63+
expect(htmlOverflowY).not.toBe('hidden');
64+
});
65+
1066
describeConformance(<Combobox.Positioner />, () => ({
1167
refInstanceof: window.HTMLDivElement,
1268
render(node) {

packages/react/src/combobox/root/AriaCombobox.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
8484
onSelectedValueChange,
8585
defaultInputValue: defaultInputValueProp,
8686
inputValue: inputValueProp,
87+
open: openProp,
88+
defaultOpen = false,
8789
selectionMode = 'none',
8890
onItemHighlighted: onItemHighlightedProp,
8991
name: nameProp,
@@ -219,8 +221,8 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
219221
});
220222

221223
const [open, setOpenUnwrapped] = useControlled({
222-
controlled: props.open,
223-
default: props.defaultOpen,
224+
controlled: openProp,
225+
default: defaultOpen,
224226
name: 'Combobox',
225227
state: 'open',
226228
});

0 commit comments

Comments
 (0)