fix $().button('toggle') not working for checkbox inside label (#30388

)
twbs · Mar 16, 2020 · 8b6dd44 · 8b6dd44
1 parent ac5685d
commit 8b6dd44
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 9 deletions.
diff --git a/js/src/button.js b/js/src/button.js
@@ -84,17 +84,13 @@ class Button {
               $(activeElement).removeClass(ClassName.ACTIVE)
             }
           }
-        } else if (input.type === 'checkbox') {
-          if (this._element.tagName === 'LABEL' && input.checked === this._element.classList.contains(ClassName.ACTIVE)) {
-            triggerChangeEvent = false
-          }
-        } else {
-          // if it's not a radio button or checkbox don't add a pointless/invalid checked property to the input
-          triggerChangeEvent = false
         }
 
         if (triggerChangeEvent) {
-          input.checked = !this._element.classList.contains(ClassName.ACTIVE)
+          // if it's not a radio button or checkbox don't add a pointless/invalid checked property to the input
+          if (input.type === 'checkbox' || input.type === 'radio') {
+            input.checked = !this._element.classList.contains(ClassName.ACTIVE)
+          }
           $(input).trigger('change')
         }
 
@@ -147,6 +143,7 @@ class Button {
 $(document)
   .on(Event.CLICK_DATA_API, Selector.DATA_TOGGLE_CARROT, (event) => {
     let button = event.target
+    const initialButton = button
 
     if (!$(button).hasClass(ClassName.BUTTON)) {
       button = $(button).closest(Selector.BUTTON)[0]
@@ -162,6 +159,9 @@ $(document)
         return
       }
 
+      if (initialButton.tagName === 'LABEL' && inputBtn && inputBtn.type === 'checkbox') {
+        event.preventDefault() // work around event sent to label and input
+      }
       Button._jQueryInterface.call($(button), 'toggle')
     }
   })

diff --git a/js/tests/unit/button.js b/js/tests/unit/button.js
@@ -181,7 +181,7 @@ $(function () {
   })
 
   QUnit.test('should check for closest matching toggle', function (assert) {
-    assert.expect(12)
+    assert.expect(18)
     var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
       '<label class="btn btn-primary active">' +
       '<input type="radio" name="options" id="option1" checked="true"> Option 1' +
@@ -213,6 +213,13 @@ $(function () {
     assert.ok(!$btn1.find('input').prop('checked'), 'btn1 is not checked')
     assert.ok($btn2.hasClass('active'), 'btn2 has active class')
     assert.ok($btn2.find('input').prop('checked'), 'btn2 is checked')
+    $btn1.bootstrapButton('toggle')
+    assert.ok($btn1.hasClass('active'), 'btn1 has active class')
+    assert.ok($btn1.find('input').prop('checked'), 'btn1 prop is checked')
+    assert.ok($btn1.find('input')[0].checked, 'btn1 is checked with jquery')
+    assert.ok(!$btn2.hasClass('active'), 'btn2 does not have active class')
+    assert.ok(!$btn2.find('input').prop('checked'), 'btn2 is not checked')
+    assert.ok(!$btn2.find('input')[0].checked, 'btn2 is not checked')
   })
 
   QUnit.test('should not add aria-pressed on labels for radio/checkbox inputs in a data-toggle="buttons" group', function (assert) {
@@ -309,6 +316,27 @@ $(function () {
     assert.ok($input.prop('checked'), 'checkbox is checked after click')
   })
 
+  QUnit.test('should correctly set checked state on input and active class on the label when using button toggle', function (assert) {
+    assert.expect(6)
+    var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
+        '<label class="btn">' +
+          '<input type="checkbox">' +
+        '</label>' +
+      '</div>'
+    var $group = $(groupHTML).appendTo('#qunit-fixture')
+
+    var $btn = $group.children().eq(0)
+    var $input = $btn.children().eq(0)
+
+    assert.ok($btn.is(':not(.active)'), '<label> is initially not active')
+    assert.ok(!$input.prop('checked'), 'checkbox property is initially not checked')
+    assert.ok(!$input[0].checked, 'checkbox is not checked by jquery after click')
+    $btn.bootstrapButton('toggle')
+    assert.ok($btn.is('.active'), '<label> is active after click')
+    assert.ok($input.prop('checked'), 'checkbox property is checked after click')
+    assert.ok($input[0].checked, 'checkbox is checked by jquery after click')
+  })
+
   QUnit.test('should not do anything if the click was just sent to the outer container with data-toggle', function (assert) {
     assert.expect(4)
     var groupHTML = '<div class="btn-group" data-toggle="buttons">' +