From fe40f4a14efb688634fcad35778963ef2f725574 Mon Sep 17 00:00:00 2001
From: adamviktora <84135613+adamviktora@users.noreply.github.com>
Date: Tue, 9 Jan 2024 17:59:38 +0100
Subject: [PATCH] Add support for isLabelWrapped and component in Checkbox /
 Radio (#9830)

* feat(Checkbox): add support for isLabelWrapped and isLabelBeforeButton

* feat(Radio): add support for component

* refactor(Radio/Checkbox): check isLabelBeforeButton only once

* feat(Radio/Checkbox): add support for component === "label" behaving the same as isLabelWrapped

* docs(Radio/Checkbox): update props description

* test(Checkbox): add unit tests

* test(Checkbox): unit test

* test(Radio): unit tests
---
 .../src/components/Checkbox/Checkbox.tsx      | 92 +++++++++++++------
 .../Checkbox/__tests__/Checkbox.test.tsx      | 41 +++++++++
 .../react-core/src/components/Radio/Radio.tsx | 77 ++++++++--------
 .../components/Radio/__tests__/Radio.test.tsx | 41 +++++++++
 4 files changed, 184 insertions(+), 67 deletions(-)

diff --git a/packages/react-core/src/components/Checkbox/Checkbox.tsx b/packages/react-core/src/components/Checkbox/Checkbox.tsx
index 699bbe3d3ff..93b9816e4c0 100644
--- a/packages/react-core/src/components/Checkbox/Checkbox.tsx
+++ b/packages/react-core/src/components/Checkbox/Checkbox.tsx
@@ -8,10 +8,16 @@ import { ASTERISK } from '../../helpers/htmlConstants';
 export interface CheckboxProps
   extends Omit<React.HTMLProps<HTMLInputElement>, 'type' | 'onChange' | 'disabled' | 'label'>,
     OUIAProps {
-  /** Additional classes added to the checkbox. */
+  /** Additional classes added to the checkbox wrapper. This wrapper will be div element by default. It will be a label element if
+   * isLabelWrapped is true, or it can be overridden by any element specified in the component prop.
+   */
   className?: string;
-  /** Additional classed added to the radio input */
+  /** Additional classes added to the checkbox input. */
   inputClassName?: string;
+  /** Flag to indicate whether the checkbox wrapper element is a <label> element for the checkbox input. Will not apply if a component prop (with a value other than a "label") is specified. */
+  isLabelWrapped?: boolean;
+  /** Flag to show if the checkbox label is shown before the checkbox input. */
+  isLabelBeforeButton?: boolean;
   /** Flag to show if the checkbox selection is valid or invalid. */
   isValid?: boolean;
   /** Flag to show if the checkbox is disabled. */
@@ -33,7 +39,7 @@ export interface CheckboxProps
   description?: React.ReactNode;
   /** Body text of the checkbox */
   body?: React.ReactNode;
-  /** Sets the input wrapper component to render. Defaults to <div> */
+  /** Sets the checkbox wrapper component to render. Defaults to "div". If set to "label", behaves the same as if isLabelWrapped prop was specified. */
   component?: React.ElementType;
   /** Value to overwrite the randomly generated data-ouia-component-id.*/
   ouiaId?: number | string;
@@ -52,13 +58,13 @@ class Checkbox extends React.Component<CheckboxProps, CheckboxState> {
   static displayName = 'Checkbox';
   static defaultProps: PickOptional<CheckboxProps> = {
     className: '',
+    isLabelWrapped: false,
     isValid: true,
     isDisabled: false,
     isRequired: false,
     isChecked: false,
     onChange: defaultOnChange,
-    ouiaSafe: true,
-    component: 'div'
+    ouiaSafe: true
   };
 
   constructor(props: any) {
@@ -78,6 +84,8 @@ class Checkbox extends React.Component<CheckboxProps, CheckboxState> {
       className,
       inputClassName,
       onChange,
+      isLabelWrapped,
+      isLabelBeforeButton,
       isValid,
       isDisabled,
       isRequired,
@@ -89,7 +97,7 @@ class Checkbox extends React.Component<CheckboxProps, CheckboxState> {
       body,
       ouiaId,
       ouiaSafe,
-      component: Component,
+      component,
       ...props
     } = this.props;
     if (!props.id) {
@@ -107,31 +115,57 @@ class Checkbox extends React.Component<CheckboxProps, CheckboxState> {
       checkedProps.defaultChecked = defaultChecked;
     }
 
+    const inputRendered = (
+      <input
+        {...props}
+        className={css(styles.checkInput, inputClassName)}
+        type="checkbox"
+        onChange={this.handleChange}
+        aria-invalid={!isValid}
+        aria-label={ariaLabel}
+        disabled={isDisabled}
+        required={isRequired}
+        ref={(elem) => elem && (elem.indeterminate = isChecked === null)}
+        {...checkedProps}
+        {...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)}
+      />
+    );
+
+    const wrapWithLabel = (isLabelWrapped && !component) || component === 'label';
+
+    const Label = wrapWithLabel ? 'span' : 'label';
+    const labelRendered = label ? (
+      <Label
+        className={css(styles.checkLabel, isDisabled && styles.modifiers.disabled)}
+        htmlFor={!wrapWithLabel && props.id}
+      >
+        {label}
+        {isRequired && (
+          <span className={css(styles.checkLabelRequired)} aria-hidden="true">
+            {ASTERISK}
+          </span>
+        )}
+      </Label>
+    ) : null;
+
+    const Component = component ?? (wrapWithLabel ? 'label' : 'div');
+
     checkedProps.checked = checkedProps.checked === null ? false : checkedProps.checked;
     return (
-      <Component className={css(styles.check, !label && styles.modifiers.standalone, className)}>
-        <input
-          {...props}
-          className={css(styles.checkInput, inputClassName)}
-          type="checkbox"
-          onChange={this.handleChange}
-          aria-invalid={!isValid}
-          aria-label={ariaLabel}
-          disabled={isDisabled}
-          required={isRequired}
-          ref={(elem) => elem && (elem.indeterminate = isChecked === null)}
-          {...checkedProps}
-          {...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)}
-        />
-        {label && (
-          <label className={css(styles.checkLabel, isDisabled && styles.modifiers.disabled)} htmlFor={props.id}>
-            {label}
-            {isRequired && (
-              <span className={css(styles.checkLabelRequired)} aria-hidden="true">
-                {ASTERISK}
-              </span>
-            )}
-          </label>
+      <Component
+        className={css(styles.check, !label && styles.modifiers.standalone, className)}
+        htmlFor={wrapWithLabel && props.id}
+      >
+        {isLabelBeforeButton ? (
+          <>
+            {labelRendered}
+            {inputRendered}
+          </>
+        ) : (
+          <>
+            {inputRendered}
+            {labelRendered}
+          </>
         )}
         {description && <span className={css(styles.checkDescription)}>{description}</span>}
         {body && <span className={css(styles.checkBody)}>{body}</span>}
diff --git a/packages/react-core/src/components/Checkbox/__tests__/Checkbox.test.tsx b/packages/react-core/src/components/Checkbox/__tests__/Checkbox.test.tsx
index 38fdb71b497..aa3948f77f4 100644
--- a/packages/react-core/src/components/Checkbox/__tests__/Checkbox.test.tsx
+++ b/packages/react-core/src/components/Checkbox/__tests__/Checkbox.test.tsx
@@ -231,6 +231,47 @@ test('Renders with the provided component', () => {
   expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('SPAN');
 });
 
+test('Renders with the label wrapper if isLabelWrapped is provided', () => {
+  render(<Checkbox id="test-id" isLabelWrapped />);
+
+  expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('LABEL');
+});
+
+test('Renders with span element around the inner label text if isLabelWrapped is provided', () => {
+  const labelText = "test checkbox label";
+  render(<Checkbox id="test-id" isLabelWrapped label={labelText} />);
+
+  expect(screen.getByText(labelText).tagName).toBe('SPAN');
+});
+
+test('Renders with the provided component although isLabelWrapped is provided', () => {
+  render(<Checkbox id="test-id" isLabelWrapped component="h3" />);
+
+  expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('H3');
+});
+
+test('Renders with the label wrapper if component is set to label', () => {
+  render(<Checkbox id="test-id" component="label" />);
+
+  expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('LABEL');
+});
+
+test('Renders with span element around the inner label text if component is set to label', () => {
+  const labelText = "test checkbox label";
+  render(<Checkbox id="test-id" component="label" label={labelText} />);
+
+  expect(screen.getByText(labelText).tagName).toBe('SPAN');
+});
+
+test('Renders label before checkbox input if isLabelBeforeButton is provided', () => {
+  render(<Checkbox id="test-id" isLabelBeforeButton label={"test checkbox label"} />);
+
+  const wrapper = screen.getByRole('checkbox').parentElement!;
+
+  expect(wrapper.children[0].tagName).toBe('LABEL');
+  expect(wrapper.children[1].tagName).toBe('INPUT');
+});
+
 test(`Spreads additional props`, () => {
   render(<Checkbox id="test-id" data-testid="test-id" />);
 
diff --git a/packages/react-core/src/components/Radio/Radio.tsx b/packages/react-core/src/components/Radio/Radio.tsx
index c0fb12479c2..b02675c4b22 100644
--- a/packages/react-core/src/components/Radio/Radio.tsx
+++ b/packages/react-core/src/components/Radio/Radio.tsx
@@ -7,17 +7,17 @@ import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers';
 export interface RadioProps
   extends Omit<React.HTMLProps<HTMLInputElement>, 'disabled' | 'label' | 'onChange' | 'type'>,
     OUIAProps {
-  /** Additional classes added to the radio wrapper.  This will be a div element if
-   * isLabelWrapped is true, otherwise this will be a label element.
+  /** Additional classes added to the radio wrapper. This wrapper will be div element by default. It will be a label element if
+   * isLabelWrapped is true, or it can be overridden by any element specified in the component prop.
    */
   className?: string;
-  /** Additional classed added to the radio input */
+  /** Additional classes added to the radio input. */
   inputClassName?: string;
   /** Id of the radio. */
   id: string;
-  /** Flag to show if the radio label is wrapped on small screen. */
+  /** Flag to indicate whether the radio wrapper element is a native label element for the radio input. Will not apply if a component prop (with a value other than a "label") is specified. */
   isLabelWrapped?: boolean;
-  /** Flag to show if the radio label is shown before the radio button. */
+  /** Flag to show if the radio label is shown before the radio input. */
   isLabelBeforeButton?: boolean;
   /** Flag to show if the radio is checked. */
   checked?: boolean;
@@ -39,6 +39,8 @@ export interface RadioProps
   description?: React.ReactNode;
   /** Body of the radio. */
   body?: React.ReactNode;
+  /** Sets the radio wrapper component to render. Defaults to "div". If set to "label", behaves the same as if isLabelWrapped prop was specified. */
+  component?: React.ElementType;
   /** Value to overwrite the randomly generated data-ouia-component-id.*/
   ouiaId?: number | string;
   /** Set the value of data-ouia-safe. Only set to true when the component is in a static state, i.e. no animations are occurring. At all other times, this value must be false. */
@@ -88,6 +90,7 @@ class Radio extends React.Component<RadioProps, { ouiaStateId: string }> {
       body,
       ouiaId,
       ouiaSafe = true,
+      component,
       ...props
     } = this.props;
     if (!props.id) {
@@ -110,41 +113,39 @@ class Radio extends React.Component<RadioProps, { ouiaStateId: string }> {
       />
     );
 
-    let labelRendered: React.ReactNode = null;
-    if (label && isLabelWrapped) {
-      labelRendered = <span className={css(styles.radioLabel, isDisabled && styles.modifiers.disabled)}>{label}</span>;
-    } else if (label) {
-      labelRendered = (
-        <label className={css(styles.radioLabel, isDisabled && styles.modifiers.disabled)} htmlFor={props.id}>
-          {label}
-        </label>
-      );
-    }
+    const wrapWithLabel = (isLabelWrapped && !component) || component === 'label';
 
-    const descRender = description ? <span className={css(styles.radioDescription)}>{description}</span> : null;
-    const bodyRender = body ? <span className={css(styles.radioBody)}>{body}</span> : null;
-    const childrenRendered = isLabelBeforeButton ? (
-      <>
-        {labelRendered}
-        {inputRendered}
-        {descRender}
-        {bodyRender}
-      </>
-    ) : (
-      <>
-        {inputRendered}
-        {labelRendered}
-        {descRender}
-        {bodyRender}
-      </>
-    );
+    const Label = wrapWithLabel ? 'span' : 'label';
+    const labelRendered = label ? (
+      <Label
+        className={css(styles.radioLabel, isDisabled && styles.modifiers.disabled)}
+        htmlFor={!wrapWithLabel && props.id}
+      >
+        {label}
+      </Label>
+    ) : null;
+
+    const Component = component ?? (wrapWithLabel ? 'label' : 'div');
 
-    return isLabelWrapped ? (
-      <label className={css(styles.radio, className)} htmlFor={props.id}>
-        {childrenRendered}
-      </label>
-    ) : (
-      <div className={css(styles.radio, !label && styles.modifiers.standalone, className)}>{childrenRendered}</div>
+    return (
+      <Component
+        className={css(styles.radio, !label && styles.modifiers.standalone, className)}
+        htmlFor={wrapWithLabel && props.id}
+      >
+        {isLabelBeforeButton ? (
+          <>
+            {labelRendered}
+            {inputRendered}
+          </>
+        ) : (
+          <>
+            {inputRendered}
+            {labelRendered}
+          </>
+        )}
+        {description && <span className={css(styles.radioDescription)}>{description}</span>}
+        {body && <span className={css(styles.radioBody)}>{body}</span>}
+      </Component>
     );
   }
 }
diff --git a/packages/react-core/src/components/Radio/__tests__/Radio.test.tsx b/packages/react-core/src/components/Radio/__tests__/Radio.test.tsx
index 28ad8da24ff..7f4167def2e 100644
--- a/packages/react-core/src/components/Radio/__tests__/Radio.test.tsx
+++ b/packages/react-core/src/components/Radio/__tests__/Radio.test.tsx
@@ -100,4 +100,45 @@ describe('Radio', () => {
 
     expect(myMock).toHaveBeenCalled();
   });
+
+  test('Renders with the label wrapper if isLabelWrapped is provided', () => {
+    render(<Radio id="test-id" name="check" isLabelWrapped />);
+
+    expect(screen.getByRole('radio').parentElement?.tagName).toBe('LABEL');
+  });
+
+  test('Renders with span element around the inner label text if isLabelWrapped is provided', () => {
+    const labelText = 'test radio label';
+    render(<Radio id="test-id" name="check" isLabelWrapped label={labelText} />);
+
+    expect(screen.getByText(labelText).tagName).toBe('SPAN');
+  });
+
+  test('Renders with the provided component although isLabelWrapped is provided', () => {
+    render(<Radio id="test-id" name="check" isLabelWrapped component="h3" />);
+
+    expect(screen.getByRole('radio').parentElement?.tagName).toBe('H3');
+  });
+
+  test('Renders with the label wrapper if component is set to label', () => {
+    render(<Radio id="test-id" name="check" component="label" />);
+
+    expect(screen.getByRole('radio').parentElement?.tagName).toBe('LABEL');
+  });
+
+  test('Renders with span element around the inner label text if component is set to label', () => {
+    const labelText = 'test radio label';
+    render(<Radio id="test-id" name="check" component="label" label={labelText} />);
+
+    expect(screen.getByText(labelText).tagName).toBe('SPAN');
+  });
+
+  test('Renders label before radio input if isLabelBeforeButton is provided', () => {
+    render(<Radio id="test-id" name="check" isLabelBeforeButton label={"test radio label"} />);
+  
+    const wrapper = screen.getByRole('radio').parentElement!;
+  
+    expect(wrapper.children[0].tagName).toBe('LABEL');
+    expect(wrapper.children[1].tagName).toBe('INPUT');
+  });
 });