Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buttons plugin : avoid multiple change event trigger #31000

Merged
merged 8 commits into from
Oct 2, 2020
4 changes: 2 additions & 2 deletions .bundlewatch.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@
},
{
"path": "./dist/js/bootstrap.bundle.js",
"maxSize": "47.50 kB"
"maxSize": "47.5 kB"
},
{
"path": "./dist/js/bootstrap.bundle.min.js",
"maxSize": "21.5 kB"
},
{
"path": "./dist/js/bootstrap.js",
"maxSize": "25 kB"
"maxSize": "25.5 kB"
},
{
"path": "./dist/js/bootstrap.min.js",
Expand Down
13 changes: 9 additions & 4 deletions js/src/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const EVENT_LOAD_DATA_API = `load${EVENT_KEY}${DATA_API_KEY}`
class Button {
constructor(element) {
this._element = element
this.shouldAvoidTriggerChange = false
}

// Getters
Expand Down Expand Up @@ -83,7 +84,9 @@ class Button {
input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE)
}

$(input).trigger('change')
if (!this.shouldAvoidTriggerChange) {
$(input).trigger('change')
Johann-S marked this conversation as resolved.
Show resolved Hide resolved
}
}

input.focus()
Expand All @@ -109,7 +112,7 @@ class Button {

// Static

static _jQueryInterface(config) {
static _jQueryInterface(config, avoidTriggerChange) {
return this.each(function () {
const $element = $(this)
let data = $element.data(DATA_KEY)
Expand All @@ -119,6 +122,8 @@ class Button {
$element.data(DATA_KEY, data)
}

data.shouldAvoidTriggerChange = avoidTriggerChange

if (config === 'toggle') {
data[config]()
}
Expand Down Expand Up @@ -151,8 +156,8 @@ $(document)
return
}

if (initialButton.tagName !== 'LABEL' || inputBtn && inputBtn.type !== 'checkbox') {
Button._jQueryInterface.call($(button), 'toggle')
if (initialButton.tagName === 'INPUT' || button.tagName !== 'LABEL') {
Button._jQueryInterface.call($(button), 'toggle', initialButton.tagName === 'INPUT')
}
}
})
Expand Down
26 changes: 26 additions & 0 deletions js/tests/unit/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,32 @@ $(function () {
$group.find('label').trigger('click')
})

QUnit.test('should trigger label change event only once', function (assert) {
assert.expect(1)
var done = assert.async()
var countChangeEvent = 0

var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-primary">' +
'<input type="checkbox"><span class="check">✓</span> <i class="far fa-clipboard"></i> <span class="d-none d-lg-inline">checkbox</span>' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')

var $btn = $group.children().eq(0)

$group.find('label').on('change', function () {
countChangeEvent++
})

setTimeout(function () {
assert.ok(countChangeEvent === 1, 'onchange event fired only once')
done()
}, 5)

$btn[0].click()
})

QUnit.test('should check for closest matching toggle', function (assert) {
assert.expect(18)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
Expand Down