fix: add Field wrapper to CheckboxGroup and RadioGroup (#1762)

* fix: added Field wrapper to CheckboxGroup and RadioGroup * feat: converted Field to use FormLabel and setup fieldsets * fix: lerna build running on SVGs * fix(odyssey-storybook): name -> id
okta · May 4, 2023 · 6f8c14d · 6f8c14d
1 parent 7561823
commit 6f8c14d
Show file tree

Hide file tree

Showing 12 changed files with 110 additions and 106 deletions.
diff --git a/package.json b/package.json
@@ -11,7 +11,7 @@
     "node": ">=16 <17"
   },
   "scripts": {
-    "build": "lerna run build --ignore @okta/odyssey-storybook",
+    "build": "lerna run build --ignore @okta/odyssey-storybook --ignore @okta/odyssey-icons --ignore @okta/odyssey-svgr",
     "dev:source": "lerna run dev:source",
     "eslint": "eslint . --ext .js,.jsx,.ts,.tsx",
     "generate:icons": "yarn workspace @okta/odyssey-icons build && yarn workspace @okta/odyssey-svgr build && yarn workspace @okta/odyssey-react-mui build:icons",

diff --git a/packages/odyssey-react-mui/src/Autocomplete.tsx b/packages/odyssey-react-mui/src/Autocomplete.tsx
@@ -102,8 +102,9 @@ const Autocomplete = <
   const renderInput = useCallback(
     ({ InputLabelProps, InputProps, ...params }) => (
       <Field
-        {...InputLabelProps}
+        fieldType="single"
         hasVisibleLabel
+        id={InputLabelProps.htmlFor}
         hint={hint}
         label={label}
         renderFieldComponent={({ ariaDescribedBy, id }) => (

diff --git a/packages/odyssey-react-mui/src/CheckboxGroup.tsx b/packages/odyssey-react-mui/src/CheckboxGroup.tsx
@@ -10,62 +10,54 @@
  * See the License for the specific language governing permissions and limitations under the License.
  */
 
-import { memo, ReactElement, useMemo } from "react";
+import { FormGroup as MuiFormGroup } from "@mui/material";
+import { memo, ReactElement, useCallback } from "react";
 
-import {
-  Checkbox,
-  FormControl,
-  FormGroup,
-  FormHelperText,
-  FormLabel,
-  ScreenReaderText,
-} from ".";
+import { Checkbox } from "./Checkbox";
+import { Field } from "./Field";
 
 export type CheckboxGroupProps = {
   children:
     | ReactElement<typeof Checkbox>
     | Array<ReactElement<typeof Checkbox>>;
   errorMessage?: string;
   hint?: string;
+  /**
+   * The id of the `input` element. This will also be the input's `name` field.
+   */
+  id?: string;
   isDisabled?: boolean;
-  label?: string;
-  name?: string;
+  label: string;
 };
 
 const CheckboxGroup = ({
   children,
   isDisabled,
   errorMessage,
   hint,
+  id: idOverride,
   label,
-  name,
 }: CheckboxGroupProps) => {
-  const ariaDescribedBy = useMemo(
-    () =>
-      errorMessage || hint
-        ? [hint && `${name}-hint`, errorMessage && `${name}-error`]
-            .filter(Boolean)
-            .join(" ")
-        : undefined,
-    [errorMessage, hint, name]
+  const renderFieldComponent = useCallback(
+    ({ ariaDescribedBy, id }) => (
+      <MuiFormGroup aria-describedby={ariaDescribedBy} id={id}>
+        {children}
+      </MuiFormGroup>
+    ),
+    [children]
   );
 
   return (
-    <FormControl
-      component="fieldset"
-      disabled={isDisabled}
-      error={Boolean(errorMessage)}
-      name={name}
-    >
-      {label && <FormLabel component="legend">{label}</FormLabel>}
-      {hint && <FormHelperText id={`${name}-hint`}>{hint}</FormHelperText>}
-      <FormGroup aria-describedby={ariaDescribedBy}>{children}</FormGroup>
-      {errorMessage && (
-        <FormHelperText id={`${name}-error`} error>
-          <ScreenReaderText>Error:</ScreenReaderText> {errorMessage}
-        </FormHelperText>
-      )}
-    </FormControl>
+    <Field
+      errorMessage={errorMessage}
+      fieldType="group"
+      hasVisibleLabel={false}
+      hint={hint}
+      id={idOverride}
+      isDisabled={isDisabled}
+      label={label}
+      renderFieldComponent={renderFieldComponent}
+    />
   );
 };
 

diff --git a/packages/odyssey-react-mui/src/Field.tsx b/packages/odyssey-react-mui/src/Field.tsx
@@ -13,18 +13,23 @@
 import { memo, ReactElement, useMemo } from "react";
 
 import {
-  FieldError,
-  FieldHint,
-  FieldLabel,
-  FormControl,
-  useUniqueId,
-} from "./";
+  FormControl as MuiFormControl,
+  FormLabel as MuiFormLabel,
+} from "@mui/material";
+import { FieldError } from "./FieldError";
+import { FieldHint } from "./FieldHint";
+import { FieldLabel } from "./FieldLabel";
+import { useUniqueId } from "./useUniqueId";
 
 export type FieldProps = {
   /**
    * If `error` is not undefined, the `input` will indicate an error.
    */
   errorMessage?: string;
+  /**
+   * The field type determines how ARIA components are setup. It's important to use this to denote if you expect only one component (like a text field) or multiple (like a radio group).
+   */
+  fieldType: "single" | "group";
   hasVisibleLabel: boolean;
   /**
    * The helper text content.
@@ -34,6 +39,10 @@ export type FieldProps = {
    * The id of the `input` element.
    */
   id?: string;
+  /**
+   * Important for narrowing down the `fieldset` role to "radiogroup".
+   */
+  isRadioGroup?: boolean;
   /**
    * If `true`, the component is disabled.
    */
@@ -54,6 +63,9 @@ export type FieldProps = {
    * The short hint displayed in the `input` before the user enters a value.
    */
   placeholder?: string;
+  /**
+   * Render-props function that sends back ARIA props to your field component.
+   */
   renderFieldComponent: ({
     ariaDescribedBy,
     id,
@@ -65,10 +77,12 @@ export type FieldProps = {
 
 const Field = ({
   errorMessage,
+  fieldType,
   hasVisibleLabel,
   hint,
   id: idOverride,
   isDisabled = false,
+  isRadioGroup = false,
   isOptional = false,
   label,
   optionalLabel,
@@ -85,15 +99,24 @@ const Field = ({
   );
 
   return (
-    <FormControl disabled={isDisabled} error={Boolean(errorMessage)}>
-      <FieldLabel
-        hasVisibleLabel={hasVisibleLabel}
-        id={labelId}
-        inputId={id}
-        isOptional={isOptional}
-        optionalText={optionalLabel}
-        text={label}
-      />
+    <MuiFormControl
+      component={fieldType === "group" ? "fieldset" : "div"}
+      disabled={isDisabled}
+      error={Boolean(errorMessage)}
+      role={isRadioGroup ? "radiogroup" : undefined}
+    >
+      {fieldType === "group" ? (
+        <MuiFormLabel component="legend">{label}</MuiFormLabel>
+      ) : (
+        <FieldLabel
+          hasVisibleLabel={hasVisibleLabel}
+          id={labelId}
+          inputId={id}
+          isOptional={isOptional}
+          optionalText={optionalLabel}
+          text={label}
+        />
+      )}
 
       {hint && <FieldHint id={hintId} text={hint} />}
 
@@ -103,7 +126,7 @@ const Field = ({
       })}
 
       {errorMessage && <FieldError id={errorId} text={errorMessage} />}
-    </FormControl>
+    </MuiFormControl>
   );
 };
 

diff --git a/packages/odyssey-react-mui/src/FieldLabel.tsx b/packages/odyssey-react-mui/src/FieldLabel.tsx
@@ -10,7 +10,7 @@
  * See the License for the specific language governing permissions and limitations under the License.
  */
 
-import { InputLabel } from "@mui/material";
+import { InputLabel as MuiInputLabel } from "@mui/material";
 import { memo, useMemo } from "react";
 
 import { ScreenReaderText } from "./ScreenReaderText";
@@ -35,12 +35,12 @@ const FieldLabel = ({
 }: FieldLabelProps) => {
   const inputLabel = useMemo(
     () => (
-      <InputLabel htmlFor={inputId} id={id}>
+      <MuiInputLabel htmlFor={inputId} id={id}>
         {text}
         {isOptional && (
           <Typography variant="subtitle1">{optionalText}</Typography>
         )}
-      </InputLabel>
+      </MuiInputLabel>
     ),
     [id, inputId, isOptional, optionalText, text]
   );

diff --git a/packages/odyssey-react-mui/src/PasswordField.tsx b/packages/odyssey-react-mui/src/PasswordField.tsx
@@ -162,6 +162,7 @@ const PasswordField = forwardRef<HTMLInputElement, PasswordFieldProps>(
     return (
       <Field
         errorMessage={errorMessage}
+        fieldType="single"
         hasVisibleLabel
         hint={hint}
         id={idOverride}

diff --git a/packages/odyssey-react-mui/src/RadioGroup.tsx b/packages/odyssey-react-mui/src/RadioGroup.tsx
@@ -11,17 +11,10 @@
  */
 
 import { RadioGroup as MuiRadioGroup } from "@mui/material";
-import { ChangeEventHandler, memo, ReactElement, useMemo } from "react";
+import { ChangeEventHandler, memo, ReactElement, useCallback } from "react";
 
-import {
-  FormControl,
-  FormLabel,
-  FormHelperText,
-  Radio,
-  ScreenReaderText,
-  useUniqueId,
-  RadioProps,
-} from ".";
+import { Radio, RadioProps } from "./Radio";
+import { Field } from "./Field";
 
 export type RadioGroupProps = {
   /**
@@ -40,6 +33,10 @@ export type RadioGroupProps = {
    * Optional hint text
    */
   hint?: string;
+  /**
+   * The id of the `input` element. This will also be the input's `name` field.
+   */
+  id?: string;
   /**
    * Disables the whole radio group
    */
@@ -48,10 +45,6 @@ export type RadioGroupProps = {
    * The text label for the radio group
    */
   label: string;
-  /**
-   * The name of the radio group, which only needs to be changed if there are multiple radio groups on the same screen
-   */
-  name?: string;
   /**
    * Listen for changes in the browser that change `value`.
    */
@@ -67,47 +60,39 @@ const RadioGroup = ({
   defaultValue,
   errorMessage,
   hint,
+  id: idOverride,
   isDisabled,
   label,
-  name,
   onChange,
+  value,
 }: RadioGroupProps) => {
-  const ariaDescribedBy = useMemo(
-    () =>
-      errorMessage || hint
-        ? [hint && `${name}-hint`, errorMessage && `${name}-error`]
-            .filter(Boolean)
-            .join(" ")
-        : undefined,
-    [errorMessage, hint, name]
-  );
-
-  const uniqueName = useUniqueId(name);
-
-  return (
-    <FormControl
-      component="fieldset"
-      disabled={isDisabled}
-      error={Boolean(errorMessage)}
-    >
-      <FormLabel component="legend">{label}</FormLabel>
-      {hint && (
-        <FormHelperText id={`${uniqueName}-hint`}>{hint}</FormHelperText>
-      )}
+  const renderFieldComponent = useCallback(
+    ({ ariaDescribedBy, id }) => (
       <MuiRadioGroup
         aria-describedby={ariaDescribedBy}
         defaultValue={defaultValue}
-        name={uniqueName}
+        id={id}
+        name={id}
         onChange={onChange}
+        value={value}
       >
         {children}
       </MuiRadioGroup>
-      {errorMessage && (
-        <FormHelperText id={`${uniqueName}-error`} error>
-          <ScreenReaderText>Error:</ScreenReaderText> {errorMessage}
-        </FormHelperText>
-      )}
-    </FormControl>
+    ),
+    [children, defaultValue, onChange, value]
+  );
+
+  return (
+    <Field
+      errorMessage={errorMessage}
+      fieldType="group"
+      hasVisibleLabel={false}
+      hint={hint}
+      id={idOverride}
+      isDisabled={isDisabled}
+      label={label}
+      renderFieldComponent={renderFieldComponent}
+    />
   );
 };
 

diff --git a/packages/odyssey-react-mui/src/SearchField.tsx b/packages/odyssey-react-mui/src/SearchField.tsx
@@ -121,6 +121,7 @@ const SearchField = forwardRef<HTMLInputElement, SearchFieldProps>(
 
     return (
       <Field
+        fieldType="single"
         hasVisibleLabel={false}
         id={idOverride}
         isDisabled={isDisabled}

diff --git a/packages/odyssey-react-mui/src/TextField.tsx b/packages/odyssey-react-mui/src/TextField.tsx
@@ -176,6 +176,7 @@ const TextField = forwardRef<HTMLInputElement, TextFieldProps>(
     return (
       <Field
         errorMessage={errorMessage}
+        fieldType="single"
         hasVisibleLabel
         hint={hint}
         id={idOverride}

diff --git a/packages/odyssey-storybook/src/components/odyssey-mui/Form/Form.stories.tsx b/packages/odyssey-storybook/src/components/odyssey-mui/Form/Form.stories.tsx
@@ -222,7 +222,7 @@ const Template: Story = (args) => {
             <FormLabel component="legend">Destination</FormLabel>
             <RadioGroup
               defaultValue="Lightspeed"
-              name="radio-buttons-group"
+              id="radio-buttons-group"
               label="Speed"
               aria-describedby="radio-hint radio-error"
             >