Skip to content

Commit 69c0549

Browse files
atomiksatomiks
committed
[field] Fix stale filled and focused state on control replacement
1 parent ffea482 commit 69c0549

4 files changed

Lines changed: 301 additions & 13 deletions

File tree

‎packages/react/src/checkbox/root/CheckboxRoot.test.tsx‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,72 @@ describe('<Checkbox.Root />', () => {
16231623
});
16241624
});
16251625

1626+
// Association is re-checked whenever the control commits. Label changes that occur with no
1627+
// control rerender at all (e.g. next to a memoized control) are not tracked in v1.
1628+
describe('fallback `aria-labelledby` with an independently rendered label', () => {
1629+
it('tracks a label toggled in a different container than the wrapped control', async () => {
1630+
function WrappedTestCase() {
1631+
const [showLabel, setShowLabel] = React.useState(false);
1632+
1633+
return (
1634+
<div>
1635+
<div>{showLabel && <label htmlFor="checkbox-input">Label</label>}</div>
1636+
<div>
1637+
<Checkbox.Root id="checkbox-input" />
1638+
</div>
1639+
<button type="button" onClick={() => setShowLabel((prev) => !prev)}>
1640+
Toggle label
1641+
</button>
1642+
</div>
1643+
);
1644+
}
1645+
1646+
await render(<WrappedTestCase />);
1647+
1648+
const checkbox = screen.getByRole('checkbox');
1649+
expect(checkbox).not.toHaveAttribute('aria-labelledby');
1650+
1651+
fireEvent.click(screen.getByRole('button', { name: 'Toggle label' }));
1652+
1653+
await waitFor(() => {
1654+
expect(checkbox).toHaveAttribute('aria-labelledby', screen.getByText('Label').id);
1655+
});
1656+
1657+
fireEvent.click(screen.getByRole('button', { name: 'Toggle label' }));
1658+
1659+
await waitFor(() => {
1660+
expect(checkbox).not.toHaveAttribute('aria-labelledby');
1661+
});
1662+
});
1663+
1664+
it('tracks a label whose `htmlFor` is retargeted to the control', async () => {
1665+
function RetargetTestCase() {
1666+
const [target, setTarget] = React.useState('other-input');
1667+
1668+
return (
1669+
<div>
1670+
<label htmlFor={target}>Label</label>
1671+
<Checkbox.Root id="checkbox-input" />
1672+
<button type="button" onClick={() => setTarget('checkbox-input')}>
1673+
Retarget
1674+
</button>
1675+
</div>
1676+
);
1677+
}
1678+
1679+
await render(<RetargetTestCase />);
1680+
1681+
const checkbox = screen.getByRole('checkbox');
1682+
expect(checkbox).not.toHaveAttribute('aria-labelledby');
1683+
1684+
fireEvent.click(screen.getByRole('button', { name: 'Retarget' }));
1685+
1686+
await waitFor(() => {
1687+
expect(checkbox).toHaveAttribute('aria-labelledby', screen.getByText('Label').id);
1688+
});
1689+
});
1690+
});
1691+
16261692
it('can render a native button', async () => {
16271693
const { container, user } = await render(<Checkbox.Root render={<button />} nativeButton />);
16281694

‎packages/react/src/field/control/FieldControl.test.tsx‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect, vi } from 'vitest';
2+
import * as React from 'react';
23
import { createRenderer, fireEvent, screen } from '@mui/internal-test-utils';
34
import { Field } from '@base-ui/react/field';
45
import { Form } from '@base-ui/react/form';
@@ -98,6 +99,171 @@ describe('<Field.Control />', () => {
9899
expect(screen.getByText('Required')).toBeInTheDocument();
99100
});
100101

102+
describe('filled state ownership', () => {
103+
it('publishes an empty state when a filled control is replaced by a fresh one', async () => {
104+
function TestCase() {
105+
const [instance, setInstance] = React.useState(0);
106+
107+
return (
108+
<Field.Root data-testid="root">
109+
<Field.Control key={instance} />
110+
<button type="button" onClick={() => setInstance(1)}>
111+
Replace
112+
</button>
113+
</Field.Root>
114+
);
115+
}
116+
117+
await render(<TestCase />);
118+
119+
fireEvent.change(screen.getByRole('textbox'), { target: { value: 'a' } });
120+
expect(screen.getByTestId('root')).toHaveAttribute('data-filled', '');
121+
122+
fireEvent.click(screen.getByRole('button', { name: 'Replace' }));
123+
124+
expect(screen.getByRole('textbox')).toHaveValue('');
125+
expect(screen.getByTestId('root')).not.toHaveAttribute('data-filled');
126+
});
127+
128+
it('does not let a superseded control clear the active control state', async () => {
129+
function TestCase() {
130+
const [value, setValue] = React.useState('a');
131+
132+
return (
133+
<Field.Root data-testid="root">
134+
<Field.Control value={value} onValueChange={setValue} />
135+
<Field.Control defaultValue="filled" />
136+
<button type="button" onClick={() => setValue('')}>
137+
Clear first
138+
</button>
139+
</Field.Root>
140+
);
141+
}
142+
143+
await render(<TestCase />);
144+
145+
expect(screen.getByTestId('root')).toHaveAttribute('data-filled', '');
146+
147+
fireEvent.click(screen.getByRole('button', { name: 'Clear first' }));
148+
149+
expect(screen.getByTestId('root')).toHaveAttribute('data-filled', '');
150+
});
151+
152+
it('keeps the active control readable after a superseded control unmounts', async () => {
153+
const validate = vi.fn<(value: unknown) => string | null>(() => null);
154+
155+
function TestCase() {
156+
const actionsRef = React.useRef<Field.Root.Actions>(null);
157+
const [oldMounted, setOldMounted] = React.useState(true);
158+
159+
return (
160+
<div>
161+
<Field.Root validate={validate} actionsRef={actionsRef}>
162+
{oldMounted && <Field.Control key="old" defaultValue="old" />}
163+
<Field.Control key="new" defaultValue="new" />
164+
</Field.Root>
165+
<button type="button" onClick={() => setOldMounted(false)}>
166+
Unmount old
167+
</button>
168+
<button type="button" onClick={() => actionsRef.current?.validate()}>
169+
Validate
170+
</button>
171+
</div>
172+
);
173+
}
174+
175+
await render(<TestCase />);
176+
177+
fireEvent.click(screen.getByRole('button', { name: 'Unmount old' }));
178+
fireEvent.click(screen.getByRole('button', { name: 'Validate' }));
179+
180+
expect(validate).toHaveBeenCalledTimes(1);
181+
expect(validate.mock.lastCall?.[0]).toBe('new');
182+
});
183+
184+
it('lets a remaining control publish after the owning control unmounts', async () => {
185+
function TestCase() {
186+
const [value, setValue] = React.useState('a');
187+
const [mounted, setMounted] = React.useState(true);
188+
189+
return (
190+
<Field.Root data-testid="root">
191+
<Field.Control value={value} onValueChange={setValue} />
192+
{mounted && <Field.Control defaultValue="filled" />}
193+
<button type="button" onClick={() => setMounted(false)}>
194+
Unmount second
195+
</button>
196+
<button type="button" onClick={() => setValue('')}>
197+
Clear first
198+
</button>
199+
</Field.Root>
200+
);
201+
}
202+
203+
await render(<TestCase />);
204+
205+
expect(screen.getByTestId('root')).toHaveAttribute('data-filled', '');
206+
207+
fireEvent.click(screen.getByRole('button', { name: 'Unmount second' }));
208+
fireEvent.click(screen.getByRole('button', { name: 'Clear first' }));
209+
210+
expect(screen.getByTestId('root')).not.toHaveAttribute('data-filled');
211+
});
212+
});
213+
214+
describe('focused state ownership', () => {
215+
it('releases the focused state when the focused control unmounts', async () => {
216+
function TestCase() {
217+
const [mounted, setMounted] = React.useState(true);
218+
219+
return (
220+
<Field.Root data-testid="root">
221+
{mounted && <Field.Control />}
222+
<button type="button" onClick={() => setMounted(false)}>
223+
Remove
224+
</button>
225+
</Field.Root>
226+
);
227+
}
228+
229+
await render(<TestCase />);
230+
231+
fireEvent.focus(screen.getByRole('textbox'));
232+
expect(screen.getByTestId('root')).toHaveAttribute('data-focused', '');
233+
234+
fireEvent.click(screen.getByRole('button', { name: 'Remove' }));
235+
236+
expect(screen.getByTestId('root')).not.toHaveAttribute('data-focused');
237+
});
238+
239+
it('does not let a blurred control release the focused state of another control', async () => {
240+
function TestCase() {
241+
const [mounted, setMounted] = React.useState(true);
242+
243+
return (
244+
<Field.Root data-testid="root">
245+
{mounted && <Field.Control data-testid="first" />}
246+
<Field.Control data-testid="second" />
247+
<button type="button" onClick={() => setMounted(false)}>
248+
Remove first
249+
</button>
250+
</Field.Root>
251+
);
252+
}
253+
254+
await render(<TestCase />);
255+
256+
fireEvent.focus(screen.getByTestId('first'));
257+
fireEvent.blur(screen.getByTestId('first'));
258+
fireEvent.focus(screen.getByTestId('second'));
259+
expect(screen.getByTestId('root')).toHaveAttribute('data-focused', '');
260+
261+
fireEvent.click(screen.getByRole('button', { name: 'Remove first' }));
262+
263+
expect(screen.getByTestId('root')).toHaveAttribute('data-focused', '');
264+
});
265+
});
266+
101267
it.skipIf(isJSDOM)('should sync focused state when autoFocus is used with SSR', async () => {
102268
vi.spyOn(console, 'error')
103269
.mockName('console.error')

‎packages/react/src/field/control/FieldControl.tsx‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,22 +72,45 @@ export const FieldControl = React.forwardRef(function FieldControl(
7272

7373
const id = useLabelableId({ id: idProp });
7474

75+
const inputRef = React.useRef<HTMLInputElement>(null);
76+
77+
// The field's `filled` state belongs to whichever control currently owns the shared input ref,
78+
// which is the last one to attach. Publishing it unconditionally stops a fresh control from
79+
// inheriting the state of the control it replaced, while the ownership check stops a
80+
// superseded control from clearing the active one's state when it rerenders. A null ref means
81+
// the owner unmounted, so the next control to get here reclaims it.
7582
useIsoLayoutEffect(() => {
76-
const hasExternalValue = valueProp != null;
77-
if (validation.inputRef.current?.value || (hasExternalValue && valueProp !== '')) {
78-
setFilled(true);
79-
} else if (hasExternalValue && valueProp === '') {
80-
setFilled(false);
83+
if (validation.inputRef.current !== null && validation.inputRef.current !== inputRef.current) {
84+
return;
8185
}
86+
87+
validation.inputRef.current = inputRef.current;
88+
setFilled(valueProp != null ? valueProp !== '' : Boolean(inputRef.current?.value));
8289
}, [validation.inputRef, setFilled, valueProp]);
8390

84-
const inputRef = React.useRef<HTMLElement>(null);
91+
const focusedRef = React.useRef(false);
92+
93+
const updateFocused = useStableCallback((focused: boolean) => {
94+
focusedRef.current = focused;
95+
setFocused(focused);
96+
});
97+
98+
// A control removed while focused never fires blur, which would leave the field focused
99+
// forever. Only release the state when this control is the one still holding it.
100+
useIsoLayoutEffect(
101+
() => () => {
102+
if (focusedRef.current) {
103+
setFocused(false);
104+
}
105+
},
106+
[setFocused],
107+
);
85108

86109
useIsoLayoutEffect(() => {
87110
if (autoFocus && inputRef.current === activeElement(ownerDocument(inputRef.current))) {
88-
setFocused(true);
111+
updateFocused(true);
89112
}
90-
}, [autoFocus, setFocused]);
113+
}, [autoFocus, updateFocused]);
91114

92115
const [valueUnwrapped] = useControlled({
93116
controlled: valueProp,
@@ -98,9 +121,11 @@ export const FieldControl = React.forwardRef(function FieldControl(
98121

99122
const isControlled = valueProp !== undefined;
100123
const value = isControlled ? valueUnwrapped : undefined;
101-
const getValueFromInput = useStableCallback(() => validation.inputRef.current?.value);
124+
// Read this control's own element, not the mutable shared ref, so the active registration
125+
// stays readable regardless of which control last touched the shared ref.
126+
const getValueFromInput = useStableCallback(() => inputRef.current?.value);
102127

103-
useRegisterFieldControl(validation.inputRef, id, value, getValueFromInput, !disabled, nameProp);
128+
useRegisterFieldControl(inputRef, id, value, getValueFromInput, !disabled, nameProp);
104129

105130
const element = useRenderElement('input', componentProps, {
106131
ref: [forwardedRef, inputRef],
@@ -128,11 +153,11 @@ export const FieldControl = React.forwardRef(function FieldControl(
128153
}
129154
},
130155
onFocus() {
131-
setFocused(true);
156+
updateFocused(true);
132157
},
133158
onBlur(event) {
134159
setTouched(true);
135-
setFocused(false);
160+
updateFocused(false);
136161

137162
if (validationMode === 'onBlur') {
138163
validation.commit(event.currentTarget.value);

‎packages/react/src/otp-field/root/OTPFieldRoot.test.tsx‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect, vi } from 'vitest';
22
import * as React from 'react';
33
import { SafeReact } from '@base-ui/utils/safeReact';
4-
import { act, fireEvent, screen } from '@mui/internal-test-utils';
4+
import { act, fireEvent, screen, waitFor } from '@mui/internal-test-utils';
55
import { OTPField as OTPFieldBase } from '@base-ui/react/otp-field';
66
import { Field } from '@base-ui/react/field';
77
import { Form } from '@base-ui/react/form';
@@ -713,6 +713,37 @@ describe('<OTPField.Root />', () => {
713713
});
714714

715715
describe('accessibility', () => {
716+
it('associates a wrapping native label when inputs mount after the root', async () => {
717+
function TestCase() {
718+
const [show, setShow] = React.useState(false);
719+
720+
return (
721+
<div>
722+
<label>
723+
Code
724+
<OTPFieldBase.Root length={2}>
725+
{show && [<OTPFieldBase.Input key={0} />, <OTPFieldBase.Input key={1} />]}
726+
</OTPFieldBase.Root>
727+
</label>
728+
<button type="button" onClick={() => setShow(true)}>
729+
Show
730+
</button>
731+
</div>
732+
);
733+
}
734+
735+
await render(<TestCase />);
736+
737+
fireEvent.click(screen.getByRole('button', { name: 'Show' }));
738+
739+
await waitFor(() => {
740+
expect(screen.getByRole('group')).toHaveAttribute(
741+
'aria-labelledby',
742+
screen.getByText('Code').id,
743+
);
744+
});
745+
});
746+
716747
it('forwards root `aria-describedby` to the group', async () => {
717748
await render(<OTPField aria-describedby="description-id" />);
718749

0 commit comments

Comments
 (0)