From 69c05497ebfd4e5d8c311b85307ea7f4acf4388e Mon Sep 17 00:00:00 2001 From: atomiks Date: Mon, 27 Jul 2026 14:34:38 +1000 Subject: [PATCH] [field] Fix stale filled and focused state on control replacement --- .../src/checkbox/root/CheckboxRoot.test.tsx | 66 +++++++ .../src/field/control/FieldControl.test.tsx | 166 ++++++++++++++++++ .../react/src/field/control/FieldControl.tsx | 49 ++++-- .../src/otp-field/root/OTPFieldRoot.test.tsx | 33 +++- 4 files changed, 301 insertions(+), 13 deletions(-) diff --git a/packages/react/src/checkbox/root/CheckboxRoot.test.tsx b/packages/react/src/checkbox/root/CheckboxRoot.test.tsx index 96d483f6296..a2468ec1c44 100644 --- a/packages/react/src/checkbox/root/CheckboxRoot.test.tsx +++ b/packages/react/src/checkbox/root/CheckboxRoot.test.tsx @@ -1623,6 +1623,72 @@ describe('', () => { }); }); + // Association is re-checked whenever the control commits. Label changes that occur with no + // control rerender at all (e.g. next to a memoized control) are not tracked in v1. + describe('fallback `aria-labelledby` with an independently rendered label', () => { + it('tracks a label toggled in a different container than the wrapped control', async () => { + function WrappedTestCase() { + const [showLabel, setShowLabel] = React.useState(false); + + return ( +
+
{showLabel && }
+
+ +
+ +
+ ); + } + + await render(); + + const checkbox = screen.getByRole('checkbox'); + expect(checkbox).not.toHaveAttribute('aria-labelledby'); + + fireEvent.click(screen.getByRole('button', { name: 'Toggle label' })); + + await waitFor(() => { + expect(checkbox).toHaveAttribute('aria-labelledby', screen.getByText('Label').id); + }); + + fireEvent.click(screen.getByRole('button', { name: 'Toggle label' })); + + await waitFor(() => { + expect(checkbox).not.toHaveAttribute('aria-labelledby'); + }); + }); + + it('tracks a label whose `htmlFor` is retargeted to the control', async () => { + function RetargetTestCase() { + const [target, setTarget] = React.useState('other-input'); + + return ( +
+ + + +
+ ); + } + + await render(); + + const checkbox = screen.getByRole('checkbox'); + expect(checkbox).not.toHaveAttribute('aria-labelledby'); + + fireEvent.click(screen.getByRole('button', { name: 'Retarget' })); + + await waitFor(() => { + expect(checkbox).toHaveAttribute('aria-labelledby', screen.getByText('Label').id); + }); + }); + }); + it('can render a native button', async () => { const { container, user } = await render(} nativeButton />); diff --git a/packages/react/src/field/control/FieldControl.test.tsx b/packages/react/src/field/control/FieldControl.test.tsx index 2a4f3d6f603..7494882b862 100644 --- a/packages/react/src/field/control/FieldControl.test.tsx +++ b/packages/react/src/field/control/FieldControl.test.tsx @@ -1,4 +1,5 @@ import { expect, vi } from 'vitest'; +import * as React from 'react'; import { createRenderer, fireEvent, screen } from '@mui/internal-test-utils'; import { Field } from '@base-ui/react/field'; import { Form } from '@base-ui/react/form'; @@ -98,6 +99,171 @@ describe('', () => { expect(screen.getByText('Required')).toBeInTheDocument(); }); + describe('filled state ownership', () => { + it('publishes an empty state when a filled control is replaced by a fresh one', async () => { + function TestCase() { + const [instance, setInstance] = React.useState(0); + + return ( + + + + + ); + } + + await render(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'a' } }); + expect(screen.getByTestId('root')).toHaveAttribute('data-filled', ''); + + fireEvent.click(screen.getByRole('button', { name: 'Replace' })); + + expect(screen.getByRole('textbox')).toHaveValue(''); + expect(screen.getByTestId('root')).not.toHaveAttribute('data-filled'); + }); + + it('does not let a superseded control clear the active control state', async () => { + function TestCase() { + const [value, setValue] = React.useState('a'); + + return ( + + + + + + ); + } + + await render(); + + expect(screen.getByTestId('root')).toHaveAttribute('data-filled', ''); + + fireEvent.click(screen.getByRole('button', { name: 'Clear first' })); + + expect(screen.getByTestId('root')).toHaveAttribute('data-filled', ''); + }); + + it('keeps the active control readable after a superseded control unmounts', async () => { + const validate = vi.fn<(value: unknown) => string | null>(() => null); + + function TestCase() { + const actionsRef = React.useRef(null); + const [oldMounted, setOldMounted] = React.useState(true); + + return ( +
+ + {oldMounted && } + + + + +
+ ); + } + + await render(); + + fireEvent.click(screen.getByRole('button', { name: 'Unmount old' })); + fireEvent.click(screen.getByRole('button', { name: 'Validate' })); + + expect(validate).toHaveBeenCalledTimes(1); + expect(validate.mock.lastCall?.[0]).toBe('new'); + }); + + it('lets a remaining control publish after the owning control unmounts', async () => { + function TestCase() { + const [value, setValue] = React.useState('a'); + const [mounted, setMounted] = React.useState(true); + + return ( + + + {mounted && } + + + + ); + } + + await render(); + + expect(screen.getByTestId('root')).toHaveAttribute('data-filled', ''); + + fireEvent.click(screen.getByRole('button', { name: 'Unmount second' })); + fireEvent.click(screen.getByRole('button', { name: 'Clear first' })); + + expect(screen.getByTestId('root')).not.toHaveAttribute('data-filled'); + }); + }); + + describe('focused state ownership', () => { + it('releases the focused state when the focused control unmounts', async () => { + function TestCase() { + const [mounted, setMounted] = React.useState(true); + + return ( + + {mounted && } + + + ); + } + + await render(); + + fireEvent.focus(screen.getByRole('textbox')); + expect(screen.getByTestId('root')).toHaveAttribute('data-focused', ''); + + fireEvent.click(screen.getByRole('button', { name: 'Remove' })); + + expect(screen.getByTestId('root')).not.toHaveAttribute('data-focused'); + }); + + it('does not let a blurred control release the focused state of another control', async () => { + function TestCase() { + const [mounted, setMounted] = React.useState(true); + + return ( + + {mounted && } + + + + ); + } + + await render(); + + fireEvent.focus(screen.getByTestId('first')); + fireEvent.blur(screen.getByTestId('first')); + fireEvent.focus(screen.getByTestId('second')); + expect(screen.getByTestId('root')).toHaveAttribute('data-focused', ''); + + fireEvent.click(screen.getByRole('button', { name: 'Remove first' })); + + expect(screen.getByTestId('root')).toHaveAttribute('data-focused', ''); + }); + }); + it.skipIf(isJSDOM)('should sync focused state when autoFocus is used with SSR', async () => { vi.spyOn(console, 'error') .mockName('console.error') diff --git a/packages/react/src/field/control/FieldControl.tsx b/packages/react/src/field/control/FieldControl.tsx index 619511afd42..cc5919b5253 100644 --- a/packages/react/src/field/control/FieldControl.tsx +++ b/packages/react/src/field/control/FieldControl.tsx @@ -72,22 +72,45 @@ export const FieldControl = React.forwardRef(function FieldControl( const id = useLabelableId({ id: idProp }); + const inputRef = React.useRef(null); + + // The field's `filled` state belongs to whichever control currently owns the shared input ref, + // which is the last one to attach. Publishing it unconditionally stops a fresh control from + // inheriting the state of the control it replaced, while the ownership check stops a + // superseded control from clearing the active one's state when it rerenders. A null ref means + // the owner unmounted, so the next control to get here reclaims it. useIsoLayoutEffect(() => { - const hasExternalValue = valueProp != null; - if (validation.inputRef.current?.value || (hasExternalValue && valueProp !== '')) { - setFilled(true); - } else if (hasExternalValue && valueProp === '') { - setFilled(false); + if (validation.inputRef.current !== null && validation.inputRef.current !== inputRef.current) { + return; } + + validation.inputRef.current = inputRef.current; + setFilled(valueProp != null ? valueProp !== '' : Boolean(inputRef.current?.value)); }, [validation.inputRef, setFilled, valueProp]); - const inputRef = React.useRef(null); + const focusedRef = React.useRef(false); + + const updateFocused = useStableCallback((focused: boolean) => { + focusedRef.current = focused; + setFocused(focused); + }); + + // A control removed while focused never fires blur, which would leave the field focused + // forever. Only release the state when this control is the one still holding it. + useIsoLayoutEffect( + () => () => { + if (focusedRef.current) { + setFocused(false); + } + }, + [setFocused], + ); useIsoLayoutEffect(() => { if (autoFocus && inputRef.current === activeElement(ownerDocument(inputRef.current))) { - setFocused(true); + updateFocused(true); } - }, [autoFocus, setFocused]); + }, [autoFocus, updateFocused]); const [valueUnwrapped] = useControlled({ controlled: valueProp, @@ -98,9 +121,11 @@ export const FieldControl = React.forwardRef(function FieldControl( const isControlled = valueProp !== undefined; const value = isControlled ? valueUnwrapped : undefined; - const getValueFromInput = useStableCallback(() => validation.inputRef.current?.value); + // Read this control's own element, not the mutable shared ref, so the active registration + // stays readable regardless of which control last touched the shared ref. + const getValueFromInput = useStableCallback(() => inputRef.current?.value); - useRegisterFieldControl(validation.inputRef, id, value, getValueFromInput, !disabled, nameProp); + useRegisterFieldControl(inputRef, id, value, getValueFromInput, !disabled, nameProp); const element = useRenderElement('input', componentProps, { ref: [forwardedRef, inputRef], @@ -128,11 +153,11 @@ export const FieldControl = React.forwardRef(function FieldControl( } }, onFocus() { - setFocused(true); + updateFocused(true); }, onBlur(event) { setTouched(true); - setFocused(false); + updateFocused(false); if (validationMode === 'onBlur') { validation.commit(event.currentTarget.value); diff --git a/packages/react/src/otp-field/root/OTPFieldRoot.test.tsx b/packages/react/src/otp-field/root/OTPFieldRoot.test.tsx index ddc7864ad53..5cbb8c3a156 100644 --- a/packages/react/src/otp-field/root/OTPFieldRoot.test.tsx +++ b/packages/react/src/otp-field/root/OTPFieldRoot.test.tsx @@ -1,7 +1,7 @@ import { expect, vi } from 'vitest'; import * as React from 'react'; import { SafeReact } from '@base-ui/utils/safeReact'; -import { act, fireEvent, screen } from '@mui/internal-test-utils'; +import { act, fireEvent, screen, waitFor } from '@mui/internal-test-utils'; import { OTPField as OTPFieldBase } from '@base-ui/react/otp-field'; import { Field } from '@base-ui/react/field'; import { Form } from '@base-ui/react/form'; @@ -713,6 +713,37 @@ describe('', () => { }); describe('accessibility', () => { + it('associates a wrapping native label when inputs mount after the root', async () => { + function TestCase() { + const [show, setShow] = React.useState(false); + + return ( +
+ + +
+ ); + } + + await render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show' })); + + await waitFor(() => { + expect(screen.getByRole('group')).toHaveAttribute( + 'aria-labelledby', + screen.getByText('Code').id, + ); + }); + }); + it('forwards root `aria-describedby` to the group', async () => { await render();