Skip to content

Commit

Permalink
fix twbs#30077 for V4 $().button('toggle') not working for checkbox i…
Browse files Browse the repository at this point in the history
…nside label

add unit test to avoid further errors

Need to check on V5 without Jquery button plugin have the previous bug (checked state not change)
  • Loading branch information
Lausselloic committed Mar 12, 2020
1 parent ac5685d commit 45b5acb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
16 changes: 8 additions & 8 deletions js/src/button.js
Expand Up @@ -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')
}

Expand Down Expand Up @@ -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]
Expand All @@ -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')
}
})
Expand Down
30 changes: 29 additions & 1 deletion js/tests/unit/button.js
Expand Up @@ -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' +
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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">' +
Expand Down

0 comments on commit 45b5acb

Please sign in to comment.