fix: fix state handling for #checkbox, #switch, single #radio button …

…and #toggle-button
dnbexperience · Nov 29, 2020 · 54735aa · 54735aa
1 parent baa4cc0
commit 54735aa
Show file tree

Hide file tree

Showing 12 changed files with 723 additions and 106 deletions.
diff --git a/packages/dnb-ui-lib/src/components/checkbox/Checkbox.js b/packages/dnb-ui-lib/src/components/checkbox/Checkbox.js
@@ -44,7 +44,7 @@ export default class Checkbox extends React.PureComponent {
     ]),
     label_position: PropTypes.oneOf(['left', 'right']),
     title: PropTypes.string,
-    default_state: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
+    default_state: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]), // Deprecated
     checked: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
     disabled: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
     id: PropTypes.string,
@@ -82,8 +82,8 @@ export default class Checkbox extends React.PureComponent {
     label: null,
     label_position: null,
     title: null,
-    default_state: undefined,
-    checked: undefined,
+    default_state: null, // Deprecated
+    checked: null,
     disabled: null,
     id: null,
     size: null,
@@ -116,16 +116,15 @@ export default class Checkbox extends React.PureComponent {
 
   static getDerivedStateFromProps(props, state) {
     if (state._listenForPropChanges) {
-      if (
-        typeof props.default_state !== 'undefined' &&
-        typeof state.checked === 'undefined'
-      ) {
-        state.checked = Checkbox.parseChecked(props.default_state)
-      } else if (props.checked !== state._checked) {
-        state.checked = Checkbox.parseChecked(props.checked)
-      }
-      if (typeof props.checked !== 'undefined') {
-        state._checked = props.checked
+      if (props.checked !== state._checked) {
+        if (
+          props.default_state !== null &&
+          typeof state.checked === 'undefined'
+        ) {
+          state.checked = Checkbox.parseChecked(props.default_state)
+        } else {
+          state.checked = Checkbox.parseChecked(props.checked)
+        }
       }
     }
     state._listenForPropChanges = true
@@ -136,9 +135,7 @@ export default class Checkbox extends React.PureComponent {
       })
     }
 
-    if (typeof state.checked === 'undefined') {
-      state.checked = false
-    }
+    state._checked = props.checked
     state.__checked = state.checked
 
     return state

diff --git a/packages/dnb-ui-lib/src/components/checkbox/__tests__/Checkbox.test.js b/packages/dnb-ui-lib/src/components/checkbox/__tests__/Checkbox.test.js
@@ -34,17 +34,17 @@ describe('Checkbox component', () => {
 
   it('has correct state after "change" trigger', () => {
     // default checked value has to be false
-    expect(Comp.state().checked).toBe(false)
+    expect(Comp.find('input').instance().checked).toBe(false)
 
     Comp.find('input').simulate('change') // we could send inn the event data structure like this: , { target: { checked: true } }
-    expect(Comp.state().checked).toBe(true)
+    expect(Comp.find('input').instance().checked).toBe(true)
 
     Comp.find('input').simulate('change')
-    expect(Comp.state().checked).toBe(false)
+    expect(Comp.find('input').instance().checked).toBe(false)
 
     // also check if getDerivedStateFromProps sets the state as expected
     Comp.setProps({ checked: true })
-    expect(Comp.state().checked).toBe(true)
+    expect(Comp.find('input').instance().checked).toBe(true)
 
     const value = 'new value'
     Comp.setProps({ value })
@@ -65,6 +65,86 @@ describe('Checkbox component', () => {
     expect(my_event.mock.calls[0][0].checked).toBe(true)
   })
 
+  it('uses "default_value" as the startup state – if given', () => {
+    expect(
+      mount(<Component default_state={true} />)
+        .find('input')
+        .instance().checked
+    ).toBe(true)
+
+    expect(
+      mount(<Component default_state={true} checked={false} />)
+        .find('input')
+        .instance().checked
+    ).toBe(true)
+
+    const Comp = mount(<Component default_state={false} checked={true} />)
+    expect(Comp.find('input').instance().checked).toBe(false)
+
+    Comp.find('input').simulate('change')
+    expect(Comp.find('input').instance().checked).toBe(true)
+  })
+
+  it('does handle controlled vs uncontrolled state properly', () => {
+    const ControlledVsUncontrolled = () => {
+      const [checked, setChecked] = React.useState(true)
+      const [random, setRandom] = React.useState()
+
+      return (
+        <>
+          <Component
+            checked={checked}
+            on_change={({ checked }) => setChecked(checked)}
+          />
+          <button id="set-state" onClick={() => setChecked(true)} />
+          <button
+            id="reset-undefined"
+            onClick={() => setChecked(undefined)}
+          />
+          <button id="reset-null" onClick={() => setChecked(null)} />
+          <button id="rerender" onClick={() => setRandom(Math.random())} />
+          <code>{JSON.stringify({ checked, random })}</code>
+        </>
+      )
+    }
+
+    const TestStates = (Comp) => {
+      // re-render + default state is true
+      Comp.find('button#rerender').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(true)
+
+      // change it to false
+      Comp.find('input').simulate('change')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // set it to true
+      Comp.find('button#set-state').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(true)
+
+      // reset it with undefined to false
+      Comp.find('button#reset-undefined').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // set it to true + reset it with null to false
+      Comp.find('button#set-state').simulate('click')
+      Comp.find('button#reset-null').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // re-render + still false
+      Comp.find('button#rerender').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+    }
+
+    TestStates(mount(<ControlledVsUncontrolled />))
+    TestStates(
+      mount(
+        <React.StrictMode>
+          <ControlledVsUncontrolled />
+        </React.StrictMode>
+      )
+    )
+  })
+
   it('has a disabled attribute, once we set disabled to true', () => {
     const Comp = mount(<Component />)
     Comp.setProps({

diff --git a/packages/dnb-ui-lib/src/components/checkbox/__tests__/__snapshots__/Checkbox.test.js.snap b/packages/dnb-ui-lib/src/components/checkbox/__tests__/__snapshots__/Checkbox.test.js.snap
@@ -42,10 +42,12 @@ exports[`Checkbox component have to match snapshot 1`] = `
     }
   }
   attributes={null}
+  checked={null}
   class={null}
   className={null}
   custom_element={null}
   custom_method={null}
+  default_state={null}
   disabled={null}
   global_status_id={null}
   id="checkbox"

diff --git a/packages/dnb-ui-lib/src/components/radio/Radio.js b/packages/dnb-ui-lib/src/components/radio/Radio.js
@@ -84,7 +84,7 @@ export default class Radio extends React.PureComponent {
     label: null,
     label_sr_only: null,
     label_position: null,
-    checked: undefined,
+    checked: null,
     disabled: false,
     id: null,
     size: null,
@@ -123,9 +123,6 @@ export default class Radio extends React.PureComponent {
       if (props.checked !== state._checked) {
         state.checked = Radio.parseChecked(props.checked)
       }
-      if (typeof props.checked !== 'undefined') {
-        state._checked = props.checked
-      }
     }
     state._listenForPropChanges = true
 
@@ -135,9 +132,7 @@ export default class Radio extends React.PureComponent {
       })
     }
 
-    if (typeof state.checked === 'undefined') {
-      state.checked = false
-    }
+    state._checked = props.checked
     state.__checked = state.checked
 
     return state

diff --git a/packages/dnb-ui-lib/src/components/radio/__tests__/Radio.test.js b/packages/dnb-ui-lib/src/components/radio/__tests__/Radio.test.js
@@ -35,17 +35,17 @@ describe('Radio component', () => {
 
   it('has correct state after "change" trigger', () => {
     // default checked value has to be false
-    expect(Comp.state().checked).toBe(false)
+    expect(Comp.find('input').instance().checked).toBe(false)
 
     Comp.find('input').simulate('change') // we could send inn the event data structure like this: , { target: { checked: true } }
-    expect(Comp.state().checked).toBe(true)
+    expect(Comp.find('input').instance().checked).toBe(true)
 
     Comp.find('input').simulate('change')
-    expect(Comp.state().checked).toBe(false)
+    expect(Comp.find('input').instance().checked).toBe(false)
 
     // also check if getDerivedStateFromProps sets the state as expected
     Comp.setProps({ checked: true })
-    expect(Comp.state().checked).toBe(true)
+    expect(Comp.find('input').instance().checked).toBe(true)
 
     const value = 'new value'
     Comp.setProps({ value })
@@ -71,6 +71,66 @@ describe('Radio component', () => {
     expect(my_event.mock.calls[0][0].checked).toBe(true)
   })
 
+  it('does handle controlled vs uncontrolled state properly', () => {
+    const ControlledVsUncontrolled = () => {
+      const [checked, setChecked] = React.useState(true)
+      const [random, setRandom] = React.useState()
+
+      return (
+        <>
+          <Component
+            checked={checked}
+            on_change={({ checked }) => setChecked(checked)}
+          />
+          <button id="set-state" onClick={() => setChecked(true)} />
+          <button
+            id="reset-undefined"
+            onClick={() => setChecked(undefined)}
+          />
+          <button id="reset-null" onClick={() => setChecked(null)} />
+          <button id="rerender" onClick={() => setRandom(Math.random())} />
+          <code>{JSON.stringify({ checked, random })}</code>
+        </>
+      )
+    }
+
+    const TestStates = (Comp) => {
+      // re-render + default state is true
+      Comp.find('button#rerender').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(true)
+
+      // change it to false
+      Comp.find('input').simulate('change')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // set it to true
+      Comp.find('button#set-state').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(true)
+
+      // reset it with undefined to false
+      Comp.find('button#reset-undefined').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // set it to true + reset it with null to false
+      Comp.find('button#set-state').simulate('click')
+      Comp.find('button#reset-null').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+
+      // re-render + still false
+      Comp.find('button#rerender').simulate('click')
+      expect(Comp.find('input').instance().checked).toBe(false)
+    }
+
+    TestStates(mount(<ControlledVsUncontrolled />))
+    TestStates(
+      mount(
+        <React.StrictMode>
+          <ControlledVsUncontrolled />
+        </React.StrictMode>
+      )
+    )
+  })
+
   it('has a disabled attribute, once we set disabled to true', () => {
     const Comp = mount(<Component />)
     Comp.setProps({

diff --git a/packages/dnb-ui-lib/src/components/radio/__tests__/__snapshots__/Radio.test.js.snap b/packages/dnb-ui-lib/src/components/radio/__tests__/__snapshots__/Radio.test.js.snap
@@ -279,6 +279,7 @@ exports[`Radio group component have to match group snapshot 1`] = `
               >
                 <Radio
                   attributes={null}
+                  checked={null}
                   class={null}
                   className={null}
                   custom_element={null}

diff --git a/packages/dnb-ui-lib/src/components/switch/Switch.js b/packages/dnb-ui-lib/src/components/switch/Switch.js
@@ -83,8 +83,8 @@ export default class Switch extends React.PureComponent {
     label: null,
     label_position: null,
     title: null,
-    default_state: undefined, // Deprecated
-    checked: undefined,
+    default_state: null, // Deprecated
+    checked: null,
     disabled: null,
     id: null,
     size: null,
@@ -118,16 +118,15 @@ export default class Switch extends React.PureComponent {
 
   static getDerivedStateFromProps(props, state) {
     if (state._listenForPropChanges) {
-      if (
-        typeof props.default_state !== 'undefined' &&
-        typeof state.checked === 'undefined'
-      ) {
-        state.checked = Switch.parseChecked(props.default_state)
-      } else if (props.checked !== state._checked) {
-        state.checked = Switch.parseChecked(props.checked)
-      }
-      if (typeof props.checked !== 'undefined') {
-        state._checked = props.checked
+      if (props.checked !== state._checked) {
+        if (
+          props.default_state !== null &&
+          typeof state.checked === 'undefined'
+        ) {
+          state.checked = Switch.parseChecked(props.default_state)
+        } else {
+          state.checked = Switch.parseChecked(props.checked)
+        }
       }
     }
     state._listenForPropChanges = true
@@ -138,9 +137,7 @@ export default class Switch extends React.PureComponent {
       })
     }
 
-    if (typeof state.checked === 'undefined') {
-      state.checked = false
-    }
+    state._checked = props.checked
     state.__checked = state.checked
 
     return state