From 0e2b53b740a5ed852521705ccfe0e1d83e9aab23 Mon Sep 17 00:00:00 2001
From: Josh Black <josh@josh.black>
Date: Wed, 7 Jul 2021 08:25:07 -0500
Subject: [PATCH] fix(react): update radio button group state initialization
 (#9104)

* fix(react): update radio button group state initialization

* test(react): add test for RadioButtonGroup

* feat(jest-config): add jest-dom as default matcher

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
---
 .../jest-config-carbon/setup/setupAfterEnv.js |   2 +
 .../Accordion/__tests__/Accordion-test.js     |   1 -
 .../src/components/Button/Button-test.js      |   1 -
 .../src/components/Checkbox/Checkbox-test.js  |   1 -
 .../react/src/components/Link/Link-test.js    |   1 -
 .../Notification/next/Notification-test.js    |   1 -
 .../RadioButtonGroup/RadioButtonGroup-test.js | 301 +++++++++++-------
 .../RadioButtonGroup/RadioButtonGroup.js      |  14 +-
 .../components/TabContent/TabContent-test.js  |   1 -
 9 files changed, 197 insertions(+), 126 deletions(-)

diff --git a/config/jest-config-carbon/setup/setupAfterEnv.js b/config/jest-config-carbon/setup/setupAfterEnv.js
index ee334a8f3814..748b6a4e2305 100644
--- a/config/jest-config-carbon/setup/setupAfterEnv.js
+++ b/config/jest-config-carbon/setup/setupAfterEnv.js
@@ -10,6 +10,8 @@ const util = require('util');
 const toHaveNoAxeViolations = require('../matchers/toHaveNoAxeViolations');
 const toHaveNoACViolations = require('../matchers/toHaveNoACViolations');
 
+require('@testing-library/jest-dom');
+
 // We can extend `expect` using custom matchers as defined by:
 // https://jest-bot.github.io/jest/docs/expect.html#expectextendmatchers
 //
diff --git a/packages/react/src/components/Accordion/__tests__/Accordion-test.js b/packages/react/src/components/Accordion/__tests__/Accordion-test.js
index df2f4a2d02b5..58db60bc2eea 100644
--- a/packages/react/src/components/Accordion/__tests__/Accordion-test.js
+++ b/packages/react/src/components/Accordion/__tests__/Accordion-test.js
@@ -9,7 +9,6 @@ import React from 'react';
 import { default as Accordion, AccordionItem } from '../';
 import { cleanup, render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import '@testing-library/jest-dom';
 
 describe('Accordion', () => {
   afterEach(cleanup);
diff --git a/packages/react/src/components/Button/Button-test.js b/packages/react/src/components/Button/Button-test.js
index cc17b1e7bbe3..d073f9c0b5f0 100644
--- a/packages/react/src/components/Button/Button-test.js
+++ b/packages/react/src/components/Button/Button-test.js
@@ -14,7 +14,6 @@ import { shallow, mount } from 'enzyme';
 import { settings } from 'carbon-components';
 import { cleanup, render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import '@testing-library/jest-dom';
 
 const { prefix } = settings;
 
diff --git a/packages/react/src/components/Checkbox/Checkbox-test.js b/packages/react/src/components/Checkbox/Checkbox-test.js
index 207ac66339d0..e4cb628aecf3 100644
--- a/packages/react/src/components/Checkbox/Checkbox-test.js
+++ b/packages/react/src/components/Checkbox/Checkbox-test.js
@@ -12,7 +12,6 @@ import { mount } from 'enzyme';
 import { settings } from 'carbon-components';
 import { cleanup, render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import '@testing-library/jest-dom';
 
 const { prefix } = settings;
 
diff --git a/packages/react/src/components/Link/Link-test.js b/packages/react/src/components/Link/Link-test.js
index 173ad112da4e..f78f5bb7c892 100644
--- a/packages/react/src/components/Link/Link-test.js
+++ b/packages/react/src/components/Link/Link-test.js
@@ -10,7 +10,6 @@ import Link from '../Link';
 import { shallow } from 'enzyme';
 import { settings } from 'carbon-components';
 import { cleanup, render, screen } from '@testing-library/react';
-import '@testing-library/jest-dom';
 import userEvent from '@testing-library/user-event';
 import { describe, document } from 'window-or-global';
 
diff --git a/packages/react/src/components/Notification/next/Notification-test.js b/packages/react/src/components/Notification/next/Notification-test.js
index 263cf7873b69..5ecbecdd59dd 100644
--- a/packages/react/src/components/Notification/next/Notification-test.js
+++ b/packages/react/src/components/Notification/next/Notification-test.js
@@ -15,7 +15,6 @@ import {
 } from '../next/Notification';
 import { render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import '@testing-library/jest-dom';
 import { settings } from 'carbon-components';
 import { it } from 'window-or-global';
 
diff --git a/packages/react/src/components/RadioButtonGroup/RadioButtonGroup-test.js b/packages/react/src/components/RadioButtonGroup/RadioButtonGroup-test.js
index d3a1f8047c49..44a3720afd81 100644
--- a/packages/react/src/components/RadioButtonGroup/RadioButtonGroup-test.js
+++ b/packages/react/src/components/RadioButtonGroup/RadioButtonGroup-test.js
@@ -5,155 +5,222 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import { render, screen } from '@testing-library/react';
 import React from 'react';
 import { shallow, mount } from 'enzyme';
 import RadioButtonGroup from '../RadioButtonGroup';
 import RadioButton from '../RadioButton';
-import { settings } from 'carbon-components';
-
-const { prefix } = settings;
 
 describe('RadioButtonGroup', () => {
-  describe('renders as expected', () => {
-    const wrapper = mount(
-      <RadioButtonGroup
-        defaultSelected="female"
-        name="gender"
-        legendText="Radio legend">
-        <RadioButton labelText="Male" value="male" />
-        <RadioButton labelText="Female" value="female" />
+  it('should render `legendText` in a <label>', () => {
+    render(
+      <RadioButtonGroup defaultSelected="test-1" name="test" legendText="test">
+        <RadioButton labelText="test-1" value="test-1" />
+        <RadioButton labelText="test-2" value="test-2" />
       </RadioButtonGroup>
     );
 
-    describe('wrapping fieldset', () => {
-      const fieldset = wrapper.find('fieldset');
-      const legend = wrapper.find('legend');
-
-      it('renders a fieldset', () => {
-        expect(fieldset.length).toEqual(1);
-      });
-
-      it('renders a legend if legendText is provided', () => {
-        expect(legend.length).toEqual(1);
-      });
-
-      it('sets classes that are passed via className prop', () => {
-        wrapper.setProps({ className: 'extra-class' });
-        expect(wrapper.find('fieldset').hasClass('extra-class')).toBe(true);
-      });
-
-      it('sets disabled attribute if disabled prop is set', () => {
-        wrapper.setProps({ disabled: true });
-        expect(wrapper.first().props().disabled).toEqual(true);
-      });
+    const legend = screen.getByText('test', {
+      selector: 'legend',
     });
+    expect(legend).toBeDefined();
+  });
 
-    describe('children', () => {
-      const radioButton = () => wrapper.find(RadioButton);
-
-      it('renders expected number of children', () => {
-        expect(radioButton().length).toEqual(2);
-      });
-
-      it('should set checked property based on defaultSelected prop', () => {
-        expect(radioButton().last().props().checked).toEqual(true);
-      });
-
-      it('should set checked property based on valueSelected prop', () => {
-        wrapper.setProps({ valueSelected: 'male' });
-        expect(radioButton().first().props().checked).toEqual(true);
-        wrapper.setProps({ valueSelected: 'female' });
-        expect(radioButton().last().props().checked).toEqual(true);
-      });
+  it('should render `legendText` in a <fieldset>', () => {
+    render(
+      <RadioButtonGroup defaultSelected="test-1" name="test" legendText="test">
+        <RadioButton labelText="test-1" value="test-1" />
+        <RadioButton labelText="test-2" value="test-2" />
+      </RadioButtonGroup>
+    );
 
-      it('should set expected props on children', () => {
-        const firstChild = radioButton().first();
-        expect(firstChild.props().name).toEqual('gender');
-        expect(firstChild.props().value).toEqual('male');
-      });
-    });
+    const fieldset = screen
+      .getByText('test', {
+        selector: 'legend',
+      })
+      .closest('fieldset');
+    expect(fieldset).toBeDefined();
   });
 
-  describe('onChange event', () => {
-    const onChange = jest.fn();
-    const wrapper = mount(
-      <RadioButtonGroup onChange={onChange} name="gender">
-        <RadioButton labelText="Male" value="male" />
-        <RadioButton labelText="Female" value="female" />
+  it('should render <RadioButton> as children', () => {
+    render(
+      <RadioButtonGroup defaultSelected="test-1" name="test" legendText="test">
+        <RadioButton labelText="test-1" value="test-1" />
+        <RadioButton labelText="test-2" value="test-2" />
       </RadioButtonGroup>
     );
 
-    const firstRadio = wrapper.find(RadioButton).first();
-    const args = ['male', 'gender', { test: 'test event' }];
+    const fieldset = screen
+      .getByText('test', {
+        selector: 'legend',
+      })
+      .closest('fieldset');
+    expect(fieldset).toContainElement(screen.getByLabelText('test-1'));
+    expect(fieldset).toContainElement(screen.getByLabelText('test-2'));
+  });
+
+  describe('Component API', () => {
+    it('should support a custom className on the <fieldset>', () => {
+      render(
+        <RadioButtonGroup
+          className="custom-class"
+          defaultSelected="test-1"
+          name="test"
+          legendText="test">
+          <RadioButton labelText="test-1" value="test-1" />
+          <RadioButton labelText="test-2" value="test-2" />
+        </RadioButtonGroup>
+      );
+
+      const fieldset = screen
+        .getByText('test', {
+          selector: 'legend',
+        })
+        .closest('fieldset');
+      expect(fieldset).toHaveClass('custom-class');
+    });
 
-    it('first child should not have checked set initially', () => {
-      expect(firstRadio.props().checked).toEqual(false);
+    it('should support passing in disabled to disable the <fieldset>', () => {
+      render(
+        <RadioButtonGroup
+          defaultSelected="test-1"
+          disabled
+          name="test"
+          legendText="test">
+          <RadioButton labelText="test-1" value="test-1" />
+          <RadioButton labelText="test-2" value="test-2" />
+        </RadioButtonGroup>
+      );
+      const fieldset = screen
+        .getByText('test', {
+          selector: 'legend',
+        })
+        .closest('fieldset');
+      expect(fieldset).toBeDisabled();
     });
 
-    it('invoking onChange sets checked on correct child', () => {
-      firstRadio.props().onChange(...args);
-      wrapper.update();
-      expect(wrapper.find(RadioButton).first().props().checked).toEqual(true);
+    it('should support `defaultSelected` as a way to select a radio button', () => {
+      render(
+        <RadioButtonGroup
+          defaultSelected="test-1"
+          name="test"
+          legendText="test">
+          <RadioButton labelText="test-1" value="test-1" />
+          <RadioButton labelText="test-2" value="test-2" />
+        </RadioButtonGroup>
+      );
+
+      expect(screen.getByLabelText('test-1')).toEqual(
+        screen.getByRole('radio', {
+          checked: true,
+        })
+      );
     });
 
-    it('should invoke onChange with correct arguments', () => {
-      expect(onChange).toHaveBeenCalledWith(...args);
+    it('should support `valueSelected` as a way to select a radio button', () => {
+      const { rerender } = render(
+        <RadioButtonGroup valueSelected="test-1" name="test" legendText="test">
+          <RadioButton labelText="test-1" value="test-1" />
+          <RadioButton labelText="test-2" value="test-2" />
+        </RadioButtonGroup>
+      );
+
+      expect(screen.getByLabelText('test-1')).toEqual(
+        screen.getByRole('radio', {
+          checked: true,
+        })
+      );
+
+      rerender(
+        <RadioButtonGroup valueSelected="test-2" name="test" legendText="test">
+          <RadioButton labelText="test-1" value="test-1" />
+          <RadioButton labelText="test-2" value="test-2" />
+        </RadioButtonGroup>
+      );
+
+      expect(screen.getByLabelText('test-2')).toEqual(
+        screen.getByRole('radio', {
+          checked: true,
+        })
+      );
     });
 
-    it('calling onChange with same args should not call onChange prop', () => {
-      onChange.mockClear();
-      firstRadio.props().onChange(...args);
-      expect(onChange).not.toHaveBeenCalled();
+    it('should support a 0 value for `valueSelected` (#9041)', () => {
+      render(
+        <RadioButtonGroup valueSelected={0} name="test" legendText="test">
+          <RadioButton labelText="test-1" value={1} />
+          <RadioButton labelText="test-0" value={0} />
+        </RadioButtonGroup>
+      );
+
+      expect(screen.getByLabelText('test-0')).toEqual(
+        screen.getByRole('radio', {
+          checked: true,
+        })
+      );
     });
-  });
 
-  describe('Getting derived state from props', () => {
-    const wrapper = shallow(
-      <RadioButtonGroup
-        valueSelected="male"
-        defaultSelected="female"
-        name="gender">
-        <RadioButton labelText="Male" value="male" />
-        <RadioButton labelText="Female" value="female" />
-      </RadioButtonGroup>
-    );
+    describe('onChange event', () => {
+      const onChange = jest.fn();
+      const wrapper = mount(
+        <RadioButtonGroup onChange={onChange} name="gender">
+          <RadioButton labelText="Male" value="male" />
+          <RadioButton labelText="Female" value="female" />
+        </RadioButtonGroup>
+      );
 
-    it('should initialize the current selection from props', () => {
-      expect(wrapper.state().selected).toEqual('male');
-    });
+      const firstRadio = wrapper.find(RadioButton).first();
+      const args = ['male', 'gender', { test: 'test event' }];
 
-    it('should change the current selection upon change in props', () => {
-      wrapper.setProps({ valueSelected: 'male' });
-      wrapper.setState({ selected: 'male' });
-      wrapper.setProps({ valueSelected: undefined });
-      expect(wrapper.state().selected).toEqual('female');
-    });
+      it('first child should not have checked set initially', () => {
+        expect(firstRadio.props().checked).toEqual(false);
+      });
+
+      it('invoking onChange sets checked on correct child', () => {
+        firstRadio.props().onChange(...args);
+        wrapper.update();
+        expect(wrapper.find(RadioButton).first().props().checked).toEqual(true);
+      });
+
+      it('should invoke onChange with correct arguments', () => {
+        expect(onChange).toHaveBeenCalledWith(...args);
+      });
 
-    it('should avoid change the current selection upon setting props, unless there the value actually changes', () => {
-      wrapper.setProps({ valueSelected: 'female' });
-      wrapper.setState({ selected: 'male' });
-      wrapper.setProps({ valueSelected: 'female' });
-      expect(wrapper.state().selected).toEqual('male');
+      it('calling onChange with same args should not call onChange prop', () => {
+        onChange.mockClear();
+        firstRadio.props().onChange(...args);
+        expect(onChange).not.toHaveBeenCalled();
+      });
     });
-  });
 
-  describe('Custom class name should stay with original class name', () => {
-    const wrapper = shallow(
-      <RadioButtonGroup
-        className="my-radio-group"
-        valueSelected="male"
-        defaultSelected="female"
-        name="gender">
-        <RadioButton labelText="Male" value="male" />
-        <RadioButton labelText="Female" value="female" />
-      </RadioButtonGroup>
-    );
+    describe('Getting derived state from props', () => {
+      const wrapper = shallow(
+        <RadioButtonGroup
+          valueSelected="male"
+          defaultSelected="female"
+          name="gender">
+          <RadioButton labelText="Male" value="male" />
+          <RadioButton labelText="Female" value="female" />
+        </RadioButtonGroup>
+      );
+
+      it('should initialize the current selection from props', () => {
+        expect(wrapper.state().selected).toEqual('male');
+      });
 
-    it('should found the provided class along with the base class', () => {
-      expect(wrapper.exists('.my-radio-group')).toBe(true);
-      expect(
-        wrapper.exists(`.${prefix}--radio-button-group.my-radio-group`)
-      ).toBe(true);
+      it('should change the current selection upon change in props', () => {
+        wrapper.setProps({ valueSelected: 'male' });
+        wrapper.setState({ selected: 'male' });
+        wrapper.setProps({ valueSelected: undefined });
+        expect(wrapper.state().selected).toEqual('female');
+      });
+
+      it('should avoid change the current selection upon setting props, unless there the value actually changes', () => {
+        wrapper.setProps({ valueSelected: 'female' });
+        wrapper.setState({ selected: 'male' });
+        wrapper.setProps({ valueSelected: 'female' });
+        expect(wrapper.state().selected).toEqual('male');
+      });
     });
   });
 });
diff --git a/packages/react/src/components/RadioButtonGroup/RadioButtonGroup.js b/packages/react/src/components/RadioButtonGroup/RadioButtonGroup.js
index a65bd3450aa4..f514822fe1c9 100644
--- a/packages/react/src/components/RadioButtonGroup/RadioButtonGroup.js
+++ b/packages/react/src/components/RadioButtonGroup/RadioButtonGroup.js
@@ -14,8 +14,6 @@ import { settings } from 'carbon-components';
 const { prefix } = settings;
 
 export default class RadioButtonGroup extends React.Component {
-  state = { selected: this.props.valueSelected || this.props.defaultSelected };
-
   static propTypes = {
     /**
      * Provide a collection of <RadioButton> components to render in the group
@@ -81,11 +79,21 @@ export default class RadioButtonGroup extends React.Component {
     return prevValueSelected === valueSelected
       ? null
       : {
-          selected: valueSelected || defaultSelected,
+          selected:
+            typeof valueSelected !== 'undefined'
+              ? valueSelected
+              : defaultSelected,
           prevValueSelected: valueSelected,
         };
   }
 
+  state = {
+    selected:
+      typeof this.props.valueSelected !== 'undefined'
+        ? this.props.valueSelected
+        : this.props.defaultSelected,
+  };
+
   getRadioButtons = () => {
     const children = React.Children.map(this.props.children, (radioButton) => {
       const { value } = radioButton.props;
diff --git a/packages/react/src/components/TabContent/TabContent-test.js b/packages/react/src/components/TabContent/TabContent-test.js
index 0ac53779850b..4fb0dd02e2de 100644
--- a/packages/react/src/components/TabContent/TabContent-test.js
+++ b/packages/react/src/components/TabContent/TabContent-test.js
@@ -8,7 +8,6 @@
 import React from 'react';
 import TabContent from '../TabContent';
 import { render, screen } from '@testing-library/react';
-import '@testing-library/jest-dom';
 
 describe('TabContent', () => {
   describe('renders as expected', () => {