From a0c2a39dc7d2f4984855306c5711fb65d22fc72b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 3 Feb 2020 17:59:13 +0100 Subject: [PATCH 1/7] make a static dialog close again if background is clicked --- src/Modal.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index b6215b2b5a..54e56edd72 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -217,9 +217,13 @@ class ModalManager { } closeAll() { - const modalsToClose = [...this._modals, this._priorityModal]; + const modalsToClose = this._modals.slice(); this._modals = []; - this._priorityModal = null; + + if (this._priorityModal) { + modalsToClose.push(this._priorityModal); + this._priorityModal = null; + } if (this._staticModal && modalsToClose.length === 0) { modalsToClose.push(this._staticModal); From 7e07a42dc14594e640598ae4613e87e61b9e3155 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 13:07:13 +0100 Subject: [PATCH 2/7] resolve finished promise when closing dialog by clicking background ... by calling the same close method as otherwise and not have a special path that just calls the onFinished callback. This will also not close all the dialogs anymore, but that sort of seems like the intented behaviour? --- src/Modal.js | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index 54e56edd72..33c3140ff1 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -47,7 +47,7 @@ class ModalManager { } */ ]; - this.closeAll = this.closeAll.bind(this); + this.onBackgroundClick = this.onBackgroundClick.bind(this); } hasDialogs() { @@ -124,6 +124,7 @@ class ModalManager { ); modal.onFinished = props ? props.onFinished : null; modal.className = className; + modal.close = closeDialog; return {modal, closeDialog, onFinishedProm}; } @@ -216,28 +217,16 @@ class ModalManager { }; } - closeAll() { - const modalsToClose = this._modals.slice(); - this._modals = []; - - if (this._priorityModal) { - modalsToClose.push(this._priorityModal); - this._priorityModal = null; + onBackgroundClick() { + const modal = this._getCurrentModal(); + if (!modal) { + return; } + modal.close(); + } - if (this._staticModal && modalsToClose.length === 0) { - modalsToClose.push(this._staticModal); - this._staticModal = null; - } - - for (let i = 0; i < modalsToClose.length; i++) { - const m = modalsToClose[i]; - if (m && m.onFinished) { - m.onFinished(false); - } - } - - this._reRender(); + _getCurrentModal() { + return this._priorityModal ? this._priorityModal : (this._modals[0] || this._staticModal); } _reRender() { @@ -268,7 +257,7 @@ class ModalManager {
{ this._staticModal.elem }
-
+
); @@ -278,8 +267,8 @@ class ModalManager { ReactDOM.unmountComponentAtNode(this.getOrCreateStaticContainer()); } - const modal = this._priorityModal ? this._priorityModal : this._modals[0]; - if (modal) { + const modal = this._getCurrentModal(); + if (modal !== this._staticModal) { const classes = "mx_Dialog_wrapper " + (this._staticModal ? "mx_Dialog_wrapperWithStaticUnder " : '') + (modal.className ? modal.className : ''); @@ -289,7 +278,7 @@ class ModalManager {
{modal.elem}
-
+
); From c44ebef06f3298e5abc34cb453a27ddf7ae26287 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 13:10:06 +0100 Subject: [PATCH 3/7] add onBeforeClose option to Modal so we can throw up another "are you sure" dialog in the cases we want to do so. This also passes a reason so we can only do so for ways of dismissing (like backgroundClick) that are easy to do by accident. --- src/Modal.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index 33c3140ff1..2e5624aa67 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -106,7 +106,7 @@ class ModalManager { return this.appendDialogAsync(...rest); } - _buildModal(prom, props, className) { + _buildModal(prom, props, className, options) { const modal = {}; // never call this from onFinished() otherwise it will loop @@ -124,14 +124,27 @@ class ModalManager { ); modal.onFinished = props ? props.onFinished : null; modal.className = className; + modal.onBeforeClose = options.onBeforeClose; + modal.beforeClosePromise = null; modal.close = closeDialog; + modal.closeReason = null; return {modal, closeDialog, onFinishedProm}; } _getCloseFn(modal, props) { const deferred = defer(); - return [(...args) => { + return [async (...args) => { + if (modal.beforeClosePromise) { + await modal.beforeClosePromise; + } else if (modal.onBeforeClose) { + modal.beforeClosePromise = modal.onBeforeClose(modal.closeReason); + const shouldClose = await modal.beforeClosePromise; + modal.beforeClosePromise = null; + if (!shouldClose) { + return; + } + } deferred.resolve(args); if (props && props.onFinished) props.onFinished.apply(null, args); const i = this._modals.indexOf(modal); @@ -186,9 +199,9 @@ class ModalManager { * static at a time. * @returns {object} Object with 'close' parameter being a function that will close the dialog */ - createDialogAsync(prom, props, className, isPriorityModal, isStaticModal) { - const {modal, closeDialog, onFinishedProm} = this._buildModal(prom, props, className); - + createDialogAsync(prom, props, className, isPriorityModal, isStaticModal, options = {}) { + const {modal, closeDialog, onFinishedProm} = this._buildModal(prom, props, className, options); + modal.close = closeDialog; if (isPriorityModal) { // XXX: This is destructive this._priorityModal = modal; @@ -207,7 +220,7 @@ class ModalManager { } appendDialogAsync(prom, props, className) { - const {modal, closeDialog, onFinishedProm} = this._buildModal(prom, props, className); + const {modal, closeDialog, onFinishedProm} = this._buildModal(prom, props, className, {}); this._modals.push(modal); this._reRender(); @@ -222,7 +235,13 @@ class ModalManager { if (!modal) { return; } + // we want to pass a reason to the onBeforeClose + // callback, but close is currently defined to + // pass all number of arguments to the onFinished callback + // so, pass the reason to close through a member variable + modal.closeReason = "backgroundClick"; modal.close(); + modal.closeReason = null; } _getCurrentModal() { From 70a4d3415eae1356fc3d3228a5a5d9e054264b4e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 13:11:24 +0100 Subject: [PATCH 4/7] confirm to close the passphrase dialog if it was done by backgroundClick as it is easy to do by accident --- src/CrossSigningManager.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/CrossSigningManager.js b/src/CrossSigningManager.js index a560c956f1..8feb651ea5 100644 --- a/src/CrossSigningManager.js +++ b/src/CrossSigningManager.js @@ -70,6 +70,7 @@ async function getSecretStorageKey({ keys: keyInfos }) { sdk.getComponent("dialogs.secretstorage.AccessSecretStorageDialog"); const { finished } = Modal.createTrackedDialog("Access Secret Storage dialog", "", AccessSecretStorageDialog, + /* props= */ { keyInfo: info, checkPrivateKey: async (input) => { @@ -77,6 +78,22 @@ async function getSecretStorageKey({ keys: keyInfos }) { return MatrixClientPeg.get().checkSecretStoragePrivateKey(key, info.pubkey); }, }, + /* className= */ null, + /* isPriorityModal= */ false, + /* isStaticModal= */ false, + /* options= */ { + onBeforeClose: async (reason) => { + if (reason !== "backgroundClick") { + return true; + } + const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); + const [sure] = await Modal.createDialog(QuestionDialog, { + title: _t("Cancel entering passphrase?"), + description: _t("If you cancel now, you won't complete your SSSS operation!"), + }).finished; + return sure; + }, + }, ); const [input] = await finished; if (!input) { From 4cd4110a52d867c4e22bf1ef87f7b43a1f97edd4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 13:13:37 +0100 Subject: [PATCH 5/7] fixup: this is already done in _buildModal --- src/Modal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Modal.js b/src/Modal.js index 2e5624aa67..f55e497e6f 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -201,7 +201,6 @@ class ModalManager { */ createDialogAsync(prom, props, className, isPriorityModal, isStaticModal, options = {}) { const {modal, closeDialog, onFinishedProm} = this._buildModal(prom, props, className, options); - modal.close = closeDialog; if (isPriorityModal) { // XXX: This is destructive this._priorityModal = modal; From cf7ad725a60765f8d98cbe1234e172e895cdaf39 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 15:06:36 +0100 Subject: [PATCH 6/7] copy and i18n --- src/CrossSigningManager.js | 2 +- src/i18n/strings/en_EN.json | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/CrossSigningManager.js b/src/CrossSigningManager.js index 8feb651ea5..15daede92b 100644 --- a/src/CrossSigningManager.js +++ b/src/CrossSigningManager.js @@ -89,7 +89,7 @@ async function getSecretStorageKey({ keys: keyInfos }) { const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); const [sure] = await Modal.createDialog(QuestionDialog, { title: _t("Cancel entering passphrase?"), - description: _t("If you cancel now, you won't complete your SSSS operation!"), + description: _t("If you cancel now, you won't complete your secret storage operation!"), }).finished; return sure; }, diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index d125d10cfb..6fb4b86aac 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -60,6 +60,8 @@ "Server may be unavailable, overloaded, or you hit a bug.": "Server may be unavailable, overloaded, or you hit a bug.", "The server does not support the room version specified.": "The server does not support the room version specified.", "Failure to create room": "Failure to create room", + "Cancel entering passphrase?": "Cancel entering passphrase?", + "If you cancel now, you won't complete your secret storage operation!": "If you cancel now, you won't complete your secret storage operation!", "Setting up keys": "Setting up keys", "Send anyway": "Send anyway", "Send": "Send", From a8958458aae19f546c185b67cc6b6ba153a48fb4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 6 Feb 2020 15:29:35 +0100 Subject: [PATCH 7/7] fix lint, add jsdoc --- src/Modal.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Modal.js b/src/Modal.js index f55e497e6f..de441740f1 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -170,6 +170,12 @@ class ModalManager { }, deferred.promise]; } + /** + * @callback onBeforeClose + * @param {string?} reason either "backgroundClick" or null + * @return {Promise} whether the dialog should close + */ + /** * Open a modal view. * @@ -197,6 +203,8 @@ class ModalManager { * also be removed from the stack. This is not compatible * with being a priority modal. Only one modal can be * static at a time. + * @param {Object} options? extra options for the dialog + * @param {onBeforeClose} options.onBeforeClose a callback to decide whether to close the dialog * @returns {object} Object with 'close' parameter being a function that will close the dialog */ createDialogAsync(prom, props, className, isPriorityModal, isStaticModal, options = {}) {