From 109db988dcb434a30ffe934a3ca82eeafdc0a7ac Mon Sep 17 00:00:00 2001 From: kyletsang <6854874+kyletsang@users.noreply.github.com> Date: Mon, 8 Feb 2021 12:06:47 -0800 Subject: [PATCH 1/6] Fix event handler removal in carousel dispose --- js/src/carousel.js | 3 ++- js/tests/unit/carousel.spec.js | 23 +++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index 2874e6e95d7b..6d41d227cc59 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -216,7 +216,6 @@ class Carousel extends BaseComponent { } dispose() { - super.dispose() EventHandler.off(this._element, EVENT_KEY) this._items = null @@ -226,6 +225,8 @@ class Carousel extends BaseComponent { this._isSliding = null this._activeElement = null this._indicatorsElement = null + + super.dispose() } // Private diff --git a/js/tests/unit/carousel.spec.js b/js/tests/unit/carousel.spec.js index 0571ac9af890..5c45efe0c10f 100644 --- a/js/tests/unit/carousel.spec.js +++ b/js/tests/unit/carousel.spec.js @@ -295,6 +295,8 @@ describe('Carousel', () => { spyOn(Carousel.prototype, '_addTouchEventListeners') + // Headless browser does not support touch events, so need to fake it + // to test that touch events are add properly. document.documentElement.ontouchstart = () => {} const carousel = new Carousel(carouselEl) @@ -1056,13 +1058,30 @@ describe('Carousel', () => { ].join('') const carouselEl = fixtureEl.querySelector('#myCarousel') + const addEventSpy = spyOn(carouselEl, 'addEventListener').and.callThrough() + const removeEventSpy = spyOn(carouselEl, 'removeEventListener').and.callThrough() + + // Headless browser does not support touch events, so need to fake it + // to test that touch events are add/removed properly. + document.documentElement.ontouchstart = () => {} + const carousel = new Carousel(carouselEl) - spyOn(EventHandler, 'off').and.callThrough() + const expectedArgs = [ + ['keydown', jasmine.any(Function), jasmine.any(Boolean)], + ['mouseover', jasmine.any(Function), jasmine.any(Boolean)], + ['mouseout', jasmine.any(Function), jasmine.any(Boolean)], + ['pointerdown', jasmine.any(Function), jasmine.any(Boolean)], + ['pointerup', jasmine.any(Function), jasmine.any(Boolean)] + ] + + expect(addEventSpy.calls.allArgs()).toEqual(expectedArgs) carousel.dispose() - expect(EventHandler.off).toHaveBeenCalled() + expect(removeEventSpy.calls.allArgs()).toEqual(expectedArgs) + + delete document.documentElement.ontouchstart }) }) From 956c292aad45d4e5d8f4a88812670c9b5b7c901d Mon Sep 17 00:00:00 2001 From: kyletsang <6854874+kyletsang@users.noreply.github.com> Date: Mon, 8 Feb 2021 12:10:57 -0800 Subject: [PATCH 2/6] Fix event handler removal in dropdown dispose --- js/src/dropdown.js | 3 ++- js/tests/unit/dropdown.spec.js | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 06b561425ae3..1e7cadd4dffc 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -222,7 +222,6 @@ class Dropdown extends BaseComponent { } dispose() { - super.dispose() EventHandler.off(this._element, EVENT_KEY) this._menu = null @@ -230,6 +229,8 @@ class Dropdown extends BaseComponent { this._popper.destroy() this._popper = null } + + super.dispose() } update() { diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index 47775678f87a..815c8977c7d4 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -862,16 +862,21 @@ describe('Dropdown', () => { ].join('') const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') + spyOn(btnDropdown, 'addEventListener').and.callThrough() + spyOn(btnDropdown, 'removeEventListener').and.callThrough() + const dropdown = new Dropdown(btnDropdown) expect(dropdown._popper).toBeNull() expect(dropdown._menu).toBeDefined() expect(dropdown._element).toBeDefined() + expect(btnDropdown.addEventListener).toHaveBeenCalledWith('click', jasmine.any(Function), jasmine.any(Boolean)) dropdown.dispose() expect(dropdown._menu).toBeNull() expect(dropdown._element).toBeNull() + expect(btnDropdown.removeEventListener).toHaveBeenCalledWith('click', jasmine.any(Function), jasmine.any(Boolean)) }) it('should dispose dropdown with Popper', () => { From 6bbaefc8592d94278ec9b86ea859ec2b6bf884c1 Mon Sep 17 00:00:00 2001 From: kyletsang <6854874+kyletsang@users.noreply.github.com> Date: Mon, 8 Feb 2021 12:11:16 -0800 Subject: [PATCH 3/6] Test event handlers in scrollspy dispose --- js/tests/unit/scrollspy.spec.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/js/tests/unit/scrollspy.spec.js b/js/tests/unit/scrollspy.spec.js index 0d175aafa384..917593f3987c 100644 --- a/js/tests/unit/scrollspy.spec.js +++ b/js/tests/unit/scrollspy.spec.js @@ -1,6 +1,5 @@ import ScrollSpy from '../../src/scrollspy' import Manipulator from '../../src/dom/manipulator' -import EventHandler from '../../src/dom/event-handler' /** Test helpers */ import { getFixture, clearFixture, createEvent, jQueryMock } from '../helpers/fixture' @@ -560,14 +559,18 @@ describe('ScrollSpy', () => { describe('dispose', () => { it('should dispose a scrollspy', () => { - spyOn(EventHandler, 'off') fixtureEl.innerHTML = '
' const divEl = fixtureEl.querySelector('div') + spyOn(divEl, 'addEventListener').and.callThrough() + spyOn(divEl, 'removeEventListener').and.callThrough() + const scrollSpy = new ScrollSpy(divEl) + expect(divEl.addEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function), jasmine.any(Boolean)) scrollSpy.dispose() - expect(EventHandler.off).toHaveBeenCalledWith(divEl, '.bs.scrollspy') + + expect(divEl.removeEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function), jasmine.any(Boolean)) }) }) From 4d884cf1ae39cc62c9e7d69d4926e76af49336a9 Mon Sep 17 00:00:00 2001 From: kyletsang <6854874+kyletsang@users.noreply.github.com> Date: Mon, 8 Feb 2021 12:11:36 -0800 Subject: [PATCH 4/6] Test event handlers in toast dispose --- js/tests/unit/toast.spec.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/js/tests/unit/toast.spec.js b/js/tests/unit/toast.spec.js index 2920eb2efa0f..44f74aa36dab 100644 --- a/js/tests/unit/toast.spec.js +++ b/js/tests/unit/toast.spec.js @@ -274,13 +274,18 @@ describe('Toast', () => { fixtureEl.innerHTML = '
' const toastEl = fixtureEl.querySelector('div') + spyOn(toastEl, 'addEventListener').and.callThrough() + spyOn(toastEl, 'removeEventListener').and.callThrough() + const toast = new Toast(toastEl) expect(Toast.getInstance(toastEl)).toBeDefined() + expect(toastEl.addEventListener).toHaveBeenCalledWith('click', jasmine.any(Function), jasmine.any(Boolean)) toast.dispose() expect(Toast.getInstance(toastEl)).toBeNull() + expect(toastEl.removeEventListener).toHaveBeenCalledWith('click', jasmine.any(Function), jasmine.any(Boolean)) }) it('should allow to destroy toast and hide it before that', done => { From 2166472b51a596d02b75888ed0080f3c9d27e68e Mon Sep 17 00:00:00 2001 From: kyletsang <6854874+kyletsang@users.noreply.github.com> Date: Mon, 8 Feb 2021 12:11:50 -0800 Subject: [PATCH 5/6] Test event handlers in tooltip dispose --- js/tests/unit/tooltip.spec.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 38af0235ba29..0e89dba72983 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -312,13 +312,26 @@ describe('Tooltip', () => { fixtureEl.innerHTML = '' const tooltipEl = fixtureEl.querySelector('a') + const addEventSpy = spyOn(tooltipEl, 'addEventListener').and.callThrough() + const removeEventSpy = spyOn(tooltipEl, 'removeEventListener').and.callThrough() + const tooltip = new Tooltip(tooltipEl) expect(Tooltip.getInstance(tooltipEl)).toEqual(tooltip) + const expectedArgs = [ + ['mouseover', jasmine.any(Function), jasmine.any(Boolean)], + ['mouseout', jasmine.any(Function), jasmine.any(Boolean)], + ['focusin', jasmine.any(Function), jasmine.any(Boolean)], + ['focusout', jasmine.any(Function), jasmine.any(Boolean)] + ] + + expect(addEventSpy.calls.allArgs()).toEqual(expectedArgs) + tooltip.dispose() expect(Tooltip.getInstance(tooltipEl)).toEqual(null) + expect(removeEventSpy.calls.allArgs()).toEqual(expectedArgs) }) it('should destroy a tooltip after it is shown and hidden', done => { From 344f2879b5076f0ac1177b04ad6315bd94eff4fd Mon Sep 17 00:00:00 2001 From: XhmikosR Date: Thu, 11 Feb 2021 19:44:18 +0200 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 36404b38cfda..bd9ce671d45e 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -38,7 +38,7 @@ }, { "path": "./dist/js/bootstrap.bundle.min.js", - "maxSize": "21.5 kB" + "maxSize": "21.75 kB" }, { "path": "./dist/js/bootstrap.esm.js", @@ -54,7 +54,7 @@ }, { "path": "./dist/js/bootstrap.min.js", - "maxSize": "15.5 kB" + "maxSize": "15.75 kB" } ], "ci": {