fix: CheckboxField to set a generated ID on the input, to match the l…

…abel's htmlFor (#25079)

* Have field _always_ set a generated ID on the input component, even if there is no field label.
* Refactor useField to not set input props to `undefined` if they aren't relevant (instead they will be unset in that case)
* Add tests for these changes.

change/@fluentui-react-field-16bb1d7a-fa7b-41d6-92ec-5e18e0669150.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix: CheckboxField to set a generated ID on the input, to match the label's htmlFor",
+  "packageName": "@fluentui/react-field",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -24,7 +24,18 @@ describe('CheckboxField', () => {
 
   // Most functionality is tested by Field.test.tsx, and Checkbox's tests
 
-  it('sets htmlFor of both label and fieldLabel', () => {
+  it('sets htmlFor on label', () => {
+    const result = render(<CheckboxField label="checkbox label" />);
+
+    const checkbox = result.getByRole('checkbox');
+    const checkboxLabel = result.getByText('checkbox label') as HTMLLabelElement;
+
+    expect(checkbox.id).toBeTruthy();
+    expect(checkboxLabel.htmlFor).toBe(checkbox.id);
+    expect(checkbox.getAttribute('aria-labelledby')).toBeFalsy();
+  });
+
+  it('sets htmlFor on both label and fieldLabel', () => {
     const result = render(<CheckboxField label="checkbox label" fieldLabel="field label" />);
 
     const checkbox = result.getByRole('checkbox');

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

@@ -102,4 +102,26 @@ describe('Field', () => {
 
     expect(input.getAttribute('aria-invalid')).toBeTruthy();
   });
+
+  it('does not override user aria props', () => {
+    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}
+      />,
+    );
+
+    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');
+  });
 });

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

@@ -16,12 +16,6 @@ const validationMessageIcons = {
   success: <CheckmarkCircle12Filled />,
 } as const;
 
-/**
- * Merge two possibly-undefined IDs for aria-describedby. If both IDs are defined, combines
- * them into a string separated by a space. Otherwise, returns just the defined ID (if any).
- */
-const mergeAriaDescribedBy = (a?: string, b?: string) => (a && b ? `${a} ${b}` : a || b);
-
 /**
  * Partition the props used by the Field itself, from the props that are passed to the underlying field component.
  */
@@ -72,22 +66,31 @@ export const useField_unstable = <T extends FieldComponent>(
   params: FieldConfig<T>,
 ): FieldState<T> => {
   const [fieldProps, controlProps] = getPartitionedFieldProps(props);
+  const { orientation = 'vertical', validationState } = fieldProps;
+  const { labelConnection = 'htmlFor' } = params;
 
   const baseId = useId('field-');
 
-  const { orientation = 'vertical', validationState } = fieldProps;
-
   const root = resolveShorthand(fieldProps.root, {
     required: true,
     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 is set below
+      htmlFor: labelConnection === 'htmlFor' ? control.id : undefined,
     },
   });
 
@@ -110,26 +113,25 @@ export const useField_unstable = <T extends FieldComponent>(
     },
   });
 
-  const { labelConnection = 'htmlFor' } = params;
-  const hasError = validationState === 'error';
-
-  const control = resolveShorthand(fieldProps.control, {
-    required: true,
-    defaultProps: {
-      ref,
-      // Add a default ID only if required for label's htmlFor prop
-      id: label && labelConnection === 'htmlFor' ? baseId + '__control' : undefined,
-      // Add aria-labelledby only if not using the label's htmlFor
-      'aria-labelledby': labelConnection !== 'htmlFor' ? label?.id : undefined,
-      'aria-describedby': hasError ? hint?.id : mergeAriaDescribedBy(validationMessage?.id, hint?.id),
-      'aria-errormessage': hasError ? validationMessage?.id : undefined,
-      'aria-invalid': hasError ? true : undefined,
-      ...controlProps,
-    },
-  });
+  // Hook up aria props on the control
+  if (label && labelConnection === 'aria-labelledby') {
+    control['aria-labelledby'] ??= label.id;
+  }
 
-  if (labelConnection === 'htmlFor' && label && !label.htmlFor) {
-    label.htmlFor = control.id;
+  if (validationState === 'error') {
+    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;
+    }
   }
 
   const state: FieldState<FieldComponent> = {