diff --git a/packages/react-core/src/components/Form/FormGroup.tsx b/packages/react-core/src/components/Form/FormGroup.tsx index 203cd32f878..77139da4011 100644 --- a/packages/react-core/src/components/Form/FormGroup.tsx +++ b/packages/react-core/src/components/Form/FormGroup.tsx @@ -41,6 +41,10 @@ export interface FormGroupProps extends Omit, 'l helperTextInvalidIcon?: React.ReactNode; /** ID of the included field. It has to be the same for proper working. */ fieldId: string; + /** Sets the role of the form group. Pass in "radiogroup" when the form group contains multiple + * radio inputs, or pass in "group" when the form group contains multiple of any other input type. + */ + role?: string; } export const FormGroup: React.FunctionComponent = ({ @@ -60,6 +64,7 @@ export const FormGroup: React.FunctionComponent = ({ helperTextIcon, helperTextInvalidIcon, fieldId, + role, ...props }: FormGroupProps) => { const validHelperText = @@ -96,9 +101,12 @@ export const FormGroup: React.FunctionComponent = ({ const helperTextToDisplay = validated === ValidatedOptions.error && helperTextInvalid ? inValidHelperText : showValidHelperTxt(validated); + const isGroupOrRadioGroup = role === 'group' || role === 'radiogroup'; + const LabelComponent = isGroupOrRadioGroup ? 'span' : 'label'; + const labelContent = ( - {' '} + {' '} {React.isValidElement(labelIcon) && labelIcon} ); return ( -
+
{label && (
= ({ labelInfo && styles.modifiers.info, hasNoPaddingTop && styles.modifiers.noPaddingTop )} + {...(isGroupOrRadioGroup && { id: `${fieldId}-legend` })} > {labelInfo && ( diff --git a/packages/react-core/src/components/Form/FormSection.tsx b/packages/react-core/src/components/Form/FormSection.tsx index 4831d9d811a..f71b5451937 100644 --- a/packages/react-core/src/components/Form/FormSection.tsx +++ b/packages/react-core/src/components/Form/FormSection.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import styles from '@patternfly/react-styles/css/components/Form/form'; import { css } from '@patternfly/react-styles'; +import { GenerateId } from '../../helpers/GenerateId/GenerateId'; export interface FormSectionProps extends Omit, 'title'> { /** Content rendered inside the section */ @@ -20,9 +21,22 @@ export const FormSection: React.FunctionComponent = ({ titleElement: TitleElement = 'div', ...props }: FormSectionProps) => ( -
- {title && {title}} - {children} -
+ + {sectionId => ( +
+ {title && ( + + {title} + + )} + {children} +
+ )} +
); FormSection.displayName = 'FormSection'; diff --git a/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx b/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx index f16c5818bbe..3c5c135bec5 100644 --- a/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx +++ b/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx @@ -42,6 +42,8 @@ export const InternalFormFieldGroup: React.FunctionComponent {isExpandable && ( diff --git a/packages/react-core/src/components/Form/__tests__/FormFieldGroup.test.tsx b/packages/react-core/src/components/Form/__tests__/FormFieldGroup.test.tsx index c9cfa8706f7..f6a9f119fe6 100644 --- a/packages/react-core/src/components/Form/__tests__/FormFieldGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormFieldGroup.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import { FormFieldGroup } from '../FormFieldGroup'; import { FormFieldGroupExpandable } from '../FormFieldGroupExpandable'; @@ -53,3 +53,25 @@ test('Verify console error logged when there is no aria-label or title', () => { const { asFragment } = render(FieldGroup); expect(consoleErrorMock).toHaveBeenCalled(); }); + +test('Verify field group has accessible name when header is passed in', () => { + render( + } + /> + } + /> + ); + + expect(screen.getByRole('group')).toHaveAccessibleName('Field group 4 (non-expandable)'); +}); + +test('Verify field group does not have accessible name when header is not passed in', () => { + render(); + + expect(screen.getByRole('group')).not.toHaveAccessibleName(); +}); diff --git a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx index 05a2001bce0..edc77aaab83 100644 --- a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { ValidatedOptions } from '../../../helpers/constants'; import { FormGroup } from '../FormGroup'; @@ -203,4 +204,92 @@ describe('FormGroup', () => { ); expect(asFragment()).toMatchSnapshot(); }); + + test('should render without group or radiogroup role when role is not passed in', () => { + render( + + + + ); + + expect(screen.queryByRole('group')).not.toBeInTheDocument(); + expect(screen.queryByRole('radiogroup')).not.toBeInTheDocument(); + }); + + test('input should receive focus when clicking text label', () => { + render( + + + + ); + + const labelElement = screen.getByText('label'); + const input = screen.getByRole('textbox'); + + userEvent.click(labelElement); + expect(input).toHaveFocus(); + }); + + describe('With multiple inputs per group', () => { + test('inputs should not receive focus when clicking text label', () => { + render( + + + + + ); + + const labelElement = screen.getByText('label'); + const inputs = screen.getAllByRole('textbox'); + + userEvent.click(labelElement); + inputs.forEach(input => { + expect(input).not.toHaveFocus(); + }); + }); + + test('should render with group role', () => { + render( + + + + + ); + + expect(screen.getByRole('group')).toBeInTheDocument(); + }); + + test('should render with radiogroup role', () => { + render( + + + + + ); + + expect(screen.getByRole('radiogroup')).toBeInTheDocument(); + }); + + test('should have accessible name when group role is passed in', () => { + render( + + + + + ); + + expect(screen.getByRole('group')).toHaveAccessibleName('label'); + }); + + test('should have accessible name when radiogroup role is passed in', () => { + render( + + + + + ); + + expect(screen.getByRole('radiogroup')).toHaveAccessibleName('label'); + }); + }); }); diff --git a/packages/react-core/src/components/Form/__tests__/FormSection.test.tsx b/packages/react-core/src/components/Form/__tests__/FormSection.test.tsx index d8844b5c665..30a54e7fe00 100644 --- a/packages/react-core/src/components/Form/__tests__/FormSection.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormSection.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { FormSection } from '../FormSection'; -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; test('Check form section example against snapshot', () => { const Section = ; @@ -13,3 +13,15 @@ test('Check form section example with title', () => { const { asFragment } = render(Section); expect(asFragment()).toMatchSnapshot(); }); + +test('Verify form section has accessible name when title is passed in', () => { + render(); + + expect(screen.getByRole('group')).toHaveAccessibleName('Form title'); +}); + +test('Verify form section does not have accessible name when title is not passed in', () => { + render(); + + expect(screen.getByRole('group')).not.toHaveAccessibleName(); +}); diff --git a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snap b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snap index 0a01a963490..d33bf2a881c 100644 --- a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snap +++ b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snap @@ -3,7 +3,9 @@ exports[`Check expandable form field group example against snapshot 1`] = `
`; @@ -11,10 +12,13 @@ exports[`Check form section example against snapshot 1`] = ` exports[`Check form section example with title 1`] = `

Title

diff --git a/packages/react-core/src/components/Form/examples/Form.md b/packages/react-core/src/components/Form/examples/Form.md index 67aabbc0153..3f70a41d01d 100644 --- a/packages/react-core/src/components/Form/examples/Form.md +++ b/packages/react-core/src/components/Form/examples/Form.md @@ -44,7 +44,7 @@ import TrashIcon from '@patternfly/react-icons/dist/esm/icons/trash-icon'; ```js import React from 'react'; -import { Form, FormGroup, TextInput, Checkbox, Popover, ActionGroup, Button } from '@patternfly/react-core'; +import { Form, FormGroup, TextInput, Checkbox, Popover, ActionGroup, Button, Radio } from '@patternfly/react-core'; import HelpIcon from '@patternfly/react-icons/dist/esm/icons/help-icon'; class SimpleForm extends React.Component { @@ -147,10 +147,15 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + + + + + + @@ -186,7 +191,8 @@ import { FormSelectOption, Checkbox, ActionGroup, - Button + Button, + Radio } from '@patternfly/react-core'; class HorizontalForm extends React.Component { @@ -226,7 +232,12 @@ class HorizontalForm extends React.Component { return (
- + - + - + + + + + + @@ -287,7 +309,7 @@ class HorizontalForm extends React.Component { ```js import React from 'react'; -import { Form, FormGroup, TextInput, Checkbox, Popover, ActionGroup, Button } from '@patternfly/react-core'; +import { Form, FormGroup, TextInput, Checkbox, Popover, ActionGroup, Button, Radio } from '@patternfly/react-core'; import HelpIcon from '@patternfly/react-icons/dist/esm/icons/help-icon'; class SimpleForm extends React.Component { @@ -390,10 +412,15 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + + + + + + @@ -605,7 +632,7 @@ class HorizontalForm extends React.Component { render() { return ( - + @@ -639,6 +666,7 @@ class HorizontalFormHelperTextOnTop extends React.Component { hasNoPaddingTop fieldId="options" isStack + role="group" > @@ -823,7 +851,12 @@ class SimpleForm extends React.Component { - +