From c3db20461468de7a7baaceade7aa14f073a0dbc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laussel=20LO=C3=AFc?= <loic.laussel@orange.com>
Date: Wed, 10 Jun 2020 16:30:21 +0300
Subject: [PATCH] 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 = '<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">' +