Skip to content

Commit 409d077

Browse files
git-nandorclaude
andcommitted
fix(many): keep form field messages out of the control's accessible name
Form control messages (error/hint/success) render inside the wrapping <label>, so they were read as part of the field's accessible name. Point each control's name at the label text via aria-labelledby and reference the messages via aria-describedby, so screen readers announce them as the field's description. Label click-to-focus is preserved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d1ac78c commit 409d077

13 files changed

Lines changed: 319 additions & 25 deletions

File tree

packages/ui-checkbox/src/Checkbox/__tests__/Checkbox.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,5 +289,35 @@ describe('<Checkbox />', () => {
289289

290290
expect(axeCheck).toBe(true)
291291
})
292+
293+
it('associates messages with the checkbox as its description, not its accessible name', async () => {
294+
render(
295+
<Checkbox
296+
label="Accept terms"
297+
value="v"
298+
messages={[{ type: 'error', text: 'You must accept' }]}
299+
/>
300+
)
301+
const input = screen.getByRole('checkbox')
302+
303+
const describedById = input.getAttribute('aria-describedby')
304+
expect(describedById).toBeTruthy()
305+
expect(document.getElementById(describedById!)).toHaveTextContent(
306+
'You must accept'
307+
)
308+
309+
const labelledById = input.getAttribute('aria-labelledby')
310+
expect(labelledById).toBeTruthy()
311+
const labelEl = document.getElementById(labelledById!)
312+
expect(labelEl).toHaveTextContent('Accept terms')
313+
expect(labelEl).not.toHaveTextContent('You must accept')
314+
})
315+
316+
it('does not set aria-labelledby when there are no messages', async () => {
317+
render(<Checkbox label="Accept terms" value="v" />)
318+
const input = screen.getByRole('checkbox')
319+
320+
expect(input).not.toHaveAttribute('aria-labelledby')
321+
})
292322
})
293323
})

packages/ui-checkbox/src/Checkbox/v2/index.tsx

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,12 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
8484
}
8585

8686
this._defaultId = props.deterministicId!()
87+
this._messagesId = props.deterministicId!('Checkbox-messages')
88+
this._labelId = props.deterministicId!('Checkbox-label')
8789
}
8890
private readonly _defaultId: string
91+
private readonly _messagesId: string
92+
private readonly _labelId: string
8993
private _input: HTMLInputElement | null = null
9094

9195
ref: Element | null = null
@@ -186,6 +190,10 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
186190
)
187191
}
188192

193+
get hasMessages() {
194+
return !!this.props.messages && this.props.messages.length > 0
195+
}
196+
189197
get invalid() {
190198
return !!this.props.messages?.find(
191199
(m) => m.type === 'newError' || m.type === 'error'
@@ -217,6 +225,24 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
217225
`[Checkbox] The \`simple\` variant does not support the \`labelPlacement\` property. Use the \`toggle\` variant instead.`
218226
)
219227

228+
// The label text gets its own id so that, when there are messages, the
229+
// input's accessible name can point at the label only (via
230+
// `aria-labelledby`) instead of also including the message text.
231+
const labelContent = (
232+
<span id={this._labelId}>
233+
{label}
234+
{isRequired && label && (
235+
<span
236+
css={this.invalid ? styles?.requiredInvalid : {}}
237+
aria-hidden={true}
238+
>
239+
{' '}
240+
*
241+
</span>
242+
)}
243+
</span>
244+
)
245+
220246
if (variant === 'toggle') {
221247
return (
222248
<ToggleFacade
@@ -232,16 +258,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
232258
themeOverride={themeOverride}
233259
invalid={this.invalid}
234260
>
235-
{label}
236-
{isRequired && label && (
237-
<span
238-
css={this.invalid ? styles?.requiredInvalid : {}}
239-
aria-hidden={true}
240-
>
241-
{' '}
242-
*
243-
</span>
244-
)}
261+
{labelContent}
245262
</ToggleFacade>
246263
)
247264
} else {
@@ -257,16 +274,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
257274
themeOverride={themeOverride}
258275
invalid={this.invalid}
259276
>
260-
{label}
261-
{isRequired && label && (
262-
<span
263-
css={this.invalid ? styles?.requiredInvalid : {}}
264-
aria-hidden={true}
265-
>
266-
{' '}
267-
*
268-
</span>
269-
)}
277+
{labelContent}
270278
</CheckboxFacade>
271279
)
272280
}
@@ -286,7 +294,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
286294
: styles?.indentedError)
287295
}
288296
>
289-
<FormFieldMessages messages={messages} />
297+
<FormFieldMessages id={this._messagesId} messages={messages} />
290298
</View>
291299
) : null
292300
}
@@ -338,6 +346,24 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
338346
aria-readonly={readOnly ? true : undefined}
339347
aria-checked={indeterminate ? 'mixed' : undefined}
340348
aria-invalid={this.invalid ? 'true' : undefined}
349+
// Keep messages in the description so the accessible name contains only the label.
350+
aria-labelledby={
351+
this.hasMessages
352+
? ((props as Record<string, unknown>)[
353+
'aria-labelledby'
354+
] as string) || this._labelId
355+
: ((props as Record<string, unknown>)['aria-labelledby'] as
356+
| string
357+
| undefined)
358+
}
359+
aria-describedby={
360+
[
361+
(props as Record<string, unknown>)['aria-describedby'],
362+
this.hasMessages ? this._messagesId : null
363+
]
364+
.filter(Boolean)
365+
.join(' ') || undefined
366+
}
341367
css={styles?.input}
342368
onClickCapture={(e) => {
343369
if (readOnly) {

packages/ui-form-field/src/FormField/v2/props.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ type FormFieldOwnProps = {
4646
* id for the form field messages
4747
*/
4848
messagesId?: string
49+
/**
50+
* id for the label element, so a single form control can reference just the
51+
* label text via `aria-labelledby` (keeping messages out of its accessible name)
52+
*/
53+
labelId?: string
4954
children?: React.ReactNode
5055
inline?: boolean
5156
layout?: 'stacked' | 'inline'
@@ -86,6 +91,7 @@ const allowedProps: AllowedPropKeys = [
8691
'id',
8792
'messages',
8893
'messagesId',
94+
'labelId',
8995
'children',
9096
'inline',
9197
'layout',

packages/ui-form-field/src/FormFieldLayout/v2/index.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const FormFieldLayout = forwardRef<Element, FormFieldLayoutProps>(
4848
label,
4949
messages,
5050
messagesId: messagesIdProp,
51+
labelId: labelIdProp,
5152
children,
5253
width,
5354
elementRef,
@@ -69,7 +70,12 @@ const FormFieldLayout = forwardRef<Element, FormFieldLayoutProps>(
6970
}, [])
7071

7172
const messagesId = messagesIdProp || deterministicId
72-
const labelId = deterministicId ? `${deterministicId}-Label` : undefined
73+
// The label element gets an id so that single form controls can point their
74+
// `aria-labelledby` at the label text only. This keeps `messages` (which
75+
// live inside the wrapping <label>) out of the control's accessible name
76+
// while preserving the native click-on-label focus behavior.
77+
const labelId =
78+
labelIdProp || (deterministicId ? `${deterministicId}-Label` : undefined)
7379

7480
// Filter out error and success messages when disabled or readOnly
7581
const filteredMessages =
@@ -183,7 +189,13 @@ const FormFieldLayout = forwardRef<Element, FormFieldLayoutProps>(
183189
</legend>
184190
)
185191
}
186-
return <span css={styles?.formFieldLabel}>{labelContent}</span>
192+
// `id` lets single form controls reference just the label text via
193+
// `aria-labelledby`, keeping `messages` out of their accessible name.
194+
return (
195+
<span css={styles?.formFieldLabel} id={labelId}>
196+
{labelContent}
197+
</span>
198+
)
187199
} else if (label) {
188200
if (ElementType === 'fieldset') {
189201
return (

packages/ui-form-field/src/FormFieldLayout/v2/props.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ type FormFieldLayoutOwnProps = {
5656
* id for the form field messages
5757
*/
5858
messagesId?: string
59+
/**
60+
* id for the label element, so a single form control can reference just the
61+
* label text via `aria-labelledby` (keeping messages out of its accessible name)
62+
*/
63+
labelId?: string
5964
children?: React.ReactNode
6065
/**
6166
* If `true` use an inline layout -- content will flow on the left/right side
@@ -129,6 +134,7 @@ const allowedProps: AllowedPropKeys = [
129134
'as',
130135
'messages',
131136
'messagesId',
137+
'labelId',
132138
'children',
133139
'inline',
134140
'layout',

packages/ui-number-input/src/NumberInput/__tests__/NumberInput.test.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,4 +304,33 @@ describe('<NumberInput />', () => {
304304
expect(onDecrement).toHaveBeenCalledTimes(1)
305305
})
306306
})
307+
308+
it('associates messages with the input as its description, not its accessible name', async () => {
309+
render(
310+
<NumberInput
311+
renderLabel="Label"
312+
messages={[{ type: 'error', text: 'some error message' }]}
313+
/>
314+
)
315+
const input = screen.getByRole('spinbutton')
316+
317+
const describedById = input.getAttribute('aria-describedby')
318+
expect(describedById).toBeTruthy()
319+
expect(document.getElementById(describedById!)).toHaveTextContent(
320+
'some error message'
321+
)
322+
323+
const labelledById = input.getAttribute('aria-labelledby')
324+
expect(labelledById).toBeTruthy()
325+
const labelEl = document.getElementById(labelledById!)
326+
expect(labelEl).toHaveTextContent('Label')
327+
expect(labelEl).not.toHaveTextContent('some error message')
328+
})
329+
330+
it('does not override the accessible name with aria-labelledby when there are no messages', async () => {
331+
render(<NumberInput renderLabel="Label" />)
332+
const input = screen.getByRole('spinbutton')
333+
334+
expect(input).not.toHaveAttribute('aria-labelledby')
335+
})
307336
})

packages/ui-number-input/src/NumberInput/v2/index.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
120120
const success =
121121
!!messages && messages.some((message) => message.type === 'success')
122122

123+
// Messages live inside the wrapping <label>. Reference them from the input
124+
// via `aria-describedby` and point the accessible name at the label text
125+
// only via `aria-labelledby`, so the messages are announced as a
126+
// description rather than as part of the control's name.
127+
const hasMessages = !!messages && messages.length > 0
128+
const messagesId = id ? `${id}-messages` : undefined
129+
const labelId = id ? `${id}-label` : undefined
130+
123131
const interaction = getInteraction({ props })
124132
if (
125133
interaction === 'disabled' &&
@@ -331,6 +339,8 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
331339
<FormField
332340
{...pickProps(props, FormField.allowedProps)}
333341
label={label}
342+
messagesId={messagesId}
343+
labelId={labelId}
334344
inline={display === 'inline-block'}
335345
id={id}
336346
elementRef={handleRef}
@@ -346,6 +356,19 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
346356
{...passedProps}
347357
css={styles?.input}
348358
aria-invalid={invalid ? 'true' : undefined}
359+
aria-describedby={
360+
[
361+
passedProps['aria-describedby'],
362+
hasMessages ? messagesId : null
363+
]
364+
.filter(Boolean)
365+
.join(' ') || undefined
366+
}
367+
aria-labelledby={
368+
hasMessages
369+
? (passedProps['aria-labelledby'] as string) || labelId
370+
: (passedProps['aria-labelledby'] as string | undefined)
371+
}
349372
id={id}
350373
type={allowStringValue ? 'text' : 'number'}
351374
inputMode={inputMode}

packages/ui-range-input/src/RangeInput/__tests__/RangeInput.test.tsx

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,41 @@ describe('<RangeInput />', () => {
211211
expect(axeCheck).toBe(true)
212212
})
213213

214+
it('associates messages with the input as its description, not its accessible name', async () => {
215+
const { container } = render(
216+
<RangeInput
217+
label="Opacity"
218+
name="opacity"
219+
max={100}
220+
min={0}
221+
defaultValue={50}
222+
messages={[{ type: 'error', text: 'some error message' }]}
223+
/>
224+
)
225+
const input = container.querySelector('input')!
226+
227+
const describedById = input.getAttribute('aria-describedby')
228+
expect(describedById).toBeTruthy()
229+
expect(document.getElementById(describedById!)).toHaveTextContent(
230+
'some error message'
231+
)
232+
233+
const labelledById = input.getAttribute('aria-labelledby')
234+
expect(labelledById).toBeTruthy()
235+
const labelEl = document.getElementById(labelledById!)
236+
expect(labelEl).toHaveTextContent('Opacity')
237+
expect(labelEl).not.toHaveTextContent('some error message')
238+
})
239+
240+
it('does not override the accessible name with aria-labelledby when there are no messages', async () => {
241+
const { container } = render(
242+
<RangeInput label="Opacity" name="opacity" max={100} min={0} />
243+
)
244+
const input = container.querySelector('input')!
245+
246+
expect(input).not.toHaveAttribute('aria-labelledby')
247+
})
248+
214249
it('formats the aria-valuetext attribute', async () => {
215250
const { container } = render(
216251
<RangeInput

packages/ui-range-input/src/RangeInput/v2/index.tsx

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,27 @@ class RangeInput extends Component<RangeInputProps, RangeInputState> {
184184
}
185185

186186
render() {
187-
const { formatValue, disabled, readOnly } = this.props
187+
const { formatValue, disabled, readOnly, messages } = this.props
188188

189-
const props = omitProps(this.props, RangeInput.allowedProps)
189+
const props = omitProps(this.props, RangeInput.allowedProps) as Record<
190+
string,
191+
unknown
192+
>
193+
194+
// Messages live inside the wrapping <label>. Reference them from the input
195+
// via `aria-describedby` and point the accessible name at the label text
196+
// only via `aria-labelledby`, so the messages are announced as a
197+
// description rather than as part of the control's name.
198+
const hasMessages = !!messages && messages.length > 0
199+
const messagesId = `${this.id}-messages`
200+
const labelId = `${this.id}-label`
190201

191202
return (
192203
<FormField
193204
{...pickProps(this.props, FormField.allowedProps)}
194205
label={this.props.label}
206+
messagesId={messagesId}
207+
labelId={labelId}
195208
id={this.id}
196209
elementRef={this.handleRef}
197210
data-cid="RangeInput"
@@ -211,6 +224,16 @@ class RangeInput extends Component<RangeInputProps, RangeInputState> {
211224
{...props}
212225
disabled={disabled || readOnly}
213226
aria-disabled={disabled || readOnly ? 'true' : undefined}
227+
aria-describedby={
228+
[props['aria-describedby'], hasMessages ? messagesId : null]
229+
.filter(Boolean)
230+
.join(' ') || undefined
231+
}
232+
aria-labelledby={
233+
hasMessages
234+
? (props['aria-labelledby'] as string) || labelId
235+
: (props['aria-labelledby'] as string | undefined)
236+
}
214237
/>
215238
{this.renderValue()}
216239
</div>

0 commit comments

Comments
 (0)