From c3db20461468de7a7baaceade7aa14f073a0dbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laussel=20LO=C3=AFc?= Date: Wed, 10 Jun 2020 16:30:21 +0300 Subject: [PATCH 1/6] avoid multiple change event trigger : - add unit test to count how many event are thrown when widget contains multiple tags inside label - add a parameter to toggle, if click event is provided onto an input then don't trigger another change event already thrown by the browser - simplify the case where toggle interface is called click provide from input itself OR it's a button without label. If label is present, then browser propagate clic event from childrens thru label and then cause multiple calls to toggle - the test assume that `.btn` class is always set onto the label if there's one, otherwise need to update this plugin and look for label arround the input Test with keyboard, mouse and js clic call --- js/src/button.js | 14 ++++++++------ js/tests/unit/button.js | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/js/src/button.js b/js/src/button.js index 2aed6b09c718..4365a65ea05e 100644 --- a/js/src/button.js +++ b/js/src/button.js @@ -56,7 +56,7 @@ class Button { // Public - toggle() { + toggle(avoidTriggerChange) { let triggerChangeEvent = true let addAriaPressed = true const rootElement = $(this._element).closest( @@ -86,7 +86,9 @@ class Button { input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE) } - $(input).trigger('change') + if (!avoidTriggerChange) { + $(input).trigger('change') + } } input.focus() @@ -113,7 +115,7 @@ class Button { // Static - static _jQueryInterface(config) { + static _jQueryInterface(config, avoidTriggerChange) { return this.each(function () { let data = $(this).data(DATA_KEY) @@ -123,7 +125,7 @@ class Button { } if (config === 'toggle') { - data[config]() + data[config](avoidTriggerChange) } }) } @@ -154,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') } } }) diff --git a/js/tests/unit/button.js b/js/tests/unit/button.js index 2adffedd9fe6..1bd62b6b8b60 100644 --- a/js/tests/unit/button.js +++ b/js/tests/unit/button.js @@ -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 = '
' + + '' + + '
' + 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 = '
' + From a01b2bf2dafad7f89ca9cd58b3c1fe575a10eaa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laussel=20LO=C3=AFc?= Date: Thu, 1 Oct 2020 14:48:07 +0200 Subject: [PATCH 2/6] replace `avoidTriggerChange` parameter by a css class `CLASS_NAME_CHANGING` on button --- js/src/button.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/js/src/button.js b/js/src/button.js index 4365a65ea05e..82fd51ece9cc 100644 --- a/js/src/button.js +++ b/js/src/button.js @@ -23,6 +23,7 @@ const JQUERY_NO_CONFLICT = $.fn[NAME] const CLASS_NAME_ACTIVE = 'active' const CLASS_NAME_BUTTON = 'btn' const CLASS_NAME_FOCUS = 'focus' +const CLASS_NAME_CHANGING = 'changing' const SELECTOR_DATA_TOGGLE_CARROT = '[data-toggle^="button"]' const SELECTOR_DATA_TOGGLES = '[data-toggle="buttons"]' @@ -56,7 +57,7 @@ class Button { // Public - toggle(avoidTriggerChange) { + toggle() { let triggerChangeEvent = true let addAriaPressed = true const rootElement = $(this._element).closest( @@ -85,10 +86,10 @@ class Button { if (input.type === 'checkbox' || input.type === 'radio') { input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE) } - - if (!avoidTriggerChange) { + if (!$(this._element).hasClass(CLASS_NAME_CHANGING)) { $(input).trigger('change') } + $(this._element).removeClass(CLASS_NAME_CHANGING) } input.focus() @@ -115,7 +116,7 @@ class Button { // Static - static _jQueryInterface(config, avoidTriggerChange) { + static _jQueryInterface(config) { return this.each(function () { let data = $(this).data(DATA_KEY) @@ -125,7 +126,7 @@ class Button { } if (config === 'toggle') { - data[config](avoidTriggerChange) + data[config]() } }) } @@ -157,7 +158,10 @@ $(document) } if (initialButton.tagName === 'INPUT' || button.tagName !== 'LABEL') { - Button._jQueryInterface.call($(button), 'toggle', initialButton.tagName === 'INPUT') + if (initialButton.tagName === 'INPUT') { + $(button).addClass(CLASS_NAME_CHANGING) + } + Button._jQueryInterface.call($(button), 'toggle') } } }) From b2b360722473b63e49b54d70b68b10769e77b460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laussel=20LO=C3=AFc?= Date: Thu, 1 Oct 2020 15:31:59 +0200 Subject: [PATCH 3/6] fix linting error and upgrade bundlesize --- .bundlewatch.config.json | 2 +- js/src/button.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index e12dda91810a..41f6d4d131b4 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -34,7 +34,7 @@ }, { "path": "./dist/js/bootstrap.js", - "maxSize": "25 kB" + "maxSize": "25.05 kB" }, { "path": "./dist/js/bootstrap.min.js", diff --git a/js/src/button.js b/js/src/button.js index 3e71cf04f490..927bc2332a5f 100644 --- a/js/src/button.js +++ b/js/src/button.js @@ -83,9 +83,11 @@ class Button { if (input.type === 'checkbox' || input.type === 'radio') { input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE) } + if (!$(this._element).hasClass(CLASS_NAME_CHANGING)) { $(input).trigger('change') } + $(this._element).removeClass(CLASS_NAME_CHANGING) } @@ -155,9 +157,11 @@ $(document) } if (initialButton.tagName === 'INPUT' || button.tagName !== 'LABEL') { + if (initialButton.tagName === 'INPUT') { $(button).addClass(CLASS_NAME_CHANGING) } + Button._jQueryInterface.call($(button), 'toggle') } } From 4162e58cdced88bc6bd4b3bd9cf03cbe36e936d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laussel=20LO=C3=AFc?= Date: Thu, 1 Oct 2020 15:46:00 +0200 Subject: [PATCH 4/6] fix latest lint error --- js/src/button.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/button.js b/js/src/button.js index 927bc2332a5f..62e8cb6f5f5d 100644 --- a/js/src/button.js +++ b/js/src/button.js @@ -157,7 +157,6 @@ $(document) } if (initialButton.tagName === 'INPUT' || button.tagName !== 'LABEL') { - if (initialButton.tagName === 'INPUT') { $(button).addClass(CLASS_NAME_CHANGING) } From 1632d3c499b1c0d8875762dd5f29d7ab4b8980eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laussel=20LO=C3=AFc?= Date: Fri, 2 Oct 2020 09:38:35 +0200 Subject: [PATCH 5/6] Revert "replace `avoidTriggerChange` parameter by a css class `CLASS_NAME_CHANGING` on button" use a class variable instead This reverts commit a01b2bf2dafad7f89ca9cd58b3c1fe575a10eaa4. # Conflicts: # js/src/button.js --- js/src/button.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/js/src/button.js b/js/src/button.js index 62e8cb6f5f5d..c95013a98cd3 100644 --- a/js/src/button.js +++ b/js/src/button.js @@ -23,7 +23,6 @@ const JQUERY_NO_CONFLICT = $.fn[NAME] const CLASS_NAME_ACTIVE = 'active' const CLASS_NAME_BUTTON = 'btn' const CLASS_NAME_FOCUS = 'focus' -const CLASS_NAME_CHANGING = 'changing' const SELECTOR_DATA_TOGGLE_CARROT = '[data-toggle^="button"]' const SELECTOR_DATA_TOGGLES = '[data-toggle="buttons"]' @@ -47,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 @@ -84,11 +84,9 @@ class Button { input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE) } - if (!$(this._element).hasClass(CLASS_NAME_CHANGING)) { + if (!this.shouldAvoidTriggerChange) { $(input).trigger('change') } - - $(this._element).removeClass(CLASS_NAME_CHANGING) } input.focus() @@ -114,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) @@ -124,6 +122,8 @@ class Button { $element.data(DATA_KEY, data) } + data.shouldAvoidTriggerChange = avoidTriggerChange + if (config === 'toggle') { data[config]() } @@ -157,11 +157,7 @@ $(document) } if (initialButton.tagName === 'INPUT' || button.tagName !== 'LABEL') { - if (initialButton.tagName === 'INPUT') { - $(button).addClass(CLASS_NAME_CHANGING) - } - - Button._jQueryInterface.call($(button), 'toggle') + Button._jQueryInterface.call($(button), 'toggle', initialButton.tagName === 'INPUT') } } }) From 8a37b3bbb7ca0f6e8ce053092752d95be7dc8131 Mon Sep 17 00:00:00 2001 From: XhmikosR Date: Fri, 2 Oct 2020 17:27:28 +0300 Subject: [PATCH 6/6] Update .bundlewatch.config.json --- .bundlewatch.config.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 41f6d4d131b4..fb76d8dd83b8 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -26,7 +26,7 @@ }, { "path": "./dist/js/bootstrap.bundle.js", - "maxSize": "47.50 kB" + "maxSize": "47.5 kB" }, { "path": "./dist/js/bootstrap.bundle.min.js", @@ -34,7 +34,7 @@ }, { "path": "./dist/js/bootstrap.js", - "maxSize": "25.05 kB" + "maxSize": "25.5 kB" }, { "path": "./dist/js/bootstrap.min.js",