Skip to content

Commit 6898a87

Browse files
adamalstonadamalston
authored
fix: ignore empty helperText across form controls (#21995)
1 parent 772779d commit 6898a87

19 files changed

Lines changed: 159 additions & 18 deletions

‎packages/react/src/components/Checkbox/Checkbox.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { deprecate } from '../../prop-types/deprecate';
1313
import { usePrefix } from '../../internal/usePrefix';
1414
import { WarningFilled, WarningAltFilled } from '@carbon/icons-react';
1515
import { useId } from '../../internal/useId';
16+
import { hasHelperText } from '../../internal/hasHelperText';
1617
import { noopFn } from '../../internal/noopFn';
1718
import { useNormalizedInputProps } from '../../internal/useNormalizedInputProps';
1819
import { AILabel } from '../AILabel';
@@ -149,7 +150,7 @@ const Checkbox = React.forwardRef(
149150

150151
const checkboxGroupInstanceId = useId();
151152

152-
const hasHelper = typeof helperText !== 'undefined' && helperText !== null;
153+
const hasHelper = hasHelperText(helperText);
153154
const helperId = !hasHelper
154155
? undefined
155156
: `checkbox-helper-text-${checkboxGroupInstanceId}`;

‎packages/react/src/components/Checkbox/__tests__/Checkbox-test.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@ describe('Checkbox', () => {
8383
expect(screen.getByText('0')).toBeInTheDocument();
8484
});
8585

86+
it('should not render helperText when helperText is an empty string', () => {
87+
const { container } = render(
88+
<Checkbox id="test" labelText="Checkbox label" helperText="" />
89+
);
90+
91+
expect(
92+
container.querySelector(`.${prefix}--form__helper-text`)
93+
).not.toBeInTheDocument();
94+
});
95+
8696
it('should set data-invalid when invalid prop is true', () => {
8797
render(
8898
<Checkbox

‎packages/react/src/components/CheckboxGroup/CheckboxGroup-test.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ describe('CheckboxGroup', () => {
4848
expect(screen.getByText('0')).toBeInTheDocument();
4949
});
5050

51+
it('should not render helperText when helperText is an empty string', () => {
52+
const { container } = render(
53+
<CheckboxGroup legendText="Checkbox heading" helperText="" />
54+
);
55+
56+
expect(
57+
container.querySelector(`.${prefix}--form__helper-text`)
58+
).not.toBeInTheDocument();
59+
expect(container.firstChild).not.toHaveAttribute('aria-describedby');
60+
});
61+
5162
it('should set data-invalid when invalid prop is true', () => {
5263
const { container } = render(
5364
<CheckboxGroup

‎packages/react/src/components/CheckboxGroup/CheckboxGroup.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { deprecate } from '../../prop-types/deprecate';
1717
import { usePrefix } from '../../internal/usePrefix';
1818
import { WarningFilled, WarningAltFilled } from '@carbon/icons-react';
1919
import { useId } from '../../internal/useId';
20+
import { hasHelperText } from '../../internal/hasHelperText';
2021
import { useNormalizedInputProps } from '../../internal/useNormalizedInputProps';
2122
import { AILabel } from '../AILabel';
2223
import { isComponentElement } from '../../internal';
@@ -80,7 +81,7 @@ const CheckboxGroup = ({
8081
const showWarning = normalizedProps.warn;
8182
const showHelper = !normalizedProps.invalid && !normalizedProps.warn;
8283

83-
const hasHelper = typeof helperText !== 'undefined' && helperText !== null;
84+
const hasHelper = hasHelperText(helperText);
8485
const helperId = !hasHelper
8586
? undefined
8687
: `checkbox-group-helper-text-${checkboxGroupInstanceId}`;

‎packages/react/src/components/MultiSelect/FilterableMultiSelect.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import {
6666
import type { TranslateWithId } from '../../types/common';
6767
import { AILabel } from '../AILabel';
6868
import { defaultItemToString, isComponentElement } from '../../internal';
69+
import { hasHelperText } from '../../internal/hasHelperText';
6970
import { useNormalizedInputProps } from '../../internal/useNormalizedInputProps';
7071
import useIsomorphicEffect from '../../internal/useIsomorphicEffect';
7172

@@ -568,7 +569,7 @@ export const FilterableMultiSelect = forwardRef(function FilterableMultiSelect<
568569
[`${prefix}--autoalign`]: autoAlign,
569570
}
570571
);
571-
const hasHelper = typeof helperText !== 'undefined' && helperText !== null;
572+
const hasHelper = hasHelperText(helperText);
572573
const helperId = !hasHelper
573574
? undefined
574575
: `filterablemultiselect-helper-text-${filterableMultiSelectInstanceId}`;
@@ -890,7 +891,7 @@ export const FilterableMultiSelect = forwardRef(function FilterableMultiSelect<
890891
const inputProp = getInputProps(
891892
getDropdownProps({
892893
'aria-controls': isOpen ? menuId : undefined,
893-
'aria-describedby': helperText && showHelperText ? helperId : undefined,
894+
'aria-describedby': hasHelper && showHelperText ? helperId : undefined,
894895
'aria-haspopup': 'listbox',
895896
// Remove excess aria `aria-labelledby`. HTML <label for>
896897
// provides this aria information.

‎packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,20 @@ describe('FilterableMultiSelect', () => {
545545
expect(screen.getByText('0')).toBeInTheDocument();
546546
});
547547

548+
it('should not render helperText when helperText is an empty string', async () => {
549+
const { container } = render(
550+
<FilterableMultiSelect {...mockProps} helperText="" />
551+
);
552+
await waitForPosition();
553+
554+
expect(
555+
container.querySelector(`.${prefix}--form__helper-text`)
556+
).not.toBeInTheDocument();
557+
expect(screen.getByRole('combobox')).not.toHaveAttribute(
558+
'aria-describedby'
559+
);
560+
});
561+
548562
it('should handle hideLabel prop', async () => {
549563
render(
550564
<FilterableMultiSelect {...mockProps} titleText="Test Title" hideLabel />

‎packages/react/src/components/RadioButtonGroup/RadioButtonGroup-test.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,21 @@ describe('RadioButtonGroup', () => {
8282
expect(screen.getByText('0')).toBeInTheDocument();
8383
});
8484

85+
it('should not render helperText when helperText is an empty string', () => {
86+
const { container } = render(
87+
<RadioButtonGroup legendText="test" helperText="">
88+
<RadioButton labelText="test-1" value="test-1" />
89+
</RadioButtonGroup>
90+
);
91+
92+
const fieldset = container.querySelector('fieldset');
93+
94+
expect(
95+
container.querySelector(`.${prefix}--form__helper-text`)
96+
).not.toBeInTheDocument();
97+
expect(fieldset).not.toHaveAttribute('aria-describedby');
98+
});
99+
85100
it('should ignore null children', () => {
86101
render(
87102
<RadioButtonGroup defaultSelected="test-1" name="test" legendText="test">

‎packages/react/src/components/RadioButtonGroup/RadioButtonGroup.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { WarningFilled, WarningAltFilled } from '@carbon/icons-react';
2323
import { deprecate } from '../../prop-types/deprecate';
2424
import { mergeRefs } from '../../tools/mergeRefs';
2525
import { useId } from '../../internal/useId';
26+
import { hasHelperText } from '../../internal/hasHelperText';
2627
import { AILabel } from '../AILabel';
2728
import { isComponentElement } from '../../internal';
2829

@@ -238,7 +239,7 @@ const RadioButtonGroup = React.forwardRef(
238239
[`${prefix}--form__helper-text--disabled`]: disabled,
239240
});
240241

241-
const hasHelper = typeof helperText !== 'undefined' && helperText !== null;
242+
const hasHelper = hasHelperText(helperText);
242243
const helperId = !hasHelper
243244
? undefined
244245
: `radio-button-group-helper-text-${radioButtonGroupInstanceId}`;
@@ -264,7 +265,7 @@ const RadioButtonGroup = React.forwardRef(
264265
className={fieldsetClasses}
265266
disabled={disabled}
266267
data-invalid={invalid ? true : undefined}
267-
aria-describedby={showHelper && helperText ? helperId : undefined}
268+
aria-describedby={showHelper && hasHelper ? helperId : undefined}
268269
{...rest}>
269270
{legendText && (
270271
<Legend className={`${prefix}--label`}>

‎packages/react/src/components/Select/Select.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { Text } from '../Text';
3030
import { AILabel } from '../AILabel';
3131
import { isComponentElement } from '../../internal';
3232
import { useNormalizedInputProps } from '../../internal/useNormalizedInputProps';
33+
import { hasHelperText } from '../../internal/hasHelperText';
3334

3435
type ExcludedAttributes = 'size';
3536

@@ -232,7 +233,7 @@ const Select = forwardRef<HTMLSelectElement, SelectProps>(
232233
const helperTextClasses = classNames(`${prefix}--form__helper-text`, {
233234
[`${prefix}--form__helper-text--disabled`]: normalizedProps.disabled,
234235
});
235-
const hasHelper = typeof helperText !== 'undefined' && helperText !== null;
236+
const hasHelper = hasHelperText(helperText);
236237
const helper = hasHelper ? (
237238
<Text
238239
as="div"

‎packages/react/src/components/Select/__tests__/Select-test.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ describe('Select', () => {
172172
{ label: 'true', value: true, shouldRender: true },
173173
{ label: 'false', value: false, shouldRender: true },
174174
{ label: 'component', value: <span>hmm</span>, shouldRender: true },
175-
{ label: 'empty string', value: '', shouldRender: true },
175+
{ label: 'empty string', value: '', shouldRender: false },
176176
{ label: 'null', value: null, shouldRender: false },
177177
{ label: 'undefined', value: undefined, shouldRender: false },
178178
{ label: 'zero', value: '0', shouldRender: true },
@@ -192,6 +192,14 @@ describe('Select', () => {
192192
}
193193
);
194194

195+
it('should not set aria-describedby when helperText is an empty string', () => {
196+
render(<Select id="select-empty" labelText="Select" helperText="" />);
197+
198+
expect(screen.getByRole('combobox')).not.toHaveAttribute(
199+
'aria-describedby'
200+
);
201+
});
202+
195203
it('should respect hideLabel prop', () => {
196204
render(<Select id="select" labelText="Select" hideLabel />);
197205

0 commit comments

Comments
 (0)