Page MenuHomePhorge

Show confirmation dialog when closing a modal if form contents have been changed
Closed, ResolvedPublic


I think there has been an open issue somewhere about this for some time. When creating a Maniphest task via a Workboard modal, if you press ESC accidentally, changes are immediately lost. This is a potential patch that could use improvement or is at least ready for testing:

Index: webroot/rsrc/externals/javelin/lib/Workflow.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
diff --git a/webroot/rsrc/externals/javelin/lib/Workflow.js b/webroot/rsrc/externals/javelin/lib/Workflow.js
--- a/webroot/rsrc/externals/javelin/lib/Workflow.js	(revision 69cb76092142c89c83115a4970cac0a079c097dd)
+++ b/webroot/rsrc/externals/javelin/lib/Workflow.js	(date 1626182217913)
@@ -403,6 +403,16 @@
           JX.$E('Response to workflow request went unhandled.');
+      var form = JX.DOM.scry(this._root, 'form', 'jx-dialog');
+      if (form.length) {
+        JX.DOM.listen(form[0], 'keydown', null, function(e) {
+          if (e.getSpecialKey()) {
+            return;
+          }
+          JX.Stratcom.addSigil(form[0], 'dialog-keydown');
+        });
+      }
     _push : function() {
       if (!this._pushed) {
@@ -535,6 +545,15 @@
         // No 'Cancel' button.
+      var form = JX.DOM.scry(active._root, 'form', 'jx-dialog');
+      if (
+        form.length &&
+        JX.Stratcom.hasSigil(form[0], 'dialog-keydown') &&
+        !confirm('Form data may have changed. Are you sure you want to close this dialog?')
+      ) {
+        return;
+      }

The idea here is that the main issue probably arises when a user has put forth a notable amount of effort with typing many words into a modal and accidentally presses ESC. So this checks for a keydown event. If no keydown event has occurred, then there is no confirmation like normal. Otherwise there is a confirmation before the modal closes.

The verbiage for the confirmation really ought to be changed to something else. Further, either me or someone else needs to look into how to implement internationalization for that text.

Event Timeline

Hit esc today - Lost lots of typing

Just an upvote for landing D25015

Hit esc today - Lost lots of typing

Just an upvote for landing D25015

This diff will break javascript handling seriously. Related issues: Q21, Q26, Q27. See my comment here:

Can't reproduce on this instance, because this instance seems to run on stable (?)

D25015 was by @dcog If it can't be fixed with an update, this should be reverted

I've proposed a fix for the regression here:


D25015 was by @dcog If it can't be fixed with an update, this should be reverted

Obviously, I proposed a fix since I love this change and I don't want to see it rollbacked 😏 thank you for this confirmation dialog

valerio.bozzolan assigned this task to dcog.

Thank you @dcog for this Dialog confirmation that saves my day.

Thank you @bekay for raising the regression.

Since now it works and there are no regressions (D25076: Fix regression in new confirmation Dialog) - we can mark this as resolved.

I noticed that, if you click on whatever other link while you are editing something, the Dialog is not show, and you just can lose your work.

I think this can happen frequently, since when you render your text as demo, you can click by mistake on a link. Also, when you upload a file you can click by mistake on a link, etc.

So, probably this Confirmation Dialog could be "hardened" to also listen to the onblur event of the whole document, instead of just listening on the the ESC key etc.

This has sense not just in a modal, but in whatever form honestly.