From 3077ba767d12d193b9187290c98c077ab9ce8e5d Mon Sep 17 00:00:00 2001
From: Carlos Wu <carlos.fei@canonical.com>
Date: Wed, 28 Sep 2022 10:27:36 +0100
Subject: [PATCH] fix: fire onChange handler when toggle and id are passed

This commit attempts to fix onChange not firing when `id` is passed.
Previouly, there was a fix to prevent two clicks from being fired by
stopping propagation, but in my example I was able to prevent two clicks
from being fired using `e.preventDefault`
---
 .../Radio/Types/RadioOnchangeExample.js       | 22 +++++++++++++++++++
 docs/src/examples/addons/Radio/Types/index.js |  5 +++++
 src/modules/Checkbox/Checkbox.js              | 15 ++++++++-----
 test/specs/modules/Checkbox/Checkbox-test.js  | 16 --------------
 4 files changed, 36 insertions(+), 22 deletions(-)
 create mode 100644 docs/src/examples/addons/Radio/Types/RadioOnchangeExample.js

diff --git a/docs/src/examples/addons/Radio/Types/RadioOnchangeExample.js b/docs/src/examples/addons/Radio/Types/RadioOnchangeExample.js
new file mode 100644
index 0000000000..b15d25c621
--- /dev/null
+++ b/docs/src/examples/addons/Radio/Types/RadioOnchangeExample.js
@@ -0,0 +1,22 @@
+import React, { useState } from 'react'
+import { Radio } from 'semantic-ui-react'
+
+const RadioOnChange = () => {
+  const [profileState, setProfileState] = useState(false)
+
+  const handleChange = () => setProfileState(!profileState)
+
+  return (
+    <>
+      <p>{profileState ? 'Profile is visible' : 'Profile is invisible'}</p>
+      <Radio
+        id='profile-radio'
+        toggle
+        label='Make my profile visible'
+        onChange={handleChange}
+      />
+    </>
+  )
+}
+
+export default RadioOnChange
diff --git a/docs/src/examples/addons/Radio/Types/index.js b/docs/src/examples/addons/Radio/Types/index.js
index 6e3524d089..f8f1e05543 100644
--- a/docs/src/examples/addons/Radio/Types/index.js
+++ b/docs/src/examples/addons/Radio/Types/index.js
@@ -6,6 +6,11 @@ import ExampleSection from 'docs/src/components/ComponentDoc/ExampleSection'
 
 const RadioTypesExamples = () => (
   <ExampleSection title='Types'>
+    <ComponentExample
+      title='Radio onChange'
+      description='Radio toggle should work when id is passed.'
+      examplePath='addons/Radio/Types/RadioOnchangeExample'
+    />
     <ComponentExample
       title='Radio'
       description='A radio for checking.'
diff --git a/src/modules/Checkbox/Checkbox.js b/src/modules/Checkbox/Checkbox.js
index bd4e02f10f..0a753cc693 100644
--- a/src/modules/Checkbox/Checkbox.js
+++ b/src/modules/Checkbox/Checkbox.js
@@ -73,7 +73,10 @@ export default class Checkbox extends Component {
     if (this.isClickFromMouse) {
       this.isClickFromMouse = false
 
-      if (isLabelClick && !hasId) {
+      if (isLabelClick) {
+        // Prevents two clicks from being fired rom the "input" click: https://github.com/Semantic-Org/Semantic-UI-React/issues/3433
+        // and also allows onChange to fire https://github.com/Semantic-Org/Semantic-UI-React/issues/3737
+        e.preventDefault()
         this.handleChange(e)
       }
 
@@ -82,11 +85,11 @@ export default class Checkbox extends Component {
         this.handleChange(e)
       }
 
-      if (isLabelClick && hasId) {
-        // To prevent two clicks from being fired from the component we have to stop the propagation
-        // from the "input" click: https://github.com/Semantic-Org/Semantic-UI-React/issues/3433
-        e.stopPropagation()
-      }
+      // if (isLabelClick && hasId) {
+      //   // To prevent two clicks from being fired from the component we have to stop the propagation
+      //   // from the "input" click: https://github.com/Semantic-Org/Semantic-UI-React/issues/3433
+      //   e.stopPropagation()
+      // }
     }
   }
 
diff --git a/test/specs/modules/Checkbox/Checkbox-test.js b/test/specs/modules/Checkbox/Checkbox-test.js
index b24faa7f43..122fec1e45 100644
--- a/test/specs/modules/Checkbox/Checkbox-test.js
+++ b/test/specs/modules/Checkbox/Checkbox-test.js
@@ -241,15 +241,6 @@ describe('Checkbox', () => {
       )
     })
 
-    it('is not called when on change when "id" is passed', () => {
-      const onChange = sandbox.spy()
-      wrapperMount(<Checkbox id='foo' onChange={onChange} />)
-
-      wrapper.find('label').simulate('mouseup')
-      wrapper.find('label').simulate('click')
-      onChange.should.have.not.been.called()
-    })
-
     it('is called when click is done on nested element', () => {
       const onChange = sandbox.spy()
       wrapperMount(<Checkbox label={{ children: <span>Foo</span> }} onChange={onChange} />)
@@ -408,13 +399,6 @@ describe('Checkbox', () => {
           input: ['click'],
         },
       },
-      {
-        description: 'click on label with "id": fires on mouse click',
-        events: {
-          label: ['mouseup', 'click'],
-        },
-        id: 'foo',
-      },
       {
         description: 'click on input with "id": fires on mouse click',
         events: {