Skip to content

Commit a7168e1

Browse files
committed
Render InputGroup, ButtonGroup, and Radio as <fieldset> to improve controlling and accessibility
`InputGroup` can be `disabled` now. ⚠️ Pontential BC: The suffix of the ID of `Radio` label element changed from `__labelText` to `__label` (so all input groups use the same approach).
1 parent 2d8f42d commit a7168e1

File tree

29 files changed

+243
-100
lines changed

29 files changed

+243
-100
lines changed

src/lib/components/Button/Button.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const Button = React.forwardRef((props, ref) => {
6060
inputGroupContext && styles.isRootInInputGroup,
6161
feedbackIcon && styles.hasRootFeedback,
6262
)}
63-
disabled={resolveContextOrProp(buttonGroupContext && buttonGroupContext.disabled, disabled) || !!feedbackIcon}
63+
disabled={resolveContextOrProp(primaryContext && primaryContext.disabled, disabled) || !!feedbackIcon}
6464
id={id}
6565
ref={ref}
6666
>

src/lib/components/Button/__tests__/Button.test.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ describe('rendering', () => {
8989
expect(within(rootElement).getByText('label')).toHaveAttribute('id', 'id__labelText');
9090
},
9191
],
92-
...labelPropTest,
92+
...labelPropTest(),
9393
[
9494
{ labelVisibility: 'sm' },
9595
(rootElement) => expect(rootElement).toHaveClass('hasLabelVisibleSm'),

src/lib/components/ButtonGroup/ButtonGroup.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ export const ButtonGroup = ({
2121
}
2222

2323
return (
24-
<div
24+
<fieldset
2525
{...transferProps(restProps)}
2626
className={classNames(
2727
styles.root,
2828
block && styles.isRootBlock,
2929
getRootPriorityClassName(priority, styles),
3030
)}
31-
role="group"
31+
disabled={disabled}
3232
>
3333
<ButtonGroupContext.Provider
3434
value={{
@@ -40,7 +40,7 @@ export const ButtonGroup = ({
4040
>
4141
{children}
4242
</ButtonGroupContext.Provider>
43-
</div>
43+
</fieldset>
4444
);
4545
};
4646

src/lib/components/ButtonGroup/README.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ See [API](#api) for all available options.
5959
the [SelectField](/components/select-field) or
6060
[Radio](/components/radio) components.
6161

62+
- In the background, ButtonGroup uses the [`fieldset`][fieldset] element. Not
63+
only it improves the [accessibility] of the group, it also allows you to make
64+
use of its built-in features like disabling all nested inputs or pairing the
65+
group with a form outside. Consult [the MDN docs][fieldset] to learn more.
66+
6267
- Be careful with using `startCorner` and `endCorner` options for grouped
6368
buttons. Overflowing elements may cause undesired interaction problems.
6469

@@ -277,5 +282,7 @@ its accessibility.
277282
| `--rui-ButtonGroup--flat__separator__width` | Separator width for `flat` buttons |
278283
| `--rui-ButtonGroup--flat__separator__color` | Separator color for `flat` buttons |
279284

285+
[fieldset]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset
286+
[accessibility]: https://www.w3.org/WAI/tutorials/forms/grouping/
280287
[React synthetic events]: https://reactjs.org/docs/events.html
281288
[div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes

src/lib/components/ButtonGroup/__tests__/ButtonGroup.test.jsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,17 @@ describe('rendering', () => {
2828
],
2929
[
3030
{ disabled: true },
31-
(rootElement) => expect(within(rootElement).getByRole('button')).toBeDisabled(),
31+
(rootElement) => {
32+
expect(rootElement).toBeDisabled();
33+
expect(within(rootElement).getByRole('button')).toBeDisabled();
34+
},
3235
],
3336
[
3437
{ disabled: false },
35-
(rootElement) => expect(within(rootElement).getByRole('button')).not.toBeDisabled(),
38+
(rootElement) => {
39+
expect(rootElement).not.toBeDisabled();
40+
expect(within(rootElement).getByRole('button')).not.toBeDisabled();
41+
},
3642
],
3743
[
3844
{ priority: 'filled' },

src/lib/components/CheckboxField/__tests__/CheckboxField.test.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { disabledPropTest } from '../../../../../tests/propTests/disabledPropTes
1010
import { refPropTest } from '../../../../../tests/propTests/refPropTest';
1111
import { helpTextPropTest } from '../../../../../tests/propTests/helpTextPropTest';
1212
import { formLayoutProviderTest } from '../../../../../tests/providerTests/formLayoutProviderTest';
13-
import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
13+
import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
1414
import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
1515
import { requiredPropTest } from '../../../../../tests/propTests/requiredPropTest';
1616
import { validationStatePropTest } from '../../../../../tests/propTests/validationStatePropTest';
@@ -42,8 +42,8 @@ describe('rendering', () => {
4242
expect(within(rootElement).getByText('validation text')).toHaveAttribute('id', 'id__validationText');
4343
},
4444
],
45-
...isLabelVisible,
46-
...labelPropTest,
45+
...isLabelVisibleTest(),
46+
...labelPropTest(),
4747
...requiredPropTest,
4848
...validationStatePropTest,
4949
...validationTextPropTest,

src/lib/components/FileInputField/__tests__/FileInputField.test.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { refPropTest } from '../../../../../tests/propTests/refPropTest';
1111
import { fullWidthPropTest } from '../../../../../tests/propTests/fullWidthPropTest';
1212
import { helpTextPropTest } from '../../../../../tests/propTests/helpTextPropTest';
1313
import { formLayoutProviderTest } from '../../../../../tests/providerTests/formLayoutProviderTest';
14-
import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
14+
import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
1515
import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
1616
import { layoutPropTest } from '../../../../../tests/propTests/layoutPropTest';
1717
import { requiredPropTest } from '../../../../../tests/propTests/requiredPropTest';
@@ -45,8 +45,8 @@ describe('rendering', () => {
4545
expect(rootElement).toHaveAttribute('id', 'id__label');
4646
},
4747
],
48-
...isLabelVisible,
49-
...labelPropTest,
48+
...isLabelVisibleTest(),
49+
...labelPropTest(),
5050
...layoutPropTest,
5151
...requiredPropTest,
5252
...validationStatePropTest,

src/lib/components/FormLayout/README.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ This is a demo of all components supported by FormLayout.
443443
value={fruit}
444444
/>
445445
<InputGroup label="Promo code">
446-
<TextField label="Promo code" />
446+
<TextField label="Code" />
447447
<Button label="Apply" color="secondary" priority="outline" />
448448
</InputGroup>
449449
</FormLayout>

src/lib/components/FormLayout/__tests__/FormLayoutCustomField.test.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('rendering', () => {
7575
{ innerFieldSize: 'large' },
7676
(rootElement) => expect(rootElement).toHaveClass('isRootSizeLarge'),
7777
],
78-
...labelPropTest,
78+
...labelPropTest(),
7979
[
8080
{
8181
label: 'label',

src/lib/components/InputGroup/InputGroup.jsx

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import styles from './InputGroup.scss';
1414

1515
export const InputGroup = ({
1616
children,
17+
disabled,
1718
id,
1819
isLabelVisible,
1920
label,
@@ -39,7 +40,7 @@ export const InputGroup = ({
3940
);
4041

4142
return (
42-
<div
43+
<fieldset
4344
{...transferProps(restProps)}
4445
id={id}
4546
className={classNames(
@@ -48,27 +49,36 @@ export const InputGroup = ({
4849
resolveContextOrProp(formLayoutContext && formLayoutContext.layout, layout) === 'horizontal'
4950
? styles.isRootLayoutHorizontal
5051
: styles.isRootLayoutVertical,
52+
disabled && styles.isRootDisabled,
5153
getRootSizeClassName(size, styles),
5254
getRootValidationStateClassName(validationState, styles),
5355
)}
56+
disabled={disabled}
5457
>
58+
<legend
59+
className={styles.legend}
60+
id={id && `${id}__label`}
61+
>
62+
{label}
63+
</legend>
5564
<div
65+
aria-hidden
5666
className={classNames(
5767
styles.label,
5868
!isLabelVisible && styles.isLabelHidden,
5969
)}
60-
id={id && `${id}__label`}
70+
id={id && `${id}__displayLabel`}
6171
>
6272
{label}
6373
</div>
6474
<div className={styles.field}>
6575
<div
6676
className={styles.inputGroup}
6777
id={id && `${id}__group`}
68-
role="group"
6978
>
7079
<InputGroupContext.Provider
7180
value={{
81+
disabled,
7282
layout,
7383
size,
7484
}}
@@ -77,24 +87,27 @@ export const InputGroup = ({
7787
</InputGroupContext.Provider>
7888
</div>
7989
{validationTexts && (
80-
<div
90+
<ul
8191
className={styles.validationText}
8292
id={id && `${id}__validationTexts`}
8393
>
8494
{validationTexts.map((validationText) => (
85-
<Text blockLevel key={validationText}>
86-
{validationText}
87-
</Text>
95+
<li key={validationText}>
96+
<Text blockLevel>
97+
{validationText}
98+
</Text>
99+
</li>
88100
))}
89-
</div>
101+
</ul>
90102
)}
91103
</div>
92-
</div>
104+
</fieldset>
93105
);
94106
};
95107

96108
InputGroup.defaultProps = {
97109
children: null,
110+
disabled: false,
98111
id: undefined,
99112
isLabelVisible: true,
100113
layout: 'vertical',
@@ -112,12 +125,17 @@ InputGroup.propTypes = {
112125
* If none are provided nothing is rendered.
113126
*/
114127
children: PropTypes.node,
128+
/**
129+
* If `true`, the whole input group with all nested inputs and buttons will be disabled.
130+
*/
131+
disabled: PropTypes.bool,
115132
/**
116133
* ID of the root HTML element.
117134
*
118135
* Also serves as base for ids of nested elements:
119-
* * `<ID>__group`
120136
* * `<ID>__label`
137+
* * `<ID>__displayLabel`
138+
* * `<ID>__group`
121139
* * `<ID>__validationTexts`
122140
*/
123141
id: PropTypes.string,

src/lib/components/InputGroup/InputGroup.scss

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,29 @@
11
// 1. The class name is intentionally singular because it's targeted by other mixins too.
22
// 2. Use a block-level display mode to prevent extra white space below grouped inputs in Safari.
33
// 3. Prevent individual inputs from overlapping inside narrow containers.
4+
// 4. Legends are tricky to style, let's use a `div` instead.
5+
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset#styling_with_css
46

57
@use "../../styles/tools/form-fields/box-field-elements";
68
@use "../../styles/tools/form-fields/box-field-layout";
79
@use "../../styles/tools/form-fields/box-field-sizes";
810
@use "../../styles/tools/form-fields/foundation";
911
@use "../../styles/tools/form-fields/variants";
1012
@use "../../styles/tools/accessibility";
13+
@use "../../styles/tools/reset";
1114
@use "theme";
1215

1316
.root {
14-
@include box-field-elements.input-container();
17+
@include foundation.root();
18+
@include foundation.fieldset();
1519
}
1620

21+
// 4.
22+
.legend {
23+
@include accessibility.hide-text();
24+
}
25+
26+
// 4.
1727
.label {
1828
@include foundation.label();
1929
}
@@ -27,6 +37,7 @@
2737

2838
// 1.
2939
.validationText {
40+
@include reset.list();
3041
@include foundation.help-text();
3142
}
3243

src/lib/components/InputGroup/README.mdx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ See [API](#api) for all available options.
7777
InputGroup is currently **not responsive: the inputs do not shrink nor wrap**.
7878
Make sure your inputs fit their container, especially on small screens.
7979

80+
- In the background, InputGroup uses the [`fieldset`][fieldset] element. Not
81+
only it improves the [accessibility] of the group, it also allows you to make
82+
use of its built-in features like disabling all nested inputs or pairing the
83+
group with a form outside. Consult [the MDN docs][fieldset] to learn more.
84+
8085
- InputGroup currently **supports grouping of**
8186
[TextField](/components/text-field), [SelectField](/components/select-field),
8287
and [Button](/components/button) components.
@@ -165,6 +170,17 @@ supports this kind of layout as well.
165170

166171
## States
167172

173+
### Disabled State
174+
175+
Disables all fields and buttons inside the group.
176+
177+
<Playground>
178+
<InputGroup disabled label="Disabled group">
179+
<TextField label="Label" />
180+
<Button label="Submit" />
181+
</InputGroup>
182+
</Playground>
183+
168184
### Validation States
169185

170186
Validation states visually present the result of validation of the grouped
@@ -256,5 +272,7 @@ interactive and helps to improve its accessibility.
256272
| `--rui-InputGroup__gap` | Gap between elements |
257273
| `--rui-InputGroup__inner-border-radius` | Inner border radius of elements |
258274

275+
[fieldset]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset
276+
[accessibility]: https://www.w3.org/WAI/tutorials/forms/grouping/
259277
[React synthetic events]: https://reactjs.org/docs/events.html
260278
[div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes

src/lib/components/InputGroup/__tests__/InputGroup.test.jsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ import {
33
render,
44
within,
55
} from '@testing-library/react';
6+
import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
67
import { Button } from '../../Button';
78
import { SelectField } from '../../SelectField';
89
import { TextField } from '../../TextField';
910
import { childrenEmptyPropTest } from '../../../../../tests/propTests/childrenEmptyPropTest';
10-
import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
11+
import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
1112
import { layoutPropTest } from '../../../../../tests/propTests/layoutPropTest';
1213
import { InputGroup } from '../InputGroup';
1314

@@ -38,6 +39,15 @@ describe('rendering', () => {
3839
expect(within(rootElement).getByRole('button')).toHaveClass('isRootInInputGroup');
3940
},
4041
],
42+
[
43+
{ disabled: true },
44+
(rootElement) => {
45+
expect(rootElement).toBeDisabled();
46+
expect(within(rootElement).getByRole('textbox')).toBeDisabled();
47+
expect(within(rootElement).getByRole('combobox')).toBeDisabled();
48+
expect(within(rootElement).getByRole('button')).toBeDisabled();
49+
},
50+
],
4151
[
4252
{
4353
id: 'id',
@@ -46,10 +56,12 @@ describe('rendering', () => {
4656
(rootElement) => {
4757
expect(rootElement).toHaveAttribute('id', 'id');
4858
expect(within(rootElement).getByTestId('id__group'));
49-
expect(within(rootElement).getByText('label')).toHaveAttribute('id', 'id__label');
59+
expect(within(rootElement).getByTestId('id__label'));
60+
expect(within(rootElement).getByTestId('id__displayLabel'));
5061
},
5162
],
52-
...isLabelVisible,
63+
...isLabelVisibleTest('legend'),
64+
...labelPropTest('legend'),
5365
...layoutPropTest,
5466
[
5567
{ size: 'small' },

src/lib/components/Modal/README.mdx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ See [API](#api) for all available options.
122122
- **Avoid stacking** of modals. While it may technically work, the modal is just
123123
not designed for that.
124124

125+
📖 [Read more about modals at Nielsen Norman Group.][nng-modal]
126+
125127
## Composition
126128

127129
Modal is decomposed into the following components:
@@ -1081,6 +1083,7 @@ accessibility.
10811083
| `--rui-Modal--fullscreen__width` | Width of fullscreen modal |
10821084
| `--rui-Modal--fullscreen__height` | Height of fullscreen modal |
10831085

1086+
[nng-modal]: https://www.nngroup.com/articles/modal-nonmodal-dialog/
10841087
[React synthetic events]: https://reactjs.org/docs/events.html
10851088
[div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes
10861089
[heading]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#attributes

0 commit comments

Comments
 (0)