Render InputGroup, ButtonGroup, and Radio as <fieldset> to im…

…prove 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).
react-ui-org · May 22, 2023 · a7168e1 · a7168e1
1 parent 2d8f42d
commit a7168e1
Show file tree

Hide file tree

Showing 29 changed files with 243 additions and 100 deletions.
diff --git a/src/lib/components/Button/Button.jsx b/src/lib/components/Button/Button.jsx
@@ -60,7 +60,7 @@ export const Button = React.forwardRef((props, ref) => {
         inputGroupContext && styles.isRootInInputGroup,
         feedbackIcon && styles.hasRootFeedback,
       )}
-      disabled={resolveContextOrProp(buttonGroupContext && buttonGroupContext.disabled, disabled) || !!feedbackIcon}
+      disabled={resolveContextOrProp(primaryContext && primaryContext.disabled, disabled) || !!feedbackIcon}
       id={id}
       ref={ref}
     >

diff --git a/src/lib/components/Button/__tests__/Button.test.jsx b/src/lib/components/Button/__tests__/Button.test.jsx
@@ -89,7 +89,7 @@ describe('rendering', () => {
         expect(within(rootElement).getByText('label')).toHaveAttribute('id', 'id__labelText');
       },
     ],
-    ...labelPropTest,
+    ...labelPropTest(),
     [
       { labelVisibility: 'sm' },
       (rootElement) => expect(rootElement).toHaveClass('hasLabelVisibleSm'),

diff --git a/src/lib/components/ButtonGroup/ButtonGroup.jsx b/src/lib/components/ButtonGroup/ButtonGroup.jsx
@@ -21,14 +21,14 @@ export const ButtonGroup = ({
   }
 
   return (
-    <div
+    <fieldset
       {...transferProps(restProps)}
       className={classNames(
         styles.root,
         block && styles.isRootBlock,
         getRootPriorityClassName(priority, styles),
       )}
-      role="group"
+      disabled={disabled}
     >
       <ButtonGroupContext.Provider
         value={{
@@ -40,7 +40,7 @@ export const ButtonGroup = ({
       >
         {children}
       </ButtonGroupContext.Provider>
-    </div>
+    </fieldset>
   );
 };
 

diff --git a/src/lib/components/ButtonGroup/README.mdx b/src/lib/components/ButtonGroup/README.mdx
@@ -59,6 +59,11 @@ See [API](#api) for all available options.
   the [SelectField](/components/select-field) or
   [Radio](/components/radio) components.
 
+- In the background, ButtonGroup uses the [`fieldset`][fieldset] element. Not
+  only it improves the [accessibility] of the group, it also allows you to make
+  use of its built-in features like disabling all nested inputs or pairing the
+  group with a form outside. Consult [the MDN docs][fieldset] to learn more.
+
 - Be careful with using `startCorner` and `endCorner` options for grouped
   buttons. Overflowing elements may cause undesired interaction problems.
 
@@ -277,5 +282,7 @@ its accessibility.
 | `--rui-ButtonGroup--flat__separator__width`                        | Separator width for `flat` buttons             |
 | `--rui-ButtonGroup--flat__separator__color`                        | Separator color for `flat` buttons             |
 
+[fieldset]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset
+[accessibility]: https://www.w3.org/WAI/tutorials/forms/grouping/
 [React synthetic events]: https://reactjs.org/docs/events.html
 [div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes
diff --git a/src/lib/components/ButtonGroup/__tests__/ButtonGroup.test.jsx b/src/lib/components/ButtonGroup/__tests__/ButtonGroup.test.jsx
@@ -28,11 +28,17 @@ describe('rendering', () => {
     ],
     [
       { disabled: true },
-      (rootElement) => expect(within(rootElement).getByRole('button')).toBeDisabled(),
+      (rootElement) => {
+        expect(rootElement).toBeDisabled();
+        expect(within(rootElement).getByRole('button')).toBeDisabled();
+      },
     ],
     [
       { disabled: false },
-      (rootElement) => expect(within(rootElement).getByRole('button')).not.toBeDisabled(),
+      (rootElement) => {
+        expect(rootElement).not.toBeDisabled();
+        expect(within(rootElement).getByRole('button')).not.toBeDisabled();
+      },
     ],
     [
       { priority: 'filled' },

diff --git a/src/lib/components/CheckboxField/__tests__/CheckboxField.test.jsx b/src/lib/components/CheckboxField/__tests__/CheckboxField.test.jsx
@@ -10,7 +10,7 @@ import { disabledPropTest } from '../../../../../tests/propTests/disabledPropTes
 import { refPropTest } from '../../../../../tests/propTests/refPropTest';
 import { helpTextPropTest } from '../../../../../tests/propTests/helpTextPropTest';
 import { formLayoutProviderTest } from '../../../../../tests/providerTests/formLayoutProviderTest';
-import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
+import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
 import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
 import { requiredPropTest } from '../../../../../tests/propTests/requiredPropTest';
 import { validationStatePropTest } from '../../../../../tests/propTests/validationStatePropTest';
@@ -42,8 +42,8 @@ describe('rendering', () => {
         expect(within(rootElement).getByText('validation text')).toHaveAttribute('id', 'id__validationText');
       },
     ],
-    ...isLabelVisible,
-    ...labelPropTest,
+    ...isLabelVisibleTest(),
+    ...labelPropTest(),
     ...requiredPropTest,
     ...validationStatePropTest,
     ...validationTextPropTest,

diff --git a/src/lib/components/FileInputField/__tests__/FileInputField.test.jsx b/src/lib/components/FileInputField/__tests__/FileInputField.test.jsx
@@ -11,7 +11,7 @@ import { refPropTest } from '../../../../../tests/propTests/refPropTest';
 import { fullWidthPropTest } from '../../../../../tests/propTests/fullWidthPropTest';
 import { helpTextPropTest } from '../../../../../tests/propTests/helpTextPropTest';
 import { formLayoutProviderTest } from '../../../../../tests/providerTests/formLayoutProviderTest';
-import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
+import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
 import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
 import { layoutPropTest } from '../../../../../tests/propTests/layoutPropTest';
 import { requiredPropTest } from '../../../../../tests/propTests/requiredPropTest';
@@ -45,8 +45,8 @@ describe('rendering', () => {
         expect(rootElement).toHaveAttribute('id', 'id__label');
       },
     ],
-    ...isLabelVisible,
-    ...labelPropTest,
+    ...isLabelVisibleTest(),
+    ...labelPropTest(),
     ...layoutPropTest,
     ...requiredPropTest,
     ...validationStatePropTest,

diff --git a/src/lib/components/FormLayout/README.mdx b/src/lib/components/FormLayout/README.mdx
@@ -443,7 +443,7 @@ This is a demo of all components supported by FormLayout.
             value={fruit}
           />
           <InputGroup label="Promo code">
-            <TextField label="Promo code" />
+            <TextField label="Code" />
             <Button label="Apply" color="secondary" priority="outline" />
           </InputGroup>
         </FormLayout>

diff --git a/src/lib/components/FormLayout/__tests__/FormLayoutCustomField.test.jsx b/src/lib/components/FormLayout/__tests__/FormLayoutCustomField.test.jsx
@@ -75,7 +75,7 @@ describe('rendering', () => {
       { innerFieldSize: 'large' },
       (rootElement) => expect(rootElement).toHaveClass('isRootSizeLarge'),
     ],
-    ...labelPropTest,
+    ...labelPropTest(),
     [
       {
         label: 'label',

diff --git a/src/lib/components/InputGroup/InputGroup.jsx b/src/lib/components/InputGroup/InputGroup.jsx
@@ -14,6 +14,7 @@ import styles from './InputGroup.scss';
 
 export const InputGroup = ({
   children,
+  disabled,
   id,
   isLabelVisible,
   label,
@@ -39,7 +40,7 @@ export const InputGroup = ({
   );
 
   return (
-    <div
+    <fieldset
       {...transferProps(restProps)}
       id={id}
       className={classNames(
@@ -48,27 +49,36 @@ export const InputGroup = ({
         resolveContextOrProp(formLayoutContext && formLayoutContext.layout, layout) === 'horizontal'
           ? styles.isRootLayoutHorizontal
           : styles.isRootLayoutVertical,
+        disabled && styles.isRootDisabled,
         getRootSizeClassName(size, styles),
         getRootValidationStateClassName(validationState, styles),
       )}
+      disabled={disabled}
     >
+      <legend
+        className={styles.legend}
+        id={id && `${id}__label`}
+      >
+        {label}
+      </legend>
       <div
+        aria-hidden
         className={classNames(
           styles.label,
           !isLabelVisible && styles.isLabelHidden,
         )}
-        id={id && `${id}__label`}
+        id={id && `${id}__displayLabel`}
       >
         {label}
       </div>
       <div className={styles.field}>
         <div
           className={styles.inputGroup}
           id={id && `${id}__group`}
-          role="group"
         >
           <InputGroupContext.Provider
             value={{
+              disabled,
               layout,
               size,
             }}
@@ -77,24 +87,27 @@ export const InputGroup = ({
           </InputGroupContext.Provider>
         </div>
         {validationTexts && (
-          <div
+          <ul
             className={styles.validationText}
             id={id && `${id}__validationTexts`}
           >
             {validationTexts.map((validationText) => (
-              <Text blockLevel key={validationText}>
-                {validationText}
-              </Text>
+              <li key={validationText}>
+                <Text blockLevel>
+                  {validationText}
+                </Text>
+              </li>
             ))}
-          </div>
+          </ul>
         )}
       </div>
-    </div>
+    </fieldset>
   );
 };
 
 InputGroup.defaultProps = {
   children: null,
+  disabled: false,
   id: undefined,
   isLabelVisible: true,
   layout: 'vertical',
@@ -112,12 +125,17 @@ InputGroup.propTypes = {
    * If none are provided nothing is rendered.
    */
   children: PropTypes.node,
+  /**
+   * If `true`, the whole input group with all nested inputs and buttons will be disabled.
+   */
+  disabled: PropTypes.bool,
   /**
    * ID of the root HTML element.
    *
    * Also serves as base for ids of nested elements:
-   * * `<ID>__group`
    * * `<ID>__label`
+   * * `<ID>__displayLabel`
+   * * `<ID>__group`
    * * `<ID>__validationTexts`
    */
   id: PropTypes.string,

diff --git a/src/lib/components/InputGroup/InputGroup.scss b/src/lib/components/InputGroup/InputGroup.scss
@@ -1,19 +1,29 @@
 // 1. The class name is intentionally singular because it's targeted by other mixins too.
 // 2. Use a block-level display mode to prevent extra white space below grouped inputs in Safari.
 // 3. Prevent individual inputs from overlapping inside narrow containers.
+// 4. Legends are tricky to style, let's use a `div` instead.
+//    https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset#styling_with_css
 
 @use "../../styles/tools/form-fields/box-field-elements";
 @use "../../styles/tools/form-fields/box-field-layout";
 @use "../../styles/tools/form-fields/box-field-sizes";
 @use "../../styles/tools/form-fields/foundation";
 @use "../../styles/tools/form-fields/variants";
 @use "../../styles/tools/accessibility";
+@use "../../styles/tools/reset";
 @use "theme";
 
 .root {
-    @include box-field-elements.input-container();
+    @include foundation.root();
+    @include foundation.fieldset();
 }
 
+// 4.
+.legend {
+    @include accessibility.hide-text();
+}
+
+// 4.
 .label {
     @include foundation.label();
 }
@@ -27,6 +37,7 @@
 
 // 1.
 .validationText {
+    @include reset.list();
     @include foundation.help-text();
 }
 

diff --git a/src/lib/components/InputGroup/README.mdx b/src/lib/components/InputGroup/README.mdx
@@ -77,6 +77,11 @@ See [API](#api) for all available options.
   InputGroup is currently **not responsive: the inputs do not shrink nor wrap**.
   Make sure your inputs fit their container, especially on small screens.
 
+- In the background, InputGroup uses the [`fieldset`][fieldset] element. Not
+  only it improves the [accessibility] of the group, it also allows you to make
+  use of its built-in features like disabling all nested inputs or pairing the
+  group with a form outside. Consult [the MDN docs][fieldset] to learn more.
+
 - InputGroup currently **supports grouping of**
   [TextField](/components/text-field), [SelectField](/components/select-field),
   and [Button](/components/button) components.
@@ -165,6 +170,17 @@ supports this kind of layout as well.
 
 ## States
 
+### Disabled State
+
+Disables all fields and buttons inside the group.
+
+<Playground>
+  <InputGroup disabled label="Disabled group">
+    <TextField label="Label" />
+    <Button label="Submit" />
+  </InputGroup>
+</Playground>
+
 ### Validation States
 
 Validation states visually present the result of validation of the grouped
@@ -256,5 +272,7 @@ interactive and helps to improve its accessibility.
 | `--rui-InputGroup__gap`                                            | Gap between elements                           |
 | `--rui-InputGroup__inner-border-radius`                            | Inner border radius of elements                |
 
+[fieldset]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset
+[accessibility]: https://www.w3.org/WAI/tutorials/forms/grouping/
 [React synthetic events]: https://reactjs.org/docs/events.html
 [div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes
diff --git a/src/lib/components/InputGroup/__tests__/InputGroup.test.jsx b/src/lib/components/InputGroup/__tests__/InputGroup.test.jsx
@@ -3,11 +3,12 @@ import {
   render,
   within,
 } from '@testing-library/react';
+import { labelPropTest } from '../../../../../tests/propTests/labelPropTest';
 import { Button } from '../../Button';
 import { SelectField } from '../../SelectField';
 import { TextField } from '../../TextField';
 import { childrenEmptyPropTest } from '../../../../../tests/propTests/childrenEmptyPropTest';
-import { isLabelVisible } from '../../../../../tests/propTests/isLabelVisible';
+import { isLabelVisibleTest } from '../../../../../tests/propTests/isLabelVisibleTest';
 import { layoutPropTest } from '../../../../../tests/propTests/layoutPropTest';
 import { InputGroup } from '../InputGroup';
 
@@ -38,6 +39,15 @@ describe('rendering', () => {
         expect(within(rootElement).getByRole('button')).toHaveClass('isRootInInputGroup');
       },
     ],
+    [
+      { disabled: true },
+      (rootElement) => {
+        expect(rootElement).toBeDisabled();
+        expect(within(rootElement).getByRole('textbox')).toBeDisabled();
+        expect(within(rootElement).getByRole('combobox')).toBeDisabled();
+        expect(within(rootElement).getByRole('button')).toBeDisabled();
+      },
+    ],
     [
       {
         id: 'id',
@@ -46,10 +56,12 @@ describe('rendering', () => {
       (rootElement) => {
         expect(rootElement).toHaveAttribute('id', 'id');
         expect(within(rootElement).getByTestId('id__group'));
-        expect(within(rootElement).getByText('label')).toHaveAttribute('id', 'id__label');
+        expect(within(rootElement).getByTestId('id__label'));
+        expect(within(rootElement).getByTestId('id__displayLabel'));
       },
     ],
-    ...isLabelVisible,
+    ...isLabelVisibleTest('legend'),
+    ...labelPropTest('legend'),
     ...layoutPropTest,
     [
       { size: 'small' },

diff --git a/src/lib/components/Modal/README.mdx b/src/lib/components/Modal/README.mdx
@@ -122,6 +122,8 @@ See [API](#api) for all available options.
 - **Avoid stacking** of modals. While it may technically work, the modal is just
   not designed for that.
 
+📖 [Read more about modals at Nielsen Norman Group.][nng-modal]
+
 ## Composition
 
 Modal is decomposed into the following components:
@@ -1081,6 +1083,7 @@ accessibility.
 | `--rui-Modal--fullscreen__width`                     | Width of fullscreen modal                                     |
 | `--rui-Modal--fullscreen__height`                    | Height of fullscreen modal                                    |
 
+[nng-modal]: https://www.nngroup.com/articles/modal-nonmodal-dialog/
 [React synthetic events]: https://reactjs.org/docs/events.html
 [div]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#attributes
 [heading]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#attributes