Fix popup onClosing handling not working correctly - Reset value/result on canceled closing - Fix close event still firing and closing the popup on multiple close/cancel calls, even though it shouldn't have happened - Remove manual removal of popup event listeners. Not needed, if they are only subscribed to controls inside the dialog or the dialog itself. That's cleaned up automatically. Is confusing otherwise anyway.

7b7c1121bb41d9ba8ba990196beabbfe9d88cbc4

Wolfsblvt <wolfsblvt@gmail.com>

public/scripts/popup.js+27 -8 Viewed

             evt.preventDefault();
             evt.stopPropagation();
             await this.complete(POPUP_RESULT.CANCELLED);
-            window.removeEventListener('cancel', cancelListenerBound);
         };
-        const cancelListenerBound = cancelListener.bind(this);
+        this.dlg.addEventListener('cancel', cancelListener.bind(this));
-        this.dlg.addEventListener('cancel', cancelListenerBound);
+        // Don't ask me why this is needed. I don't get it. But we have to keep it.
+        // We make sure that the modal on it's own doesn't hide. Dunno why, if onClosing is triggered multiple times through the cancel event, and stopped
+        // It seems to just call 'close' on the dialog even if the 'cancel' event was prevented.
+        // Here, we just say that close should not happen if the dalog has no result.
+        const closeListener = async (evt) => {
+            if (this.result === undefined) {
+                evt.preventDefault();
+                evt.stopPropagation();
+                this.dlg.showModal();
+            }
+        };
+        this.dlg.addEventListener('close', closeListener.bind(this));
         const keyListener = async (evt) => {
             switch (evt.key) {

@@ -366,16 +377,16 @@ export class Popup {
366	evt.preventDefault();	377	evt.preventDefault();
367	evt.stopPropagation();	378	evt.stopPropagation();
368	const result = Number(document.activeElement.getAttribute('data-result') ?? this.defaultResult);	379	const result = Number(document.activeElement.getAttribute('data-result') ?? this.defaultResult);
		380
		381	// Call complete on the popup. Make sure that we handle `onClosing` cancels correctly and don't remove the listener then.
369	await this.complete(result);	382	await this.complete(result);
370	window.removeEventListener('keydown', keyListenerBound);
371		383
372	break;	384	break;
373	}	385	}
374	}	386	}
375		387
376	};	388	};
377	const keyListenerBound = keyListener.bind(this);	389	this.dlg.addEventListener('keydown', keyListener.bind(this));
378	this.dlg.addEventListener('keydown', keyListenerBound);
379	}	390	}
380		391
381	/**	392	/**

      * - popup with `POPUP_TYPE.INPUT` will return the input value - or `false` on negative and `null` on cancelled
      * - All other will return the result value as provided as `POPUP_RESULT` or a custom number value
+     *
+     * <b>IMPORTANT:</b> If the popup closing was cancelled via the `onClosing` handler, the return value will be `Promise<undefined>`.
+     *
      * @param {POPUP_RESULT|number} result - The result of the popup (either an existing `POPUP_RESULT` or a custom result value)
+     *
-     * @returns {Promise<string|number|boolean?>} A promise that resolves with the value of the popup when it is completed.
+     * @returns {Promise<string|number|boolean|undefined?>} A promise that resolves with the value of the popup when it is completed. <b>Returns `undefined` if the closing action was cancelled.</b>
      */
     async complete(result) {
         // In all cases besides INPUT the popup value should be the result

         if (this.onClosing) {
             const shouldClose = this.onClosing(this);
-            if (!shouldClose) return;
+            if (!shouldClose) {
+                // Set values back if we cancel out of closing the popup
+                this.value = undefined;
+                this.result = undefined;
+                this.inputResults = undefined;
+                return undefined;
+            }
+        }
         Popup.util.lastResult = { value, result, inputResults: this.inputResults };