fix: Field should use aria-describedby instead of aria-errormessage (#…

…25580)

Field currently sets the validationMessage as its `aria-errormessage` when it is an error. However, that is not well supported by screen readers.

fix: Field sets `aria-describedby` to the validationMessage regardless of the validationState. It no longer sets `aria-errormessage`.

change/@fluentui-react-field-bd3c7086-9be2-4721-9b99-31be563f45a3.json

@@ -0,0 +1,7 @@
+{
+ "type": "prerelease",
+ "comment": "fix: Field should use aria-describedby instead of aria-errormessage",
+ "packageName": "@fluentui/react-field",
+ "email": "behowell@microsoft.com",
+ "dependentChangeType": "patch"
+}

packages/react-components/react-field/etc/react-field.api.md

@@ -24,7 +24,7 @@ export type FieldConfig<T extends FieldControl> = {
 };
 
 // @public
-export type FieldControl = React_2.VoidFunctionComponent<Pick<React_2.HTMLAttributes<HTMLElement>, 'id' | 'className' | 'style' | 'aria-labelledby' | 'aria-describedby' | 'aria-invalid' | 'aria-errormessage'>>;
+export type FieldControl = React_2.VoidFunctionComponent<Pick<React_2.HTMLAttributes<HTMLElement>, 'id' | 'className' | 'style' | 'aria-labelledby' | 'aria-describedby' | 'aria-invalid'>>;
 
 // @public
 export type FieldProps<T extends FieldControl> = ComponentProps<Partial<FieldSlots<T>>, 'control'> & {

packages/react-components/react-field/src/components/Field/Field.test.tsx

@@ -67,7 +67,7 @@ describe('Field', () => {
  expect(input.getAttribute('aria-describedby')).toBe(hint.id);
  });
 
- it('sets aria-describedby to the validationMessage if not an error', () => {
+ it('sets aria-describedby to the validationMessage', () => {
  const result = render(<MockField validationMessage="Test validation message" validationState="warning" />);
  const input = result.getByRole('textbox');
  const validationMessage = result.getByText('Test validation message');
@@ -76,9 +76,9 @@ describe('Field', () => {
  expect(input.getAttribute('aria-describedby')).toBe(validationMessage.id);
  });
 
- it('sets aria-describedby to the hint + validationMessage if not an error', () => {
+ it('sets aria-describedby to the validationMessage + hint', () => {
  const result = render(
- <MockField hint="Test hint" validationMessage="Test validation message" validationState="success" />,
+ <MockField hint="Test hint" validationMessage="Test validation message" validationState="error" />,
  );
  const input = result.getByRole('textbox');
  const hint = result.getByText('Test hint');
@@ -87,13 +87,20 @@ describe('Field', () => {
  expect(input.getAttribute('aria-describedby')).toBe(validationMessage.id + ' ' + hint.id);
  });
 
- it('sets aria-errormessage to the validationMessage if an error', () => {
- const result = render(<MockField validationMessage="Test validation message" validationState="error" />);
+ it('sets aria-describedby to the validationMessage + hint + user aria-describedby', () => {
+ const result = render(
+ <MockField
+ hint="Test hint"
+ validationMessage="Test validation message"
+ validationState="error"
+ aria-describedby="test-describedby"
+ />,
+ );
  const input = result.getByRole('textbox');
+ const hint = result.getByText('Test hint');
  const validationMessage = result.getByText('Test validation message');
 
- expect(validationMessage.id).toBeTruthy();
- expect(input.getAttribute('aria-errormessage')).toBe(validationMessage.id);
+ expect(input.getAttribute('aria-describedby')).toBe(validationMessage.id + ' ' + hint.id + ' test-describedby');
  });
 
  it('sets aria-invalid if an error', () => {
@@ -103,15 +110,14 @@ describe('Field', () => {
  expect(input.getAttribute('aria-invalid')).toBeTruthy();
  });
 
- it('does not override user aria props', () => {
+ it('does not override user aria props, EXCEPT aria-describedby', () => {
  const result = render(
  <MockField
  label="test label"
  validationState="error"
  validationMessage="test description"
  hint="test hint"
  aria-labelledby="test-labelledby"
- aria-describedby="test-describedby"
  aria-errormessage="test-errormessage"
  aria-invalid={false}
  />,
@@ -120,8 +126,23 @@ describe('Field', () => {
  const input = result.getByRole('textbox');
 
  expect(input.getAttribute('aria-labelledby')).toBe('test-labelledby');
- expect(input.getAttribute('aria-describedby')).toBe('test-describedby');
  expect(input.getAttribute('aria-errormessage')).toBe('test-errormessage');
  expect(input.getAttribute('aria-invalid')).toBe('false');
  });
+
+ it('does not override user aria-describedby on the control slot', () => {
+ const result = render(
+ <MockField
+ label="test label"
+ validationState="error"
+ validationMessage="test description"
+ hint="test hint"
+ control={{ 'aria-describedby': 'test-describedby' }}
+ />,
+ );
+
+ const input = result.getByRole('textbox');
+
+ expect(input.getAttribute('aria-describedby')).toBe('test-describedby');
+ });
 });

packages/react-components/react-field/src/components/Field/Field.types.ts

@@ -12,7 +12,7 @@ import type { SlotComponent } from './SlotComponent.types';
 export type FieldControl = React.VoidFunctionComponent<
  Pick<
  React.HTMLAttributes<HTMLElement>,
- 'id' | 'className' | 'style' | 'aria-labelledby' | 'aria-describedby' | 'aria-invalid' | 'aria-errormessage'
+ 'id' | 'className' | 'style' | 'aria-labelledby' | 'aria-describedby' | 'aria-invalid'
  >
 >;
 
@@ -80,6 +80,11 @@ export type FieldProps<T extends FieldControl> = ComponentProps<Partial<FieldSlo
  * but doesn't add them to the public API of fields that don't support them.
  */
 export type FieldPropsWithOptionalComponentProps<T extends FieldControl> = FieldProps<T> & {
+ /**
+ * A ref to the underlying control.
+ */
+ ref?: React.Ref<HTMLElement>;
+
  /**
  * Whether the field label should be marked as required.
  */
@@ -119,7 +124,7 @@ export type FieldConfig<T extends FieldControl> = {
  labelConnection?: 'htmlFor' | 'aria-labelledby';
 
  /**
- * Should the aria-invalid and aria-errormessage attributes be set when validationState="error".
+ * Should the aria-invalid attribute be set when validationState="error".
  *
  * @default true
  */

packages/react-components/react-field/src/components/Field/useField.tsx

@@ -2,13 +2,7 @@ import * as React from 'react';
 import { CheckmarkCircle12Filled, ErrorCircle12Filled, Warning12Filled } from '@fluentui/react-icons';
 import { Label } from '@fluentui/react-label';
 import { getNativeElementProps, resolveShorthand, useId } from '@fluentui/react-utilities';
-import type {
- FieldConfig,
- FieldControl,
- FieldProps,
- FieldPropsWithOptionalComponentProps,
- FieldState,
-} from './Field.types';
+import type { FieldConfig, FieldControl, FieldPropsWithOptionalComponentProps, FieldState } from './Field.types';
 
 const validationMessageIcons = {
  error: <ErrorCircle12Filled />,
@@ -19,7 +13,7 @@ const validationMessageIcons = {
 /**
  * Partition the props used by the Field itself, from the props that are passed to the underlying field component.
  */
-export const getPartitionedFieldProps = <Props extends FieldProps<FieldControl>>(props: Props) => {
+export const getPartitionedFieldProps = (props: FieldPropsWithOptionalComponentProps<FieldControl>) => {
  const {
  className,
  control,
@@ -76,21 +70,12 @@ export const useField_unstable = <T extends FieldControl>(
  defaultProps: getNativeElementProps('div', fieldProps),
  });
 
- const control = resolveShorthand(fieldProps.control, {
- required: true,
- defaultProps: {
- ref,
- id: baseId + '__control',
- ...controlProps,
- },
- });
-
  const label = resolveShorthand(fieldProps.label, {
  defaultProps: {
  id: baseId + '__label',
  required: controlProps.required,
  size: typeof controlProps.size === 'string' ? controlProps.size : undefined,
- htmlFor: labelConnection === 'htmlFor' ? control.id : undefined,
+ // htmlFor is handled below
  },
  });
 
@@ -115,23 +100,33 @@ export const useField_unstable = <T extends FieldControl>(
 
  // Hook up aria props on the control
  if (label && labelConnection === 'aria-labelledby') {
- control['aria-labelledby'] ??= label.id;
+ controlProps['aria-labelledby'] ??= label.id;
+ }
+
+ if (validationMessage || hint) {
+ // The control is described by the validation message, or hint, or both
+ // We also preserve and append any aria-describedby supplied by the user
+ // For reference: https://github.com/microsoft/fluentui/pull/25580#discussion_r1017259933
+ controlProps['aria-describedby'] = [validationMessage?.id, hint?.id, controlProps['aria-describedby']]
+ .filter(Boolean)
+ .join(' ');
  }
 
  if (validationState === 'error' && ariaInvalidOnError) {
- control['aria-invalid'] ??= true;
- if (validationMessage) {
- control['aria-errormessage'] ??= validationMessage.id;
- }
- if (hint) {
- control['aria-describedby'] ??= hint.id;
- }
- } else {
- // If the state is not an error, then the control is described by the validation message, or hint, or both
- const describedby = validationMessage || hint;
- if (describedby) {
- control['aria-describedby'] ??= validationMessage && hint ? `${validationMessage.id} ${hint.id}` : describedby.id;
- }
+ controlProps['aria-invalid'] ??= true;
+ }
+
+ const control = resolveShorthand(fieldProps.control, {
+ required: true,
+ defaultProps: {
+ ref,
+ id: baseId + '__control',
+ ...controlProps,
+ },
+ });
+
+ if (label && labelConnection === 'htmlFor') {
+ label.htmlFor ??= control.id;
  }
 
  const state: FieldState<FieldControl> = {

packages/react-components/react-progress/src/components/ProgressField/ProgressField.test.tsx

@@ -36,7 +36,7 @@ describe('ProgressField', () => {
  expect(label.htmlFor).toBeFalsy();
  });
 
- it('uses aria-describedby on error, instead of aria-errormessage ', () => {
+ it('does not set aria-invalid on error ', () => {
  const result = render(<ProgressField label="Test label" validationState="error" validationMessage="Test error" />);
 
  const progress = result.getByRole('progressbar');