Skip to content

Commit ccc9516

Browse files
git-nandorclaude
andcommitted
fix(many): only reference rendered messages via aria-describedby
Address PR review feedback on the form-field messages work: - The aria-describedby wiring referenced a messages element even when the message had no text. FormField (via FormFieldLayout) only renders messages that have text, so this left a dangling reference — e.g. DateTimeInput passes an empty-text error message to its sub-inputs just to force the invalid styling. Gate the wiring on messages that actually render (`messages.some(m => !!m.text)`). Checkbox is unchanged here: it renders messages via FormFieldMessages on `length > 0`, so its reference always resolves. - Drop the now-unnecessary type casts around the aria-* reads in Checkbox, TextArea, NumberInput and RangeInput. - TextInput: expand the comment to explain why messages must be kept out of the name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 409d077 commit ccc9516

5 files changed

Lines changed: 38 additions & 35 deletions

File tree

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -349,16 +349,12 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
349349
// Keep messages in the description so the accessible name contains only the label.
350350
aria-labelledby={
351351
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)
352+
? props['aria-labelledby'] || this._labelId
353+
: props['aria-labelledby']
358354
}
359355
aria-describedby={
360356
[
361-
(props as Record<string, unknown>)['aria-describedby'],
357+
props['aria-describedby'],
362358
this.hasMessages ? this._messagesId : null
363359
]
364360
.filter(Boolean)

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
124124
// via `aria-describedby` and point the accessible name at the label text
125125
// only via `aria-labelledby`, so the messages are announced as a
126126
// description rather than as part of the control's name.
127-
const hasMessages = !!messages && messages.length > 0
127+
// Only when a message actually renders (has text); FormField skips
128+
// empty-text messages, so otherwise aria-describedby would dangle.
129+
const hasMessages = !!messages?.some((m) => !!m.text)
128130
const messagesId = id ? `${id}-messages` : undefined
129131
const labelId = id ? `${id}-label` : undefined
130132

@@ -328,7 +330,9 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
328330

329331
const label = callRenderProp(renderLabel)
330332

331-
const passedProps = passthroughProps(rest)
333+
// `passthroughProps` is typed with `unknown` values; widen once here so the
334+
// aria-* reads below don't each need a cast.
335+
const passedProps = passthroughProps(rest) as Record<string, any>
332336

333337
// Don't render until we have an ID
334338
if (!id) {
@@ -366,8 +370,8 @@ const NumberInput = forwardRef<NumberInputHandle, NumberInputProps>(
366370
}
367371
aria-labelledby={
368372
hasMessages
369-
? (passedProps['aria-labelledby'] as string) || labelId
370-
: (passedProps['aria-labelledby'] as string | undefined)
373+
? passedProps['aria-labelledby'] || labelId
374+
: passedProps['aria-labelledby']
371375
}
372376
id={id}
373377
type={allowStringValue ? 'text' : 'number'}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,15 @@ class RangeInput extends Component<RangeInputProps, RangeInputState> {
186186
render() {
187187
const { formatValue, disabled, readOnly, messages } = this.props
188188

189-
const props = omitProps(this.props, RangeInput.allowedProps) as Record<
190-
string,
191-
unknown
192-
>
189+
const props = omitProps(this.props, RangeInput.allowedProps)
193190

194191
// Messages live inside the wrapping <label>. Reference them from the input
195192
// via `aria-describedby` and point the accessible name at the label text
196193
// only via `aria-labelledby`, so the messages are announced as a
197194
// description rather than as part of the control's name.
198-
const hasMessages = !!messages && messages.length > 0
195+
// Only when a message actually renders (has text); FormField skips
196+
// empty-text messages, so otherwise aria-describedby would dangle.
197+
const hasMessages = !!messages?.some((m) => !!m.text)
199198
const messagesId = `${this.id}-messages`
200199
const labelId = `${this.id}-label`
201200

@@ -231,8 +230,8 @@ class RangeInput extends Component<RangeInputProps, RangeInputState> {
231230
}
232231
aria-labelledby={
233232
hasMessages
234-
? (props['aria-labelledby'] as string) || labelId
235-
: (props['aria-labelledby'] as string | undefined)
233+
? props['aria-labelledby'] || labelId
234+
: props['aria-labelledby']
236235
}
237236
/>
238237
{this.renderValue()}

packages/ui-text-area/src/TextArea/v2/index.tsx

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ const TextArea = forwardRef<TextAreaElement, TextAreaProps>((props, ref) => {
132132
// via `aria-describedby` and point the accessible name at the label text only
133133
// via `aria-labelledby`, so the messages are announced as a description
134134
// rather than as part of the control's name.
135-
const hasMessages = !!messages && messages.length > 0
135+
// Only when a message actually renders (has text); FormField skips empty-text
136+
// messages, so otherwise aria-describedby would dangle.
137+
const hasMessages = !!messages?.some((m) => !!m.text)
136138
const messagesId = `${id}-messages`
137139
const labelId = `${id}-label`
138140

@@ -402,20 +404,14 @@ const TextArea = forwardRef<TextAreaElement, TextAreaProps>((props, ref) => {
402404
aria-required={required}
403405
aria-invalid={isInvalid ? 'true' : undefined}
404406
aria-describedby={
405-
[
406-
(rest as Record<string, unknown>)['aria-describedby'],
407-
hasMessages ? messagesId : null
408-
]
407+
[rest['aria-describedby'], hasMessages ? messagesId : null]
409408
.filter(Boolean)
410409
.join(' ') || undefined
411410
}
412411
aria-labelledby={
413412
hasMessages
414-
? ((rest as Record<string, unknown>)['aria-labelledby'] as string) ||
415-
labelId
416-
: ((rest as Record<string, unknown>)['aria-labelledby'] as
417-
| string
418-
| undefined)
413+
? rest['aria-labelledby'] || labelId
414+
: rest['aria-labelledby']
419415
}
420416
disabled={disabled}
421417
readOnly={readOnly}

packages/ui-text-input/src/TextInput/v2/index.tsx

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,12 @@ class TextInput extends Component<TextInputProps> {
160160
}
161161

162162
get hasMessages() {
163-
return !!this.props.messages && this.props.messages.length > 0
163+
// FormField only renders messages that have text, so only treat the field
164+
// as having messages then. Otherwise the input's `aria-describedby` would
165+
// reference a messages element that was never rendered — e.g. DateTimeInput
166+
// passes an empty-text error message to its sub-inputs just to force the
167+
// invalid styling.
168+
return !!this.props.messages?.some((m) => !!m.text)
164169
}
165170

166171
get invalid() {
@@ -233,12 +238,15 @@ class TextInput extends Component<TextInputProps> {
233238
if (props['aria-describedby']) {
234239
descriptionIds = `${props['aria-describedby']}`
235240
}
236-
// When there are messages, associate them with the input as its description
237-
// (they are rendered by FormField with `id={this._messagesId}`) and point
238-
// the accessible name at the label text only via `aria-labelledby`. This
239-
// keeps the messages — which live inside the wrapping <label> — out of the
240-
// control's name while preserving the click-on-label focus behavior.
241-
// Any consumer-provided `aria-labelledby` takes precedence.
241+
// FormField renders this control and its `messages` inside a single wrapping
242+
// <label>, so by default the messages' text becomes part of the control's
243+
// accessible *name* (e.g. "Password Password must be at least 6 characters"),
244+
// which is confusing and repeats on every announcement. Messages should be
245+
// the field's *description* instead. So when there are messages, reference
246+
// them via `aria-describedby` and pin the name to the label text only via
247+
// `aria-labelledby`. Keeping the messages inside the <label> preserves the
248+
// native click-on-label focus behavior. A consumer-provided `aria-labelledby`
249+
// takes precedence.
242250
let labelledById = props['aria-labelledby'] as string | undefined
243251
if (this.hasMessages) {
244252
descriptionIds = [descriptionIds, this._messagesId]

0 commit comments

Comments
 (0)