fix(checkbox, radioGroup): add a default id when id prop isn't passed (…

…#2667)

.changeset/wise-cameras-sing.md

@@ -0,0 +1,7 @@
+---
+'@twilio-paste/checkbox': patch
+'@twilio-paste/radio-group': patch
+'@twilio-paste/core': patch
+---
+
+[Checkbox, RadioGroup] When no id is passed, use a unique id. This fixes an issue where id is not marked as a required prop, but the control won't work unless it is passed.

packages/paste-core/components/checkbox/__tests__/checkbox.test.tsx

@@ -32,8 +32,10 @@ const defaultGroupProps = {
 
 describe('Checkbox', () => {
   it('should render', () => {
-    const {getByRole, container} = render(<Checkbox {...defaultProps}>foo</Checkbox>);
-    expect(getByRole('checkbox')).not.toBeNull();
+    const {container} = render(<Checkbox {...defaultProps}>foo</Checkbox>);
+    const checkbox = screen.getByRole('checkbox');
+    expect(checkbox).not.toBeNull();
+    expect(checkbox.id).toBeDefined();
 
     const checkIcon = container.querySelector('[data-paste-element="CHECKBOX_ICON"]');
     expect(checkIcon).toHaveStyleRule('display', 'none');
@@ -48,6 +50,16 @@ describe('Checkbox', () => {
     expect(getByRole('checkbox').getAttribute('aria-invalid')).toBeTruthy();
   });
 
+  it('should use the id prop when passed', () => {
+    render(
+      <Checkbox {...defaultProps} id="my-id" onChange={NOOP}>
+        foo
+      </Checkbox>
+    );
+    const checkbox = screen.getByRole('checkbox');
+    expect(checkbox.id).toBe('my-id');
+  });
+
   it('should render as checked when defaultChecked', () => {
     const {getByLabelText, container} = render(
       <Checkbox {...defaultProps} defaultChecked onChange={NOOP}>

packages/paste-core/components/checkbox/src/Checkbox.tsx

@@ -131,6 +131,7 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
 
     const checkboxGroupContext = React.useContext(CheckboxContext);
     const helpTextId = useUID();
+    const checkboxId = id ? id : useUID();
     // We shouldn't change between controlled and uncontrolled after mount, so we memo this for safety
     const isControlled = React.useMemo(() => checked !== undefined, []);
 
@@ -176,11 +177,11 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
           aria-describedby={helpTextId}
           aria-checked={indeterminate ? 'mixed' : checked}
           aria-invalid={hasError}
-          id={id}
+          id={checkboxId}
           required={required}
           ref={ref}
         />
-        <BaseRadioCheckboxLabel disabled={disabled} htmlFor={id}>
+        <BaseRadioCheckboxLabel disabled={disabled} htmlFor={checkboxId}>
           <BaseRadioCheckboxControl
             alignItems="center"
             borderRadius="borderRadius10"

packages/paste-core/components/checkbox/stories/checkbox.stories.tsx

@@ -44,6 +44,26 @@ CheckboxChecked.story = {
   name: 'Checkbox - Checked',
 };
 
+export const CheckboxWithNoID = (): React.ReactNode => {
+  const [checked, setChecked] = React.useState(true);
+  return (
+    <Checkbox
+      value="1"
+      name="foo"
+      checked={checked}
+      onChange={(event) => {
+        setChecked(event.target.checked);
+      }}
+    >
+      Label
+    </Checkbox>
+  );
+};
+
+CheckboxWithNoID.story = {
+  name: 'Checkbox - With no ID',
+};
+
 export const CheckboxDefaultChecked = (): React.ReactNode => {
   return (
     <Checkbox id={useUID()} value="label" name="foo" defaultChecked>

packages/paste-core/components/radio-group/__tests__/radio.test.tsx

@@ -37,8 +37,22 @@ const defaultGroupProps = {
 
 describe('Radio', () => {
   it('should render', () => {
-    const {getByRole} = render(<Radio {...defaultProps}>foo</Radio>);
-    expect(getByRole('radio')).not.toBeNull();
+    render(
+      <Radio value="foo" name="foo" onChange={NOOP}>
+        foo
+      </Radio>
+    );
+
+    const radio = screen.getByRole('radio');
+    expect(radio).not.toBeNull();
+    expect(radio.id).toBeDefined();
+  });
+
+  it('should use the id prop when passed', () => {
+    render(<Radio {...defaultProps}>foo</Radio>);
+
+    const radio = screen.getByRole('radio');
+    expect(radio.id).toBe('foo');
   });
 
   it('should render as invalid', () => {

packages/paste-core/components/radio-group/src/Radio.tsx

@@ -92,6 +92,7 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
 
     const radioGroupContext = React.useContext(RadioContext);
     const helpTextId = useUID();
+    const radioId = id ? id : useUID();
     // We shouldn't change between controlled and uncontrolled after mount, so we memo this for safety
     const isControlled = React.useMemo(() => checked !== undefined || radioGroupContext.value !== '', []);
 
@@ -144,10 +145,10 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
           aria-describedby={helpTextId}
           aria-invalid={state.hasError}
           onChange={handleChange}
-          id={id}
+          id={radioId}
           ref={ref}
         />
-        <BaseRadioCheckboxLabel disabled={state.disabled} htmlFor={id}>
+        <BaseRadioCheckboxLabel disabled={state.disabled} htmlFor={radioId}>
           <BaseRadioCheckboxControl
             element={`${element}_CONTROL`}
             borderRadius="borderRadiusCircle"

packages/paste-core/components/radio-group/stories/radio-uncontrolled.stories.tsx

@@ -36,6 +36,32 @@ RadioBasic.story = {
   },
 };
 
+/**
+ * No id passed to Radio
+ */
+export const RadioWithNoID = (): React.ReactNode => {
+  return (
+    <>
+      <Radio value="1" name="foo" helpText="This is some help text.">
+        First option
+      </Radio>
+      <Radio value="2" name="foo" helpText="This is some help text.">
+        Second option
+      </Radio>
+      <Radio value="2" name="foo" helpText="This is some help text." disabled>
+        Disabled option
+      </Radio>
+    </>
+  );
+};
+
+RadioWithNoID.story = {
+  name: 'Radio - with no ID',
+  parameters: {
+    chromatic: {disableSnapshot: true},
+  },
+};
+
 /**
  * Only `defaultChecked` passed to Radio
  */