From bd53a805606a34c6fae8d1332c3e9b839627b1c1 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Mon, 6 Jun 2022 13:50:58 -0400 Subject: [PATCH 1/9] feat(Form): add functionality for form group roles --- .../src/components/Form/FormGroup.tsx | 25 +++++++- .../src/components/Form/examples/Form.md | 63 ++++++++++++++++--- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/packages/react-core/src/components/Form/FormGroup.tsx b/packages/react-core/src/components/Form/FormGroup.tsx index 203cd32f878..72a7798e65d 100644 --- a/packages/react-core/src/components/Form/FormGroup.tsx +++ b/packages/react-core/src/components/Form/FormGroup.tsx @@ -4,6 +4,12 @@ import { ASTERISK } from '../../helpers/htmlConstants'; import { css } from '@patternfly/react-styles'; import { ValidatedOptions } from '../../helpers/constants'; +export interface MultipleInputsObject { + /** Applies a role of "radiogroup" to the form group when set to "true", or + * a role of "group" when set to "false"/not passed in. + */ + isRadioGroup?: boolean; +} export interface FormGroupProps extends Omit, 'label'> { /** Anything that can be rendered as FormGroup content. */ children?: React.ReactNode; @@ -41,6 +47,9 @@ 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; + /** Object for identifying whether the form group has multiple inputs for a single label, and + * whether the multiple inputs are radio inputs or not. */ + multipleInputs?: MultipleInputsObject; } export const FormGroup: React.FunctionComponent = ({ @@ -60,6 +69,7 @@ export const FormGroup: React.FunctionComponent = ({ helperTextIcon, helperTextInvalidIcon, fieldId, + multipleInputs, ...props }: FormGroupProps) => { const validHelperText = @@ -96,9 +106,11 @@ export const FormGroup: React.FunctionComponent = ({ const helperTextToDisplay = validated === ValidatedOptions.error && helperTextInvalid ? inValidHelperText : showValidHelperTxt(validated); + const LabelComponent = multipleInputs ? 'span' : 'label'; + const labelContent = ( - {' '} + {' '} {React.isValidElement(labelIcon) && labelIcon} ); + const groupType = multipleInputs?.isRadioGroup ? 'radiogroup' : 'group'; + return ( -
+
{label && (
= ({ labelInfo && styles.modifiers.info, hasNoPaddingTop && styles.modifiers.noPaddingTop )} + {...(multipleInputs && { id: `${fieldId}-legend` })} > {labelInfo && ( diff --git a/packages/react-core/src/components/Form/examples/Form.md b/packages/react-core/src/components/Form/examples/Form.md index 67aabbc0153..8aa65ecc47e 100644 --- a/packages/react-core/src/components/Form/examples/Form.md +++ b/packages/react-core/src/components/Form/examples/Form.md @@ -7,6 +7,7 @@ propComponents: 'ActionGroup', 'Form', 'FormGroup', + 'MultipleInputsObject', 'FormSection', 'FormHelperText', 'FormFieldGroup', @@ -44,7 +45,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,11 +148,22 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + + + + + + @@ -186,7 +198,8 @@ import { FormSelectOption, Checkbox, ActionGroup, - Button + Button, + Radio } from '@patternfly/react-core'; class HorizontalForm extends React.Component { @@ -226,7 +239,12 @@ class HorizontalForm extends React.Component { return (
- + - + + + + + + @@ -287,7 +316,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,11 +419,21 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + + + + + + @@ -605,7 +644,7 @@ class HorizontalForm extends React.Component { render() { return ( - + @@ -639,6 +678,7 @@ class HorizontalFormHelperTextOnTop extends React.Component { hasNoPaddingTop fieldId="options" isStack + multipleInputs > @@ -823,7 +863,12 @@ class SimpleForm extends React.Component { - + Date: Mon, 6 Jun 2022 14:22:56 -0400 Subject: [PATCH 2/9] Add tests for new multipleInputs prop --- .../Form/__tests__/FormGroup.test.tsx | 30 +++++++++ .../__snapshots__/FormGroup.test.tsx.snap | 66 +++++++++++++++++++ 2 files changed, 96 insertions(+) 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..0d57c8fc53d 100644 --- a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx @@ -203,4 +203,34 @@ describe('FormGroup', () => { ); expect(asFragment()).toMatchSnapshot(); }); + + test('should render with group role', () => { + const { asFragment } = render( + + + + + ); + expect(asFragment()).toMatchSnapshot(); + }); + + test('should render with radiogroup role', () => { + const { asFragment } = render( + + + + + ); + expect(asFragment()).toMatchSnapshot(); + }); }); diff --git a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap index a1cf9815318..7f193d39259 100644 --- a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap +++ b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap @@ -478,3 +478,69 @@ exports[`FormGroup should render stacked horizontal form group variant 1`] = ` `; + +exports[`FormGroup should render with group role 1`] = ` + +
+
+ + + label + + + +
+
+ + +
+
+ +`; + +exports[`FormGroup should render with radiogroup role 1`] = ` + +
+
+ + + label + + + +
+
+ + +
+
+ +`; From 6c1eda812588fff3b24cef68fb656db89efe5e61 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 7 Jun 2022 09:24:18 -0400 Subject: [PATCH 3/9] Add group role to form section and field group components --- .../src/components/Form/FormSection.tsx | 22 +++++++++++++---- .../Form/InternalFormFieldGroup.tsx | 2 ++ .../Form/__tests__/FormFieldGroup.test.tsx | 24 ++++++++++++++++++- .../Form/__tests__/FormSection.test.tsx | 14 ++++++++++- .../FormFieldGroup.test.tsx.snap | 4 ++++ .../__snapshots__/FormSection.test.tsx.snap | 4 ++++ 6 files changed, 64 insertions(+), 6 deletions(-) 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__/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

From 0b84510c221d9d6d942391f57015d209d376910d Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 7 Jun 2022 10:08:06 -0400 Subject: [PATCH 4/9] Update tests for form group component --- .../Form/__tests__/FormGroup.test.tsx | 72 ++++++++++++------- .../__snapshots__/FormGroup.test.tsx.snap | 66 ----------------- 2 files changed, 45 insertions(+), 93 deletions(-) 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 0d57c8fc53d..33fd1db8c85 100644 --- a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx @@ -204,33 +204,51 @@ describe('FormGroup', () => { expect(asFragment()).toMatchSnapshot(); }); - test('should render with group role', () => { - const { asFragment } = render( - - - - - ); - expect(asFragment()).toMatchSnapshot(); - }); + describe('With multiple inputs per group', () => { + test('should render with group role when multipleInputs.isRadioGroup is false', () => { + render( + + + + + ); - test('should render with radiogroup role', () => { - const { asFragment } = render( - - - - - ); - expect(asFragment()).toMatchSnapshot(); + expect(screen.getByRole('group')).toBeInTheDocument(); + }); + + test('should render with radiogroup role when multipleInputs.isRadioGroup is true', () => { + render( + + + + + ); + + expect(screen.getByRole('radiogroup')).toBeInTheDocument(); + }); + + test('should have accessible name when multipleInputs is passed in', () => { + render( + + + + + ); + + expect(screen.getByRole('group')).toHaveAccessibleName('label'); + }); + + test('should have correct element for group label when multipleInputs is passed in', () => { + render( + + + + + ); + + const labelElement = screen.getByRole('group').querySelector('.pf-c-form__label'); + + expect(labelElement.tagName).toBe('SPAN'); + }); }); }); diff --git a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap index 7f193d39259..a1cf9815318 100644 --- a/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap +++ b/packages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snap @@ -478,69 +478,3 @@ exports[`FormGroup should render stacked horizontal form group variant 1`] = ` `; - -exports[`FormGroup should render with group role 1`] = ` - -
-
- - - label - - - -
-
- - -
-
- -`; - -exports[`FormGroup should render with radiogroup role 1`] = ` - -
-
- - - label - - - -
-
- - -
-
- -`; From 92c40cf3ad2921bc0a921f3c7c8b4e200d78cf55 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Wed, 8 Jun 2022 09:45:34 -0400 Subject: [PATCH 5/9] Update tests for checking element of text label --- .../Form/__tests__/FormGroup.test.tsx | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) 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 33fd1db8c85..4b3e75b461a 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'; @@ -204,7 +205,38 @@ describe('FormGroup', () => { expect(asFragment()).toMatchSnapshot(); }); + test('input should receive focus when clicking label', () => { + render( + + + + ); + + const labelElement = screen.getByTestId('form-group-test-id').querySelector('.pf-c-form__label'); + const input = screen.getByTestId('form-group-test-id').querySelector('input'); + + userEvent.click(labelElement); + expect(input).toHaveFocus(); + }); + describe('With multiple inputs per group', () => { + test('inputs should not receive focus when clicking label with multipleInputs passed in', () => { + render( + + + + + ); + + const labelElement = screen.getByRole('group').querySelector('.pf-c-form__label'); + const inputs = screen.getByRole('group').querySelectorAll('input'); + + userEvent.click(labelElement); + inputs.forEach(input => { + expect(input).not.toHaveFocus(); + }); + }); + test('should render with group role when multipleInputs.isRadioGroup is false', () => { render( @@ -237,18 +269,5 @@ describe('FormGroup', () => { expect(screen.getByRole('group')).toHaveAccessibleName('label'); }); - - test('should have correct element for group label when multipleInputs is passed in', () => { - render( - - - - - ); - - const labelElement = screen.getByRole('group').querySelector('.pf-c-form__label'); - - expect(labelElement.tagName).toBe('SPAN'); - }); }); }); From 210e553530abc58a06185fdd042082cfd111ac1b Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Wed, 8 Jun 2022 14:20:00 -0400 Subject: [PATCH 6/9] Remove testId in tests --- .../src/components/Form/__tests__/FormGroup.test.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 4b3e75b461a..28ea679d839 100644 --- a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx @@ -207,13 +207,13 @@ describe('FormGroup', () => { test('input should receive focus when clicking label', () => { render( - + ); - const labelElement = screen.getByTestId('form-group-test-id').querySelector('.pf-c-form__label'); - const input = screen.getByTestId('form-group-test-id').querySelector('input'); + const labelElement = screen.getByText('label'); + const input = screen.getByRole('textbox'); userEvent.click(labelElement); expect(input).toHaveFocus(); @@ -228,8 +228,8 @@ describe('FormGroup', () => { ); - const labelElement = screen.getByRole('group').querySelector('.pf-c-form__label'); - const inputs = screen.getByRole('group').querySelectorAll('input'); + const labelElement = screen.getByText('label'); + const inputs = screen.getAllByRole('textbox'); userEvent.click(labelElement); inputs.forEach(input => { From a34d26508808ceaf19651495d75b9a8892a1c984 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 9 Jun 2022 10:14:55 -0400 Subject: [PATCH 7/9] Refactor implementation of new prop --- .../src/components/Form/FormGroup.tsx | 25 ++++------ .../Form/__tests__/FormGroup.test.tsx | 29 ++++++++---- .../src/components/Form/examples/Form.md | 47 +++++++++---------- 3 files changed, 50 insertions(+), 51 deletions(-) diff --git a/packages/react-core/src/components/Form/FormGroup.tsx b/packages/react-core/src/components/Form/FormGroup.tsx index 72a7798e65d..2824cd26dca 100644 --- a/packages/react-core/src/components/Form/FormGroup.tsx +++ b/packages/react-core/src/components/Form/FormGroup.tsx @@ -4,12 +4,6 @@ import { ASTERISK } from '../../helpers/htmlConstants'; import { css } from '@patternfly/react-styles'; import { ValidatedOptions } from '../../helpers/constants'; -export interface MultipleInputsObject { - /** Applies a role of "radiogroup" to the form group when set to "true", or - * a role of "group" when set to "false"/not passed in. - */ - isRadioGroup?: boolean; -} export interface FormGroupProps extends Omit, 'label'> { /** Anything that can be rendered as FormGroup content. */ children?: React.ReactNode; @@ -47,9 +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; - /** Object for identifying whether the form group has multiple inputs for a single label, and - * whether the multiple inputs are radio inputs or not. */ - multipleInputs?: MultipleInputsObject; + /** Sets the role of the form group if multiple inputs are being passed in. If multiple radio inputs + * are passed in use "radiogroup", otherwise use "group" when passing in multiple of any other input. + */ + role?: 'group' | 'radiogroup'; } export const FormGroup: React.FunctionComponent = ({ @@ -69,7 +64,7 @@ export const FormGroup: React.FunctionComponent = ({ helperTextIcon, helperTextInvalidIcon, fieldId, - multipleInputs, + role, ...props }: FormGroupProps) => { const validHelperText = @@ -106,11 +101,11 @@ export const FormGroup: React.FunctionComponent = ({ const helperTextToDisplay = validated === ValidatedOptions.error && helperTextInvalid ? inValidHelperText : showValidHelperTxt(validated); - const LabelComponent = multipleInputs ? 'span' : 'label'; + const LabelComponent = role ? 'span' : 'label'; const labelContent = ( - + {label} {isRequired && ( ); - const groupType = multipleInputs?.isRadioGroup ? 'radiogroup' : 'group'; - return (
{label && (
= ({ labelInfo && styles.modifiers.info, hasNoPaddingTop && styles.modifiers.noPaddingTop )} - {...(multipleInputs && { id: `${fieldId}-legend` })} + {...(role && { id: `${fieldId}-legend` })} > {labelInfo && ( 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 28ea679d839..e8fc538942f 100644 --- a/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormGroup.test.tsx @@ -205,7 +205,18 @@ describe('FormGroup', () => { expect(asFragment()).toMatchSnapshot(); }); - test('input should receive focus when clicking label', () => { + 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( @@ -220,9 +231,9 @@ describe('FormGroup', () => { }); describe('With multiple inputs per group', () => { - test('inputs should not receive focus when clicking label with multipleInputs passed in', () => { + test('inputs should not receive focus when clicking text label', () => { render( - + @@ -237,9 +248,9 @@ describe('FormGroup', () => { }); }); - test('should render with group role when multipleInputs.isRadioGroup is false', () => { + test('should render with group role', () => { render( - + @@ -248,9 +259,9 @@ describe('FormGroup', () => { expect(screen.getByRole('group')).toBeInTheDocument(); }); - test('should render with radiogroup role when multipleInputs.isRadioGroup is true', () => { + test('should render with radiogroup role', () => { render( - + @@ -259,9 +270,9 @@ describe('FormGroup', () => { expect(screen.getByRole('radiogroup')).toBeInTheDocument(); }); - test('should have accessible name when multipleInputs is passed in', () => { + test('should have accessible name when role is passed in', () => { render( - + diff --git a/packages/react-core/src/components/Form/examples/Form.md b/packages/react-core/src/components/Form/examples/Form.md index 8aa65ecc47e..fb845b521a1 100644 --- a/packages/react-core/src/components/Form/examples/Form.md +++ b/packages/react-core/src/components/Form/examples/Form.md @@ -7,7 +7,6 @@ propComponents: 'ActionGroup', 'Form', 'FormGroup', - 'MultipleInputsObject', 'FormSection', 'FormHelperText', 'FormFieldGroup', @@ -148,18 +147,12 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + @@ -286,18 +279,19 @@ class HorizontalForm extends React.Component { id="horizontal-form-exp" /> - - - - - + + + + + @@ -419,17 +413,18 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + @@ -644,7 +639,7 @@ class HorizontalForm extends React.Component { render() { return (
- + @@ -678,7 +673,7 @@ class HorizontalFormHelperTextOnTop extends React.Component { hasNoPaddingTop fieldId="options" isStack - multipleInputs + role="group" > From f5b9ffcbbc874ada43d7bba5b95894a098bf9b6e Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 9 Jun 2022 14:24:03 -0400 Subject: [PATCH 8/9] Update examples per request --- .../src/components/Form/examples/Form.md | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/react-core/src/components/Form/examples/Form.md b/packages/react-core/src/components/Form/examples/Form.md index fb845b521a1..3f70a41d01d 100644 --- a/packages/react-core/src/components/Form/examples/Form.md +++ b/packages/react-core/src/components/Form/examples/Form.md @@ -147,15 +147,15 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + - - - + + + @@ -281,7 +281,6 @@ class HorizontalForm extends React.Component { - + - - - + + + @@ -413,21 +412,15 @@ class SimpleForm extends React.Component { onChange={this.handleTextInputChange3} /> - + - + - - - + + + From 7c490d18d5ac07a29d26f7d0e532d41cff559640 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Mon, 13 Jun 2022 08:22:07 -0400 Subject: [PATCH 9/9] Change prop type to string --- .../src/components/Form/FormGroup.tsx | 18 ++++++++++-------- .../Form/__tests__/FormGroup.test.tsx | 13 ++++++++++++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/react-core/src/components/Form/FormGroup.tsx b/packages/react-core/src/components/Form/FormGroup.tsx index 2824cd26dca..77139da4011 100644 --- a/packages/react-core/src/components/Form/FormGroup.tsx +++ b/packages/react-core/src/components/Form/FormGroup.tsx @@ -41,10 +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 if multiple inputs are being passed in. If multiple radio inputs - * are passed in use "radiogroup", otherwise use "group" when passing in multiple of any other input. + /** 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?: 'group' | 'radiogroup'; + role?: string; } export const FormGroup: React.FunctionComponent = ({ @@ -101,11 +101,12 @@ export const FormGroup: React.FunctionComponent = ({ const helperTextToDisplay = validated === ValidatedOptions.error && helperTextInvalid ? inValidHelperText : showValidHelperTxt(validated); - const LabelComponent = role ? 'span' : 'label'; + const isGroupOrRadioGroup = role === 'group' || role === 'radiogroup'; + const LabelComponent = isGroupOrRadioGroup ? 'span' : 'label'; const labelContent = ( - + {label} {isRequired && (