From e4e92aae56b58512a543ace13ed1199297ec827e Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Thu, 14 Jul 2022 12:24:01 +0200 Subject: [PATCH 1/7] Fix missing `modal-open` class when toggle modals --- js/src/modal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/modal.js b/js/src/modal.js index 6efb13d9d5..eb154d0eea 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -133,6 +133,7 @@ class Modal extends BaseComponent { this._focustrap.deactivate() this._element.classList.remove(CLASS_NAME_SHOW) + document.body.classList.remove(CLASS_NAME_OPEN) this._queueCallback(() => this._hideModal(), this._element, this._isAnimated()) } @@ -245,7 +246,6 @@ class Modal extends BaseComponent { this._isTransitioning = false this._backdrop.hide(() => { - document.body.classList.remove(CLASS_NAME_OPEN) this._resetAdjustments() this._scrollBar.reset() EventHandler.trigger(this._element, EVENT_HIDDEN) From 05e9eec142bc627efa19eb54f799179999033ad5 Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Wed, 20 Jul 2022 10:22:57 +0200 Subject: [PATCH 2/7] Add test to check for modal-open class for modal Make sure the `open-modal` class is removed or added to `body` when a modal is open or closed. --- js/tests/unit/modal.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 9e463d2be3..07a6f55380 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -98,6 +98,7 @@ describe('Modal', () => { expect(modalEl.getAttribute('aria-hidden')).toBeNull() expect(modalEl.style.display).toEqual('block') expect(document.querySelector('.modal-backdrop')).not.toBeNull() + expect(document.body.classList.contains('modal-open')).toBe(true) resolve() }) @@ -698,6 +699,7 @@ describe('Modal', () => { expect(modalEl.getAttribute('aria-hidden')).toEqual('true') expect(modalEl.style.display).toEqual('none') expect(backdropSpy).toHaveBeenCalled() + expect(document.body.classList.contains('modal-open')).not.toBe(true) resolve() }) From 5b36b577c93c002874e357451e20f7bdb82d090a Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Fri, 2 Sep 2022 11:54:56 +0200 Subject: [PATCH 3/7] Add `modal-open` class when transition is complete --- js/src/modal.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/js/src/modal.js b/js/src/modal.js index 0b9c382e88..fef24e8906 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -109,9 +109,6 @@ class Modal extends BaseComponent { this._isTransitioning = true this._scrollBar.hide() - - document.body.classList.add(CLASS_NAME_OPEN) - this._adjustDialog() this._backdrop.show(() => this._showElement(relatedTarget)) @@ -188,6 +185,8 @@ class Modal extends BaseComponent { this._element.classList.add(CLASS_NAME_SHOW) const transitionComplete = () => { + document.body.classList.add(CLASS_NAME_OPEN) + if (this._config.focus) { this._focustrap.activate() } From 4d0c8fee965b8a5e83238af1a30132a7854835ac Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Tue, 6 Sep 2022 21:52:07 +0200 Subject: [PATCH 4/7] remove modal-open class before backdrop is hidden --- js/src/modal.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/src/modal.js b/js/src/modal.js index fef24e8906..b442ad44b4 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -130,7 +130,6 @@ class Modal extends BaseComponent { this._focustrap.deactivate() this._element.classList.remove(CLASS_NAME_SHOW) - document.body.classList.remove(CLASS_NAME_OPEN) this._queueCallback(() => this._hideModal(), this._element, this._isAnimated()) } @@ -244,6 +243,8 @@ class Modal extends BaseComponent { this._element.removeAttribute('role') this._isTransitioning = false + document.body.classList.remove(CLASS_NAME_OPEN) + this._backdrop.hide(() => { this._resetAdjustments() this._scrollBar.reset() From b38c18ecaf316ef4b72348da758776fd52792a67 Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Tue, 6 Sep 2022 22:04:57 +0200 Subject: [PATCH 5/7] Add test case when toggle between modals Check for modal-open class when toggle between modals. --- js/tests/unit/modal.spec.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 1c3469da18..88cef571f6 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -1263,4 +1263,36 @@ describe('Modal', () => { expect(modal2._config.backdrop).toBeTrue() }) }) + + describe('toggle between modals', () => { + it('should toggle modal-open class on body', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = [ + '', + '', + '' + ].join('') + + const trigger = fixtureEl.querySelector('button') + const modalEl1 = document.querySelector('#exampleModalToggle') + const modalEl2 = document.querySelector('#exampleModalToggle2') + const modal1 = new Modal(modalEl1) + + modalEl1.addEventListener('shown.bs.modal', () => { + expect(document.body.classList.contains('modal-open')).toBe(true) + setTimeout(() => trigger.click(), 10) + }) + modalEl1.addEventListener('hidden.bs.modal', () => { + expect(document.body.classList.contains('modal-open')).not.toBe(true) + }) + + modalEl2.addEventListener('shown.bs.modal', () => { + expect(document.body.classList.contains('modal-open')).toBe(true) + resolve() + }) + + modal1.show() + }) + }) + }) }) From e0eac4c347dccb20e78f9e7dd13750e680431ddc Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Thu, 8 Sep 2022 21:21:32 +0200 Subject: [PATCH 6/7] Shorten test case codes to check for modal-open class Update code to review suggestion. Co-authored-by: GeoSot --- js/tests/unit/modal.spec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 88cef571f6..d4a5c6a294 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -98,7 +98,7 @@ describe('Modal', () => { expect(modalEl.getAttribute('aria-hidden')).toBeNull() expect(modalEl.style.display).toEqual('block') expect(document.querySelector('.modal-backdrop')).not.toBeNull() - expect(document.body.classList.contains('modal-open')).toBe(true) + expect(document.body).toHaveClasss('modal-open') resolve() }) @@ -699,7 +699,7 @@ describe('Modal', () => { expect(modalEl.getAttribute('aria-hidden')).toEqual('true') expect(modalEl.style.display).toEqual('none') expect(backdropSpy).toHaveBeenCalled() - expect(document.body.classList.contains('modal-open')).not.toBe(true) + expect(document.body).not.toHaveClass('modal-open') resolve() }) @@ -1279,15 +1279,15 @@ describe('Modal', () => { const modal1 = new Modal(modalEl1) modalEl1.addEventListener('shown.bs.modal', () => { - expect(document.body.classList.contains('modal-open')).toBe(true) + expect(document.body).toHaveClass('modal-open')) setTimeout(() => trigger.click(), 10) }) modalEl1.addEventListener('hidden.bs.modal', () => { - expect(document.body.classList.contains('modal-open')).not.toBe(true) + expect(document.body).not.toHaveClass('modal-open') }) modalEl2.addEventListener('shown.bs.modal', () => { - expect(document.body.classList.contains('modal-open')).toBe(true) + expect(document.body).toHaveClass('modal-open') resolve() }) From 0546612a5f36c56cffc67131b5acb0a2b6bc1b0d Mon Sep 17 00:00:00 2001 From: Son Tung Ngo <18316612+nstungcom@users.noreply.github.com> Date: Tue, 4 Oct 2022 21:29:20 +0200 Subject: [PATCH 7/7] Fix syntax error in `toggle between modals` test --- js/tests/unit/modal.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index a97c94e38f..b2a8e2ccc1 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -1313,7 +1313,7 @@ describe('Modal', () => { const modal1 = new Modal(modalEl1) modalEl1.addEventListener('shown.bs.modal', () => { - expect(document.body).toHaveClass('modal-open')) + expect(document.body).toHaveClass('modal-open') setTimeout(() => trigger.click(), 10) }) modalEl1.addEventListener('hidden.bs.modal', () => {