Skip to content

Commit 32cabb7

Browse files
atomiksatomiks
authored
[form][field][fieldset] Expand test coverage (#5281)
1 parent 34d9618 commit 32cabb7

16 files changed

Lines changed: 372 additions & 60 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ export const CheckboxRoot = React.forwardRef(function CheckboxRoot(
181181
const registerFieldInput = validation.registerInput;
182182
const registeredInputValue = groupContext ? value : undefined;
183183
const registerInput = React.useCallback(
184-
(element: HTMLInputElement | null) =>
184+
(element: HTMLInputElement) =>
185185
registerFieldInput(element, { controlRef, value: registeredInputValue }),
186186
[registerFieldInput, registeredInputValue],
187187
);

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect, vi } from 'vitest';
22
import { createRenderer, fireEvent, screen } from '@mui/internal-test-utils';
33
import { Field } from '@base-ui/react/field';
4+
import { Form } from '@base-ui/react/form';
45
import { describeConformance, isJSDOM } from '#test-utils';
56

67
describe('<Field.Control />', () => {
@@ -56,6 +57,31 @@ describe('<Field.Control />', () => {
5657
expect(validate.mock.lastCall?.[0]).toBe('a');
5758
});
5859

60+
it('does not clear errors or validate when change is prevented', async () => {
61+
const validate = vi.fn();
62+
const handleValueChange = vi.fn();
63+
64+
await render(
65+
<Form errors={{ message: 'Server error' }}>
66+
<Field.Root name="message" validationMode="onChange" validate={validate}>
67+
<Field.Control onValueChange={handleValueChange} />
68+
<Field.Error />
69+
</Field.Root>
70+
</Form>,
71+
);
72+
73+
const control = screen.getByRole<HTMLInputElement>('textbox');
74+
control.addEventListener('input', (event) => event.preventDefault(), {
75+
capture: true,
76+
once: true,
77+
});
78+
fireEvent.input(control, { cancelable: true, target: { value: 'a' } });
79+
80+
expect(handleValueChange).toHaveBeenCalledTimes(1);
81+
expect(validate).not.toHaveBeenCalled();
82+
expect(screen.getByText('Server error')).toBeInTheDocument();
83+
});
84+
5985
it('shows a required error when a prefilled value is cleared', async () => {
6086
await render(
6187
<Field.Root validationMode="onChange">

‎packages/react/src/field/description/FieldDescription.test.tsx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,17 @@ describe('<Field.Description />', () => {
4040
);
4141
});
4242

43+
it('does not register an empty description id', () => {
44+
render(
45+
<Field.Root>
46+
<Field.Control aria-describedby="external-description" />
47+
<Field.Description id="">Message</Field.Description>
48+
</Field.Root>,
49+
);
50+
51+
expect(screen.getByRole('textbox')).toHaveAttribute('aria-describedby', 'external-description');
52+
});
53+
4354
it('reflects the disabled state from Field.Item', async () => {
4455
await render(
4556
<Field.Root>

‎packages/react/src/field/error/FieldError.test.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,40 @@ describe('<Field.Error />', () => {
211211
expect(screen.getByTestId('default-error')).toHaveTextContent('Username is reserved');
212212
});
213213

214+
it('renders client validation error arrays as a list', async () => {
215+
await render(
216+
<Form>
217+
<Field.Root validate={() => ['First error', 'Second error']}>
218+
<Field.Control />
219+
<Field.Error data-testid="default-error" />
220+
</Field.Root>
221+
<button type="submit">submit</button>
222+
</Form>,
223+
);
224+
225+
fireEvent.click(screen.getByText('submit'));
226+
227+
const list = screen.getByTestId('default-error').querySelector('ul');
228+
expect(list).not.toBe(null);
229+
expect(list?.querySelectorAll('li')).toHaveLength(2);
230+
expect(screen.getByText('First error')).not.toBe(null);
231+
expect(screen.getByText('Second error')).not.toBe(null);
232+
});
233+
234+
it('does not register an empty error id', async () => {
235+
await render(
236+
<Field.Root invalid>
237+
<Field.Control aria-describedby="external-description" />
238+
<Field.Error id="">Message</Field.Error>
239+
</Field.Root>,
240+
);
241+
242+
expect(screen.getByRole('textbox')).toHaveAttribute(
243+
'aria-describedby',
244+
'external-description',
245+
);
246+
});
247+
214248
it('ignores empty Form error arrays', async () => {
215249
await render(
216250
<Form errors={{ username: [] }}>

‎packages/react/src/field/error/FieldError.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export const FieldError = React.forwardRef(function FieldError(
7878
error = validityData.errors;
7979
}
8080

81-
let errorMessage: React.ReactNode = error ?? '';
81+
let errorMessage: React.ReactNode = error;
8282
if (Array.isArray(error)) {
8383
errorMessage =
8484
error.length > 1 ? (
@@ -88,7 +88,7 @@ export const FieldError = React.forwardRef(function FieldError(
8888
))}
8989
</ul>
9090
) : (
91-
(error[0] ?? '')
91+
error[0]
9292
);
9393
}
9494

‎packages/react/src/field/item/FieldItem.test.tsx‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,31 @@ describe('<Field.Item />', () => {
8181
expect(onValueChange.mock.calls.length).toBe(1);
8282
});
8383
});
84+
85+
it('associates a Field.Item label with a parent checkbox', async () => {
86+
const { user } = await render(
87+
<Field.Root>
88+
<CheckboxGroup allValues={['a', 'b']}>
89+
<Field.Item>
90+
<Field.Label>
91+
<Checkbox.Root parent data-testid="parent" />
92+
Toggle all
93+
</Field.Label>
94+
</Field.Item>
95+
<Checkbox.Root value="a" data-testid="a" />
96+
<Checkbox.Root value="b" data-testid="b" />
97+
</CheckboxGroup>
98+
</Field.Root>,
99+
);
100+
101+
const label = screen.getByText('Toggle all').closest('label') as HTMLLabelElement;
102+
const parent = screen.getByTestId('parent');
103+
104+
expect(label).toHaveAttribute('for');
105+
expect(label.control).toHaveAttribute('type', 'checkbox');
106+
await user.click(screen.getByText('Toggle all'));
107+
expect(parent).toHaveAttribute('aria-checked', 'true');
108+
expect(screen.getByTestId('a')).toHaveAttribute('aria-checked', 'true');
109+
expect(screen.getByTestId('b')).toHaveAttribute('aria-checked', 'true');
110+
});
84111
});

‎packages/react/src/field/item/FieldItem.tsx‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { BaseUIComponentProps } from '../../internals/types';
77
import { useRenderElement } from '../../internals/useRenderElement';
88
import { FieldItemContext } from './FieldItemContext';
99
import { LabelableProvider } from '../../internals/labelable-provider';
10-
import { useCheckboxGroupContext } from '../../checkbox-group/CheckboxGroupContext';
1110

1211
/**
1312
* Groups individual items in a checkbox group or radio group with a label and description.
@@ -32,10 +31,6 @@ export const FieldItem = React.forwardRef(function FieldItem(
3231
const disabled = rootDisabled || disabledProp;
3332
const state: FieldItemState = { ...fieldState, disabled };
3433

35-
const checkboxGroupContext = useCheckboxGroupContext();
36-
const hasParentCheckbox = checkboxGroupContext?.allValues !== undefined;
37-
const controlId = hasParentCheckbox ? checkboxGroupContext?.parent.id : undefined;
38-
3934
const fieldItemContext: FieldItemContext = React.useMemo(() => ({ disabled }), [disabled]);
4035

4136
const element = useRenderElement('div', componentProps, {
@@ -46,7 +41,7 @@ export const FieldItem = React.forwardRef(function FieldItem(
4641
});
4742

4843
return (
49-
<LabelableProvider controlId={controlId}>
44+
<LabelableProvider>
5045
<FieldItemContext.Provider value={fieldItemContext}>{element}</FieldItemContext.Provider>
5146
</LabelableProvider>
5247
);

‎packages/react/src/field/label/FieldLabel.test.tsx‎

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect } from 'vitest';
2+
import * as React from 'react';
23
import { Field } from '@base-ui/react/field';
34
import { screen } from '@mui/internal-test-utils';
45
import { createRenderer, describeConformance } from '#test-utils';
@@ -74,55 +75,81 @@ describe('<Field.Label />', () => {
7475
errorSpy.mockRestore();
7576
});
7677

77-
it('errors if nativeLabel=true but ref is not a label', async () => {
78+
it('does not warn when the render function returns no element', async () => {
7879
const errorSpy = vi
7980
.spyOn(console, 'error')
8081
.mockName('console.error')
8182
.mockImplementation(() => {});
8283

84+
const EmptyLabel = React.forwardRef(function EmptyLabel() {
85+
return null;
86+
});
87+
8388
await render(
8489
<Field.Root>
85-
<Field.Control />
86-
<Field.Label nativeLabel render={<div />}>
87-
Label
88-
</Field.Label>
90+
<Field.Label render={<EmptyLabel />}>Label</Field.Label>
8991
</Field.Root>,
9092
);
9193

92-
expect(errorSpy).toHaveBeenCalledTimes(1);
93-
expect(errorSpy).toHaveBeenCalledWith(
94-
expect.stringContaining(
95-
'Base UI: <Field.Label> expected a <label> element because the `nativeLabel` prop is true. ' +
96-
'Rendering a non-<label> disables native label association, so `htmlFor` will not ' +
97-
'work. Use a real <label> in the `render` prop, or set `nativeLabel` to `false`.',
98-
),
99-
);
94+
expect(errorSpy).not.toHaveBeenCalled();
10095
errorSpy.mockRestore();
10196
});
10297

103-
it('errors if nativeLabel=false but ref is a label', async () => {
98+
it('errors if nativeLabel=true but ref is not a label', async () => {
10499
const errorSpy = vi
105100
.spyOn(console, 'error')
106101
.mockName('console.error')
107102
.mockImplementation(() => {});
108103

109-
await render(
110-
<Field.Root>
111-
<Field.Control />
112-
<Field.Label nativeLabel={false}>Label</Field.Label>
113-
</Field.Root>,
114-
);
104+
try {
105+
await render(
106+
<Field.Root>
107+
<Field.Control />
108+
<Field.Label nativeLabel render={<div />}>
109+
Label
110+
</Field.Label>
111+
</Field.Root>,
112+
);
113+
114+
expect(errorSpy).toHaveBeenCalledTimes(1);
115+
expect(errorSpy).toHaveBeenCalledWith(
116+
expect.stringContaining(
117+
'Base UI: <Field.Label> expected a <label> element because the `nativeLabel` prop is true. ' +
118+
'Rendering a non-<label> disables native label association, so `htmlFor` will not ' +
119+
'work. Use a real <label> in the `render` prop, or set `nativeLabel` to `false`.',
120+
),
121+
);
122+
} finally {
123+
errorSpy.mockRestore();
124+
}
125+
});
115126

116-
expect(errorSpy).toHaveBeenCalledTimes(1);
117-
expect(errorSpy).toHaveBeenCalledWith(
118-
expect.stringContaining(
119-
'Base UI: <Field.Label> expected a non-<label> element because the `nativeLabel` prop is false. ' +
120-
'Rendering a <label> assumes native label behavior while Base UI treats it as ' +
121-
'non-native, which can cause unexpected pointer behavior. Use a non-<label> in the ' +
122-
'`render` prop, or set `nativeLabel` to `true`.',
123-
),
124-
);
125-
errorSpy.mockRestore();
127+
it('errors if nativeLabel=false but ref is a label', async () => {
128+
const errorSpy = vi
129+
.spyOn(console, 'error')
130+
.mockName('console.error')
131+
.mockImplementation(() => {});
132+
133+
try {
134+
await render(
135+
<Field.Root>
136+
<Field.Control />
137+
<Field.Label nativeLabel={false}>Label</Field.Label>
138+
</Field.Root>,
139+
);
140+
141+
expect(errorSpy).toHaveBeenCalledTimes(1);
142+
expect(errorSpy).toHaveBeenCalledWith(
143+
expect.stringContaining(
144+
'Base UI: <Field.Label> expected a non-<label> element because the `nativeLabel` prop is false. ' +
145+
'Rendering a <label> assumes native label behavior while Base UI treats it as ' +
146+
'non-native, which can cause unexpected pointer behavior. Use a non-<label> in the ' +
147+
'`render` prop, or set `nativeLabel` to `true`.',
148+
),
149+
);
150+
} finally {
151+
errorSpy.mockRestore();
152+
}
126153
});
127154
});
128155
});

‎packages/react/src/field/root/FieldRoot.react17.test.tsx‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { expect, vi } from 'vitest';
2+
import * as React from 'react';
3+
import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect';
4+
import { Field } from '@base-ui/react/field';
5+
import { screen } from '@mui/internal-test-utils';
6+
import { createRenderer } from '#test-utils';
7+
8+
vi.mock('@base-ui/utils/safeReact', async (importOriginal) => {
9+
const original = await importOriginal<typeof import('@base-ui/utils/safeReact')>();
10+
11+
return {
12+
SafeReact: {
13+
...original.SafeReact,
14+
captureOwnerStack: undefined,
15+
useId: undefined,
16+
},
17+
};
18+
});
19+
20+
describe('<Field.Root /> with the React 17 id fallback', () => {
21+
const { render } = createRenderer();
22+
23+
it('allows mount-time imperative validation before the fallback id is assigned', async () => {
24+
function TestCase() {
25+
const actionsRef = React.useRef<Field.Root.Actions>(null);
26+
27+
useIsoLayoutEffect(() => {
28+
actionsRef.current?.validate();
29+
}, []);
30+
31+
return (
32+
<Field.Root actionsRef={actionsRef} validate={() => 'Mount-time error'}>
33+
<Field.Error />
34+
</Field.Root>
35+
);
36+
}
37+
38+
await render(<TestCase />);
39+
40+
expect(await screen.findByText('Mount-time error')).toBeVisible();
41+
});
42+
43+
it('reports label mismatches without the owner-stack API', async () => {
44+
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
45+
46+
try {
47+
await render(
48+
<Field.Root>
49+
<Field.Label render={<div />}>Label</Field.Label>
50+
</Field.Root>,
51+
);
52+
53+
expect(errorSpy).toHaveBeenCalledWith(
54+
expect.stringContaining('<Field.Label> expected a <label> element'),
55+
);
56+
} finally {
57+
errorSpy.mockRestore();
58+
}
59+
});
60+
61+
it('reports non-native label mismatches without the owner-stack API', async () => {
62+
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
63+
64+
try {
65+
await render(
66+
<Field.Root>
67+
<Field.Label nativeLabel={false}>Label</Field.Label>
68+
</Field.Root>,
69+
);
70+
71+
expect(errorSpy).toHaveBeenCalledWith(
72+
expect.stringContaining('<Field.Label> expected a non-<label> element'),
73+
);
74+
} finally {
75+
errorSpy.mockRestore();
76+
}
77+
});
78+
});

0 commit comments

Comments
 (0)