From 7fc0963e3cc855e5476dda7792bc9400bcca0e92 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 25 Sep 2020 12:45:46 -0400 Subject: [PATCH 1/6] Revert "Fix opening modals from other modals. (#2263)" This reverts commit 5b157f0adb576ea0cea4b85ec756feddf07b7ba3. --- js/src/common/components/Modal.js | 4 ++++ js/src/common/components/ModalManager.js | 6 ------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index b554b255e..2a59beb01 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -27,6 +27,10 @@ export default class Modal extends Component { this.attrs.onshow(() => this.onready()); } + onremove() { + this.attrs.onhide(); + } + view() { if (this.alertAttrs) { this.alertAttrs.dismissible = false; diff --git a/js/src/common/components/ModalManager.js b/js/src/common/components/ModalManager.js index 8d13fe434..35922f70d 100644 --- a/js/src/common/components/ModalManager.js +++ b/js/src/common/components/ModalManager.js @@ -16,12 +16,6 @@ export default class ModalManager extends Component { ); } - onupdate() { - if (this.$('.Modal') && !this.attrs.state.modal) { - this.animateHide(); - } - } - oncreate(vnode) { super.oncreate(vnode); From b7593bc6a8a822131ff734324e30a35252d04a59 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 25 Sep 2020 12:54:03 -0400 Subject: [PATCH 2/6] Prevent hide animation when opening modal from other modal --- js/src/common/components/Modal.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index 2a59beb01..b08f73b3a 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -27,8 +27,13 @@ export default class Modal extends Component { this.attrs.onshow(() => this.onready()); } - onremove() { - this.attrs.onhide(); + onbeforeremove() { + // If the global modal state currently contains a modal, + // we've just opened up a new one, and accordingly, + // we don't need to show a hide animation. + if (!app.modal.modal) { + this.attrs.onhide(); + } } view() { @@ -107,7 +112,7 @@ export default class Modal extends Component { * Hide the modal. */ hide() { - this.attrs.onhide(); + app.modal.close(); } /** From be8fe44f0bee2ea9dc0a8cb7fae2f2acfc3a21d9 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 25 Sep 2020 13:00:44 -0400 Subject: [PATCH 3/6] Ensure that readyCallback is called on modals opened from other modals --- js/src/common/components/ModalManager.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/js/src/common/components/ModalManager.js b/js/src/common/components/ModalManager.js index 35922f70d..1b063552e 100644 --- a/js/src/common/components/ModalManager.js +++ b/js/src/common/components/ModalManager.js @@ -28,6 +28,13 @@ export default class ModalManager extends Component { animateShow(readyCallback) { const dismissible = !!this.attrs.state.modal.componentClass.isDismissible; + // If we are opening this modal while another modal is already open, + // the shown event will not run, because the modal is already open. + // So, we need to manually trigger the readyCallback. + if (this.$().hasClass('in')) { + readyCallback(); + } + this.$() .one('shown.bs.modal', readyCallback) .modal({ From 1ac09dbc4de867a031111e3de2a4f1a7611faa35 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 25 Sep 2020 13:02:10 -0400 Subject: [PATCH 4/6] Pass ModalManagerState into Modal instances instead of calling the global. --- js/src/common/components/Modal.js | 4 ++-- js/src/common/components/ModalManager.js | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index b08f73b3a..97e3a273e 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -31,7 +31,7 @@ export default class Modal extends Component { // If the global modal state currently contains a modal, // we've just opened up a new one, and accordingly, // we don't need to show a hide animation. - if (!app.modal.modal) { + if (!this.attrs.state.modal) { this.attrs.onhide(); } } @@ -112,7 +112,7 @@ export default class Modal extends Component { * Hide the modal. */ hide() { - app.modal.close(); + this.attrs.state.close(); } /** diff --git a/js/src/common/components/ModalManager.js b/js/src/common/components/ModalManager.js index 1b063552e..124eed226 100644 --- a/js/src/common/components/ModalManager.js +++ b/js/src/common/components/ModalManager.js @@ -11,7 +11,14 @@ export default class ModalManager extends Component { return (
- {modal ? modal.componentClass.component({ ...modal.attrs, onshow: this.animateShow.bind(this), onhide: this.animateHide.bind(this) }) : ''} + {modal + ? modal.componentClass.component({ + ...modal.attrs, + onshow: this.animateShow.bind(this), + onhide: this.animateHide.bind(this), + state: this.attrs.state, + }) + : ''}
); } From a2263b8538d79500744e111dbf298c45042daaca Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 25 Sep 2020 16:37:32 -0400 Subject: [PATCH 5/6] Return on animateShow if already loaded --- js/src/common/components/ModalManager.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/common/components/ModalManager.js b/js/src/common/components/ModalManager.js index 124eed226..b4c8814b3 100644 --- a/js/src/common/components/ModalManager.js +++ b/js/src/common/components/ModalManager.js @@ -40,6 +40,7 @@ export default class ModalManager extends Component { // So, we need to manually trigger the readyCallback. if (this.$().hasClass('in')) { readyCallback(); + return; } this.$() From 9d1a87a4c445c16a04447deb356d65bfc0ea5e50 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 29 Sep 2020 18:37:26 -0400 Subject: [PATCH 6/6] Rename onshow and onhide animateShow and animateHide are more descriptive --- js/src/common/components/Modal.js | 4 ++-- js/src/common/components/ModalManager.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index 97e3a273e..0d621060a 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -24,7 +24,7 @@ export default class Modal extends Component { oncreate(vnode) { super.oncreate(vnode); - this.attrs.onshow(() => this.onready()); + this.attrs.animateShow(() => this.onready()); } onbeforeremove() { @@ -32,7 +32,7 @@ export default class Modal extends Component { // we've just opened up a new one, and accordingly, // we don't need to show a hide animation. if (!this.attrs.state.modal) { - this.attrs.onhide(); + this.attrs.animateHide(); } } diff --git a/js/src/common/components/ModalManager.js b/js/src/common/components/ModalManager.js index b4c8814b3..9d0609c18 100644 --- a/js/src/common/components/ModalManager.js +++ b/js/src/common/components/ModalManager.js @@ -14,8 +14,8 @@ export default class ModalManager extends Component { {modal ? modal.componentClass.component({ ...modal.attrs, - onshow: this.animateShow.bind(this), - onhide: this.animateHide.bind(this), + animateShow: this.animateShow.bind(this), + animateHide: this.animateHide.bind(this), state: this.attrs.state, }) : ''}