(WIP) Fix description text not read on hover by NVDA

packages/ui-checkbox/src/CheckboxGroup/__new-tests__/CheckboxGroup.test.tsx

@@ -88,8 +88,8 @@ describe('<CheckboxGroup />', () => {
     const { container } = renderCheckboxGroup({
       messages: [{ text: TEST_ERROR_MESSAGE, type: 'error' }]
     })
-    const fieldset = screen.getByRole('group')
-    const ariaDesc = fieldset.getAttribute('aria-describedby')
+    const group = screen.getByRole('group')
+    const ariaDesc = group.getAttribute('aria-describedby')
     const messageById = container.querySelector(`[id="${ariaDesc}"]`)
 
     expect(messageById).toBeInTheDocument()
@@ -98,7 +98,7 @@ describe('<CheckboxGroup />', () => {
 
   it('displays description message inside the legend', () => {
     const { container } = renderCheckboxGroup({ description: TEST_DESCRIPTION })
-    const legend = container.querySelector('legend')
+    const legend = container.querySelector(`[id^="FormField-Label_"]`)
 
     expect(legend).toBeInTheDocument()
     expect(legend).toHaveTextContent(TEST_DESCRIPTION)
@@ -217,5 +217,20 @@ describe('<CheckboxGroup />', () => {
 
       expect(axeCheck).toBe(true)
     })
+
+    it('adds the correct ARIA attributes', () => {
+      const { container } = renderCheckboxGroup({
+        disabled: true,
+        messages: [{ type: 'newError', text: 'abc' }],
+        // @ts-ignore This is a valid attribute
+        'data-id': 'group'
+      })
+      const group = container.querySelector(`[data-id="group"]`)
+      expect(group).toBeInTheDocument()
+      expect(group).toHaveAttribute('role', 'group')
+      expect(group).toHaveAttribute('aria-disabled', 'true')
+      // ARIA role 'group' cannot have the 'aria-invalid' attribute
+      expect(group).not.toHaveAttribute('aria-invalid')
+    })
   })
 })

packages/ui-checkbox/src/CheckboxGroup/props.ts

@@ -22,7 +22,7 @@
  * SOFTWARE.
  */
 
-import React from 'react'
+import React, { type InputHTMLAttributes } from 'react'
 import PropTypes from 'prop-types'
 
 import { FormPropTypes } from '@instructure/ui-form-field'
@@ -32,7 +32,10 @@ import {
 } from '@instructure/ui-prop-types'
 
 import type { FormMessage } from '@instructure/ui-form-field'
-import type { PropValidators } from '@instructure/shared-types'
+import type {
+  OtherHTMLAttributes,
+  PropValidators
+} from '@instructure/shared-types'
 import type { WithDeterministicIdProps } from '@instructure/ui-react-utils'
 
 import { Checkbox } from '../Checkbox'
@@ -58,7 +61,12 @@ type PropKeys = keyof CheckboxGroupOwnProps
 
 type AllowedPropKeys = Readonly<Array<PropKeys>>
 
-type CheckboxGroupProps = CheckboxGroupOwnProps & WithDeterministicIdProps
+type CheckboxGroupProps = CheckboxGroupOwnProps &
+  OtherHTMLAttributes<
+    CheckboxGroupOwnProps,
+    InputHTMLAttributes<CheckboxGroupOwnProps & Element>
+  > &
+  WithDeterministicIdProps
 
 const propTypes: PropValidators<PropKeys> = {
   name: PropTypes.string.isRequired,

packages/ui-form-field/src/FormField/index.tsx

@@ -67,8 +67,6 @@ class FormField extends Component<FormFieldProps> {
         {...pickProps(this.props, FormFieldLayout.allowedProps)}
         label={this.props.label}
         vAlign={this.props.vAlign}
-        as="label"
-        htmlFor={this.props.id}
         elementRef={this.handleRef}
       />
     )

packages/ui-form-field/src/FormFieldGroup/__new-tests__/FormFieldGroup.test.tsx

@@ -66,7 +66,7 @@ describe('<FormFieldGroup />', () => {
     )
 
     const formFieldGroup = container.querySelector(
-      "fieldset[class$='-formFieldLayout']"
+      "span[class$='-formFieldLabel']"
     )
     const firstNameInput = screen.getByLabelText('First:')
     const middleNameInput = screen.getByLabelText('Middle:')
@@ -94,14 +94,12 @@ describe('<FormFieldGroup />', () => {
       </FormFieldGroup>
     )
 
-    const formFieldGroup = container.querySelector(
-      "fieldset[class$='-formFieldLayout']"
-    )
+    const formFieldGroup = container.querySelector('label')
 
     expect(formFieldGroup).toBeInTheDocument()
   })
 
-  it('links the messages to the fieldset via aria-describedby', () => {
+  it('links the messages to the group via aria-describedby', () => {
     const messages: FormMessage[] = [{ text: 'Invalid name', type: 'error' }]
 
     const { container } = render(
@@ -122,10 +120,9 @@ describe('<FormFieldGroup />', () => {
     )
 
     const formFieldGroup = container.querySelector(
-      "fieldset[class$='-formFieldLayout']"
+      "span[class$='-formFieldLayout']"
     )
-    const message = container.querySelector("span[id^='FormFieldLayout_']")
-
+    const message = container.querySelector("span[class$='-formFieldMessages']")
     expect(message).toBeInTheDocument()
     expect(formFieldGroup).toBeInTheDocument()
     expect(formFieldGroup).toHaveAttribute('aria-describedby')
@@ -136,7 +133,7 @@ describe('<FormFieldGroup />', () => {
     expect(message).toHaveAttribute('id', messagesId)
   })
 
-  it('displays description message inside the legend', () => {
+  it('displays description message inside the label', () => {
     const description = 'Please enter your full name'
 
     const { container } = render(
@@ -153,9 +150,7 @@ describe('<FormFieldGroup />', () => {
       </FormFieldGroup>
     )
 
-    const legend = container.querySelector(
-      "legend[class$='-screenReaderContent']"
-    )
+    const legend = container.querySelector("span[class$='-formFieldLabel']")
 
     expect(legend).toBeInTheDocument()
     expect(legend).toHaveTextContent(description)

packages/ui-form-field/src/FormFieldGroup/index.tsx

@@ -23,7 +23,7 @@
  */
 
 /** @jsx jsx */
-import { Component, Children, ReactElement } from 'react'
+import { Component, Children, ReactElement, AriaAttributes } from 'react'
 
 import { Grid } from '@instructure/ui-grid'
 import { pickProps, omitProps } from '@instructure/ui-react-utils'
@@ -49,7 +49,7 @@ class FormFieldGroup extends Component<FormFieldGroupProps> {
   static propTypes = propTypes
   static allowedProps = allowedProps
   static defaultProps = {
-    as: 'fieldset',
+    as: 'span',
     disabled: false,
     rowSpacing: 'medium',
     colSpacing: 'small',
@@ -84,7 +84,7 @@ class FormFieldGroup extends Component<FormFieldGroupProps> {
     return (
       !!this.props.messages &&
       this.props.messages.findIndex((message) => {
-        return message.type === 'error'
+        return message.type === 'error' || message.type === 'newError'
       }) >= 0
     )
   }
@@ -134,7 +134,33 @@ class FormFieldGroup extends Component<FormFieldGroupProps> {
 
   render() {
     const { styles, makeStyles, isGroup, ...props } = this.props
-
+    const role = props.role ? props.role : 'group'
+    // This is quite ugly, but according to ARIA spec
+    // the `aria-invalid` prop can only be used with certain roles
+    // see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-invalid#associated_roles
+    let ariaInvalid: AriaAttributes['aria-invalid'] = undefined
+    if (
+      this.invalid &&
+      [
+        'application',
+        'checkbox',
+        'combobox',
+        'gridcell',
+        'listbox',
+        'radiogroup',
+        'slider',
+        'spinbutton',
+        'textbox',
+        'tree',
+        'columnheader',
+        'rowheader',
+        'searchbox',
+        'switch',
+        'treegrid'
+      ].includes(role)
+    ) {
+      ariaInvalid = 'true'
+    }
     return (
       <FormFieldLayout
         {...omitProps(props, FormFieldGroup.allowedProps)}
@@ -143,7 +169,8 @@ class FormFieldGroup extends Component<FormFieldGroupProps> {
         layout={props.layout === 'inline' ? 'inline' : 'stacked'}
         label={props.description}
         aria-disabled={props.disabled ? 'true' : undefined}
-        aria-invalid={this.invalid ? 'true' : undefined}
+        aria-invalid={ariaInvalid}
+        role={role}
         elementRef={this.handleRef}
         isGroup={isGroup}
       >

packages/ui-form-field/src/FormFieldLayout/__new-tests__/FormFieldLayout.test.tsx

@@ -23,7 +23,7 @@
  */
 
 import React from 'react'
-import { render, screen } from '@testing-library/react'
+import { render } from '@testing-library/react'
 import { vi } from 'vitest'
 import { runAxeCheck } from '@instructure/ui-axe-check'
 import '@testing-library/jest-dom'
@@ -74,15 +74,13 @@ describe('<FormFieldLayout />', () => {
 
   it('should provide a ref to the input container', () => {
     const inputContainerRef = vi.fn()
-
+    const ref = React.createRef<HTMLInputElement>()
     render(
       <FormFieldLayout label="Username" inputContainerRef={inputContainerRef}>
-        <input type="text" />
+        <input type="text" ref={ref} />
       </FormFieldLayout>
     )
-
-    const input = screen.getByLabelText('Username')
-
-    expect(inputContainerRef).toHaveBeenCalledWith(input.parentElement)
+    expect(ref.current).toBeInstanceOf(HTMLInputElement)
+    expect(inputContainerRef).toHaveBeenCalledWith(ref.current!.parentElement)
   })
 })

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

@@ -24,9 +24,7 @@
 
 /** @jsx jsx */
 import { Component } from 'react'
-
 import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
-import { ScreenReaderContent } from '@instructure/ui-a11y-content'
 import { Grid } from '@instructure/ui-grid'
 import {
   omitProps,
@@ -67,9 +65,11 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
   constructor(props: FormFieldLayoutProps) {
     super(props)
     this._messagesId = props.messagesId || props.deterministicId!()
+    this._labelId = props.deterministicId!('FormField-Label')
   }
 
   private _messagesId: string
+  private _labelId: string
 
   ref: Element | null = null
 
@@ -121,32 +121,13 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
           textAlign={this.props.labelAlign}
           width={this.inlineContainerAndLabel ? 'auto' : 3}
         >
-          <FormFieldLabel
-            aria-hidden={this.elementType === 'fieldset' ? 'true' : undefined}
-          >
-            {this.props.label}
-          </FormFieldLabel>
+          <FormFieldLabel id={this._labelId}>{this.props.label}</FormFieldLabel>
         </Grid.Col>
       )
-    } else if (this.elementType !== 'fieldset') {
-      // to avoid duplicate label/legend content
-      return this.props.label
-    } else {
-      return null
-    }
-  }
-
-  renderLegend() {
-    // note: the legend element must be the first child of a fieldset element for SR
-    // so we render it twice in that case (once for SR-only and one that is visible)
-    return (
-      <ScreenReaderContent as="legend">
-        {this.props.label}
-        {this.hasMessages && (
-          <FormFieldMessages messages={this.props.messages} />
-        )}
-      </ScreenReaderContent>
-    )
+    } else if (this.props.label) {
+      // needs to be wrapped because it needs an `id`
+      return <div id={this._labelId}>{this.props.label}</div>
+    } else return null
   }
 
   renderVisibleMessages() {
@@ -166,15 +147,25 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
   }
 
   render() {
-    // any cast is needed to prevent Expression produces a union type that is too complex to represent errors
-    const ElementType = this.elementType as any
+    const ElementType = this.elementType
 
     const { makeStyles, styles, messages, isGroup, ...props } = this.props
 
     const { width, layout, children } = props
 
     const hasNewErrorMsg =
       !!messages?.find((m) => m.type === 'newError') && isGroup
+
+    let labelConnector = {}
+    if (ElementType == 'label') {
+      // this is a `<label>`, it needs to be connected to a control
+      // This is the case for e.g. `FormField`, `TextInput`, `NumberInput`
+      labelConnector = { htmlFor: this.props.id }
+    } else if (this.props.label) {
+      // This is a FormFieldGroup (span), it needs to be connected to its label
+      // This is the case for `FormFieldGroup`
+      labelConnector = { 'aria-labelledby': this._labelId }
+    }
     return (
       <ElementType
         {...omitProps(props, [
@@ -184,9 +175,9 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
         css={styles?.formFieldLayout}
         style={{ width }}
         aria-describedby={this.hasMessages ? this._messagesId : undefined}
+        {...labelConnector}
         ref={this.handleRef}
       >
-        {this.elementType === 'fieldset' && this.renderLegend()}
         <Grid
           rowSpacing="small"
           colSpacing="small"

packages/ui-radio-input/src/RadioInputGroup/README.md

@@ -212,7 +212,7 @@ type: example
   description={
     <ScreenReaderContent>Select a fruit</ScreenReaderContent>
   }
-  messages={[{ text: 'Invalid choice', type: 'error' }]}
+  messages={[{ text: 'Invalid choice', type: 'newError' }]}
 >
   <RadioInput label="Apple" value="apple" />
   <RadioInput label="Orange" value="orange" />

packages/ui-radio-input/src/RadioInputGroup/__new-tests__/RadioInputGroup.test.tsx

@@ -228,4 +228,25 @@ describe('<RadioInputGroup />', () => {
       expect(axeCheck).toBe(true)
     })
   })
+
+  it('adds the correct ARIA attributes', () => {
+    const { container } = render(
+      <RadioInputGroup
+        name="fruit"
+        description="Select a fruit"
+        disabled={true}
+        data-id="group"
+        messages={[{ type: 'newError', text: 'abc' }]}
+      >
+        <RadioInput label="Apple" value="apple" />
+        <RadioInput label="Banana" value="banana" />
+        <RadioInput label="Orange" value="orange" />
+      </RadioInputGroup>
+    )
+    const group = container.querySelector(`[data-id="group"]`)
+    expect(group).toBeInTheDocument()
+    expect(group).toHaveAttribute('role', 'radiogroup')
+    expect(group).toHaveAttribute('aria-disabled', 'true')
+    expect(group).toHaveAttribute('aria-invalid', 'true')
+  })
 })