From ad1d0bf6f99104403dcd84debbe42be276c8b001 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 15:32:47 -0700 Subject: [PATCH] Rename modal dismissal reason to source, refs #2790 --- datasette/static/edit-tools.js | 10 +++++----- datasette/static/modal.js | 4 ++-- docs/javascript_plugins.rst | 13 +++++++++++-- tests/test_playwright.py | 22 +++++++++++----------- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index 2eeb6b21..5d068981 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -2351,7 +2351,7 @@ function ensureTableCreateDialog(manager) { updateTableCreateDialogButtons(tableCreateDialogState); }); - modal.beforeClose = function (reason) { + modal.beforeClose = function (source) { return confirmDiscardTableCreateChanges(tableCreateDialogState); }; @@ -4017,9 +4017,9 @@ function ensureTableAlterDialog(manager) { } }); - modal.beforeClose = function (reason) { + modal.beforeClose = function (source) { return ( - reason === "cancel" || + source === "cancel" || confirmDiscardTableAlterChanges(tableAlterDialogState) ); }; @@ -7220,9 +7220,9 @@ function ensureRowEditDialog(manager) { }, ); - modal.beforeClose = function (reason) { + modal.beforeClose = function (source) { return ( - reason === "cancel" || confirmDiscardRowEditChanges(rowEditDialogState) + source === "cancel" || confirmDiscardRowEditChanges(rowEditDialogState) ); }; diff --git a/datasette/static/modal.js b/datasette/static/modal.js index b252af21..5230f81d 100644 --- a/datasette/static/modal.js +++ b/datasette/static/modal.js @@ -147,9 +147,9 @@ else initialFocus?.focus(); } - requestClose(reason = "cancel") { + requestClose(source = "cancel") { if (!this.dialog.open || this.busy) return false; - if (this.beforeClose && this.beforeClose(reason) === false) return false; + if (this.beforeClose && this.beforeClose(source) === false) return false; this.close(); return true; } diff --git a/docs/javascript_plugins.rst b/docs/javascript_plugins.rst index 73136ba7..c2bef718 100644 --- a/docs/javascript_plugins.rst +++ b/docs/javascript_plugins.rst @@ -538,9 +538,11 @@ Opening and closing ``modal.show({trigger, initialFocus})`` Opens the native dialog using ``showModal()``. Both options are optional. ``trigger`` is the element to return focus to when the dialog closes; it defaults to the currently focused element. ``initialFocus`` can be an element to focus or a function that focuses a custom control. Without it, the browser chooses initial focus. Calling ``show()`` again while the dialog is open does not change where focus returns when it closes. For example, if an Edit button opened the dialog, focus will still return to that button. -``modal.requestClose(reason = "cancel")`` +``modal.requestClose(source)`` Requests dismissal through the busy-state and ``beforeClose`` guards described below. Returns ``true`` if it closes the dialog, or ``false`` if the dialog is already closed or a guard prevents dismissal. Close and Cancel buttons should use this method. + ``source`` is an optional string identifying what requested dismissal. It is passed to ``beforeClose`` and is never displayed to the user. Datasette supplies ``"escape"`` for the Escape key or a native cancel event and ``"backdrop"`` for a click outside the dialog. Calls to ``requestClose()`` default to ``"cancel"``; callers can supply any other string as their own identifier. + ``modal.close({restoreFocus = true})`` Closes the dialog directly, bypassing the guards. Use this after successfully completing an operation. Pass ``restoreFocus: false`` when your code will navigate away or move focus to another element, such as a newly inserted row. @@ -559,7 +561,14 @@ If the dialog is no longer needed, remove the wrapper with ``modal.remove()``. T Dismissal guards and busy state ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Set ``modal.beforeClose`` to a synchronous function that receives a dismissal reason and returns ``false`` to keep the dialog open. The built-in dismissal reasons are ``"escape"`` for Escape or a native cancel event and ``"backdrop"`` for a click outside the dialog. Buttons can pass ``"cancel"`` to ``requestClose()``. Your callback can apply different policies to each reason, such as prompting before discarding edits on Escape while allowing an explicit Cancel button to close immediately. +Set ``modal.beforeClose`` to a synchronous function that receives the ``source`` string described above and returns ``false`` to keep the dialog open. Your callback can apply different policies to each source, such as prompting before discarding edits on Escape while allowing an explicit Cancel button to close immediately. Any message shown to the user is supplied by your callback, separately from the source identifier: + +.. code-block:: javascript + + modal.beforeClose = (source) => { + if (source === "cancel") return true; + return confirm("Discard unsaved changes?"); + }; The callback must return synchronously: returning a Promise does not delay dismissal. For an asynchronous confirmation, return ``false`` immediately and call ``modal.close()`` yourself if the user later confirms. diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 158b2cb7..9b5bfc3e 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1761,9 +1761,9 @@ def test_modal_lifecycle(page, datasette_server): // Padding is part of the dialog, never a backdrop dismissal. dialog.style.padding = '30px'; document.body.append(testModal); - window.closeReasons = []; - testModal.beforeClose = reason => { - closeReasons.push(reason); + window.closeSources = []; + testModal.beforeClose = source => { + closeSources.push(source); return window.allowClose; }; window.allowClose = false; @@ -1788,23 +1788,23 @@ def test_modal_lifecycle(page, datasette_server): expect(dialog.get_by_role("textbox", name="First field")).to_be_focused() page.keyboard.down("Escape") - assert page.evaluate("closeReasons") == [] + assert page.evaluate("closeSources") == [] page.keyboard.up("Escape") - page.wait_for_function("closeReasons.length === 1") - assert page.evaluate("closeReasons") == ["escape"] + page.wait_for_function("closeSources.length === 1") + assert page.evaluate("closeSources") == ["escape"] expect(dialog).to_be_visible() dialog.click(position={"x": 3, "y": 3}) - assert page.evaluate("closeReasons") == ["escape"] + assert page.evaluate("closeSources") == ["escape"] # A drag which starts inside and ends on the backdrop must not dismiss. box = dialog.bounding_box() page.mouse.move(box["x"] + 3, box["y"] + 3) page.mouse.down() page.mouse.move(2, 2) page.mouse.up() - assert page.evaluate("closeReasons") == ["escape"] + assert page.evaluate("closeSources") == ["escape"] page.mouse.click(2, 2) - assert page.evaluate("closeReasons") == ["escape", "backdrop"] + assert page.evaluate("closeSources") == ["escape", "backdrop"] page.evaluate("testModal.busy = true; allowClose = true") expect(dialog).to_have_attribute("aria-busy", "true") @@ -1812,12 +1812,12 @@ def test_modal_lifecycle(page, datasette_server): page.mouse.click(2, 2) dialog.get_by_role("button", name="Cancel").click() expect(dialog).to_be_visible() - assert page.evaluate("closeReasons") == ["escape", "backdrop"] + assert page.evaluate("closeSources") == ["escape", "backdrop"] page.evaluate("testModal.busy = false") dialog.get_by_role("button", name="Cancel").click() expect(dialog).not_to_be_visible() expect(trigger).to_be_focused() - assert page.evaluate("closeReasons") == ["escape", "backdrop", "cancel"] + assert page.evaluate("closeSources") == ["escape", "backdrop", "cancel"] # Reopening, including an extra show() call, preserves the original trigger. trigger.click()