fix: take the closing flag back when a close is vetoed

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 2.3.1
+    fix: a close vetoed by a submit or cancel handler bound after dialogify's own no longer leaves the dialog stuck in the closing state, where pointer-events: none made it impossible to interact with
+    fix: the closing styles now require the dialog to have actually closed, so a lingering flag can never take an open dialog's pointer events away
+
 ## 2.3.0
     BREAKING: a modal dialog now freezes the page behind it by default; pass backgroundScroll: true to restore the old behaviour (non-modal dialogs are unchanged)
     fix: locking the page no longer shifts fixed elements sideways, because the scrollbar gutter is reserved instead of padded away

package.json

@@ -1,10 +1,10 @@
 {
     "name": "@oneup_network/dialogify",
-    "version": "2.3.0",
+    "version": "2.3.1",
     "description": "A dialog plugin built on the native <dialog> element.",
     "repository": {
         "type": "git",
-        "url": "https://github.com/OneupNetwork/dialogify"
+        "url": "git+https://github.com/OneupNetwork/dialogify.git"
     },
     "license": "MIT",
     "type": "module",

src/js/core.js

@@ -221,6 +221,9 @@ class Dialogify {
                 self._teardownAnchor();
                 $(self).triggerHandler('close');
                 self._unlockScroll();
+                // The close went through, so the optimistic flag stands and the
+                // guard that would have taken it back is no longer wanted.
+                self._clearClosingGuard();
                 // Keep the dialog rendered while the exit animation plays; it
                 // is already out of the top layer at this point. Normally the
                 // class is already there (see `_markClosing`), this is the
@@ -418,6 +421,7 @@ class Dialogify {
         const self = this;
         // A dialog reopened during its exit animation must not stay in the
         // closing state.
+        this._clearClosingGuard();
         $(this.dialog).removeClass('dialogify--closing');
         if (!this._closePromise) {
             this._closePromise = new Promise(function (resolve) {
@@ -618,9 +622,33 @@ class Dialogify {
     // the dialog has already lost its `open` attribute and would be hidden by
     // `display: none`. A paint landing in that gap makes the dialog blink out
     // and back in at full opacity before the exit animation runs.
+    //
+    // The flag is optimistic, so it has to be able to take itself back: a
+    // submit or cancel handler bound after ours can still call
+    // `preventDefault()` and leave the dialog open, and a dialog stuck in the
+    // closing state stops accepting input. The dialog has lost its `open`
+    // attribute by the next task whenever the close went through, so anything
+    // still open by then was vetoed.
     _markClosing() {
-        if (this.dialog.open) {
-            $(this.dialog).addClass('dialogify--closing');
+        if (!this.dialog.open) {
+            return;
+        }
+
+        const self = this;
+        $(this.dialog).addClass('dialogify--closing');
+        this._clearClosingGuard();
+        this._closingGuard = window.setTimeout(function () {
+            self._closingGuard = null;
+            if (self.dialog.open) {
+                $(self.dialog).removeClass('dialogify--closing');
+            }
+        }, 0);
+    }
+
+    _clearClosingGuard() {
+        if (this._closingGuard) {
+            window.clearTimeout(this._closingGuard);
+            this._closingGuard = null;
         }
     }
 

src/scss/dialogify.scss

@@ -823,8 +823,11 @@ $dialogify-toast-z-index: 1010;
 }
 
 // The dialog is closed but kept in the DOM until its exit animation ends, so
-// it has to stay rendered while no longer accepting input.
-.dialogify.dialogify--closing {
+// it has to stay rendered while no longer accepting input. Guarded on
+// `:not([open])` because the class is set optimistically, a beat before the
+// dialog actually closes: a handler can still veto that close, and an open
+// dialog must never lose its pointer events.
+.dialogify.dialogify--closing:not([open]) {
     display: block;
     pointer-events: none;
 }
@@ -840,7 +843,7 @@ $dialogify-toast-z-index: 1010;
     }
 
     // without an exit animation there is nothing to wait for, so hide it at once
-    .dialogify.dialogify--closing {
+    .dialogify.dialogify--closing:not([open]) {
         display: none;
     }
 }

test/features.test.js

@@ -976,6 +976,53 @@ describe('exit animation', () => {
         expect(document.body.contains(d.dialog)).toBe(false);
     });
 
+    it('takes the closing state back when a submit handler vetoes the close', async () => {
+        const d = new Dialogify('content');
+        d.showModal();
+
+        // Bound after dialogify's own handler, so it runs later and can still
+        // veto a close the dialog has already been flagged for.
+        d.$form[0].addEventListener('submit', (e) => e.preventDefault());
+        d.$form[0].dispatchEvent(new window.Event('submit', { bubbles: true, cancelable: true }));
+
+        expect(d.dialog.classList.contains('dialogify--closing')).toBe(true);
+
+        await new Promise((r) => setTimeout(r, 0));
+
+        expect(d.isOpen()).toBe(true);
+        expect(d.dialog.classList.contains('dialogify--closing')).toBe(false);
+    });
+
+    it('takes the closing state back when a cancel handler vetoes the close', async () => {
+        const d = new Dialogify('content', { useDialogForm: false });
+        d.showModal();
+
+        d.dialog.addEventListener('cancel', (e) => e.preventDefault());
+        d.dialog.dispatchEvent(new window.Event('cancel', { cancelable: true }));
+
+        await new Promise((r) => setTimeout(r, 0));
+
+        expect(d.isOpen()).toBe(true);
+        expect(d.dialog.classList.contains('dialogify--closing')).toBe(false);
+    });
+
+    it('keeps the closing state on a close that goes through', async () => {
+        const d = new Dialogify('content', {
+            position: Dialogify.POSITION_RIGHT,
+            autoRemove: false
+        });
+        d.dialog.getAnimations = () => [{ finished: new Promise(() => {}) }];
+
+        d.showModal();
+        d.close();
+
+        // The guard that takes the flag back must not fire for a real close,
+        // or the exit animation would be cut off after one task.
+        await new Promise((r) => setTimeout(r, 0));
+
+        expect(d.dialog.classList.contains('dialogify--closing')).toBe(true);
+    });
+
     it('drops the closing state when the dialog is reopened', async () => {
         const d = new Dialogify('content', {
             position: Dialogify.POSITION_RIGHT,
@@ -1110,6 +1157,23 @@ describe('background scroll lock', () => {
     });
 });
 
+describe('closing state stylesheet', () => {
+    const css = readFileSync('src/css/dialogify.css', 'utf8');
+
+    it('only withdraws pointer events once the dialog has actually closed', () => {
+        // The class is set optimistically, so an open dialog whose close was
+        // vetoed must keep its pointer events even if the flag lingers.
+        expect(css).toContain(
+            '.dialogify.dialogify--closing:not([open]){display:block;pointer-events:none}'
+        );
+        expect(css).not.toMatch(/\.dialogify\.dialogify--closing\{/);
+    });
+
+    it('hides the dialog under reduced motion only once it has closed', () => {
+        expect(css).toContain('.dialogify.dialogify--closing:not([open]){display:none}');
+    });
+});
+
 describe('stacking', () => {
     // The built stylesheet is committed, so it can be asserted directly; jsdom
     // never loads it, which rules out a getComputedStyle test.