Page MenuHomePhorge

Show confirmation dialog when closing a modal if form contents have been changed
Open, Needs TriagePublic


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