Skip to content

Commit 87218e2

Browse files
committed
fix(dashboard): labelled inputs keep a caller's own id and description
What the review caught: a caller passing id or aria-describedby to InputWithLabel overrode the generated attributes on the input while the label and message still used the generated id, breaking the association. The caller's id now names the field everywhere, and its description is kept with the message id added after it. The combobox's id and aria-describedby props are documented as applying to the built-in trigger.
1 parent f7abd9d commit 87218e2

3 files changed

Lines changed: 32 additions & 4 deletions

File tree

‎apps/start/src/components/forms/input-with-label.test.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,25 @@ describe('InputWithLabel', () => {
2323
expect(message?.id).not.toMatch(/\s/);
2424
});
2525

26+
test('keeps a caller-supplied id and description and adds the message to them', () => {
27+
const { container } = render(
28+
<InputWithLabel
29+
aria-describedby="outside-note"
30+
error="Required"
31+
id="client-name"
32+
label="Client name"
33+
/>
34+
);
35+
const input = container.querySelector('input');
36+
expect(input?.id).toBe('client-name');
37+
expect(container.querySelector('label')?.getAttribute('for')).toBe(
38+
'client-name'
39+
);
40+
expect(input?.getAttribute('aria-describedby')).toBe(
41+
'outside-note client-name-message'
42+
);
43+
});
44+
2645
test('omits aria-describedby when there is nothing to describe', () => {
2746
const { container } = render(<InputWithLabel label="Client name" />);
2847
expect(

‎apps/start/src/components/forms/input-with-label.tsx‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,13 @@ export const WithLabel = ({
6666
export const InputWithLabel = forwardRef<HTMLInputElement, InputWithLabelProps>(
6767
({ className, label, hint, info, error, ...inputProps }, ref) => {
6868
const hasMessage = Boolean(error ?? hint);
69-
const fieldId = useId();
69+
const generatedId = useId();
70+
// A caller's own id and description are kept; the message is added to them.
71+
const fieldId = inputProps.id ?? generatedId;
72+
const describedBy =
73+
[inputProps['aria-describedby'], hasMessage && fieldMessageId(fieldId)]
74+
.filter(Boolean)
75+
.join(' ') || undefined;
7076
return (
7177
<WithLabel
7278
className={className}
@@ -77,11 +83,11 @@ export const InputWithLabel = forwardRef<HTMLInputElement, InputWithLabelProps>(
7783
label={label}
7884
>
7985
<Input
80-
aria-describedby={hasMessage ? fieldMessageId(fieldId) : undefined}
8186
error={error}
82-
id={fieldId}
8387
ref={ref}
8488
{...inputProps}
89+
aria-describedby={describedBy}
90+
id={fieldId}
8591
/>
8692
</WithLabel>
8793
);

‎apps/start/src/components/ui/combobox.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ export interface ComboboxProps<T> {
4040
portal?: boolean;
4141
error?: string;
4242
disabled?: boolean;
43-
/** Lets a label and a hint point at the trigger. */
43+
/**
44+
* Let a label and a hint point at the built-in trigger. A custom trigger
45+
* passed as `children` carries its own id and description.
46+
*/
4447
id?: string;
4548
'aria-describedby'?: string;
4649
}

0 commit comments

Comments
 (0)