From c82a98c88a0263a051a5fd1eaa39ded62bde452e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:25 -0700 Subject: [PATCH] Refactor the create table dialog to use the shared modal, refs #2790 --- datasette/static/app.css | 86 +--------------------------------- datasette/static/edit-tools.js | 68 ++++++--------------------- tests/test_playwright.py | 35 ++++++++++++++ 3 files changed, 51 insertions(+), 138 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 32f1a84f..ce24cb5b 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -2077,46 +2077,8 @@ datasette-autocomplete input[type="text"], } dialog.table-create-dialog { - --ink: #0f0f0f; - --paper: #eef6ff; - --muted: #6b6b6b; - --rule: #d8e6f5; - --accent: #1a56db; - --card: #ffffff; - border: none; - border-radius: var(--modal-border-radius, 0.75rem); - padding: 0; - margin: auto; width: min(980px, calc(100vw - 32px)); - max-width: 95vw; max-height: min(780px, calc(100vh - 32px)); - box-shadow: var(--modal-shadow, 0 20px 25px -5px rgba(0, 0, 0, 0.1), 0 10px 10px -5px rgba(0, 0, 0, 0.04)); - animation: datasette-modal-slide-in var(--modal-animation-duration, 0.2s) ease-out; - overflow: hidden; - font-family: system-ui, -apple-system, sans-serif; - background: var(--card); -} - -dialog.table-create-dialog[open] { - display: flex; - flex-direction: column; -} - -dialog.table-create-dialog::backdrop { - background: var(--modal-backdrop-bg, rgba(0, 0, 0, 0.5)); - backdrop-filter: var(--modal-backdrop-blur, blur(4px)); - -webkit-backdrop-filter: var(--modal-backdrop-blur, blur(4px)); - animation: datasette-modal-fade-in var(--modal-animation-duration, 0.2s) ease-out; -} - -.table-create-dialog .modal-header { - padding: 20px 24px 12px; - border-bottom: 1px solid var(--rule); - display: flex; - align-items: center; - gap: 12px; - flex-shrink: 0; - min-width: 0; } .table-create-dialog .modal-title { @@ -2124,9 +2086,6 @@ dialog.table-create-dialog::backdrop { align-items: center; min-width: 0; max-width: 100%; - font-size: 1rem; - font-weight: 600; - color: var(--ink); } .table-create-form { @@ -2565,17 +2524,6 @@ select.table-create-input { outline-offset: 1px; } -.table-create-dialog .modal-footer { - padding: 14px 20px; - border-top: 1px solid var(--rule); - display: flex; - align-items: center; - justify-content: flex-end; - gap: 10px; - flex-shrink: 0; - background: var(--paper); -} - .table-create-mode-link { color: var(--accent); font-size: 0.9rem; @@ -2586,39 +2534,7 @@ select.table-create-input { display: none; } -.table-create-dialog .btn { - border: none; - border-radius: 5px; - padding: 9px 20px; - font-size: 0.85rem; - font-weight: 500; - cursor: pointer; - touch-action: manipulation; - font-family: inherit; - transition: background 0.12s; -} - -.table-create-dialog .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -.table-create-dialog .btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - -.table-create-dialog .btn-primary { - background: var(--accent); - color: #fff; -} - -.table-create-dialog .btn-primary:hover { - background: #1949b8; -} - -.table-create-dialog .btn:disabled, +.table-create-dialog .modal-btn:disabled, .table-create-add-column:disabled, .table-create-icon-button:disabled { opacity: 0.55; diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index 9e8b93f6..1e9b1aca 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -915,6 +915,7 @@ function showTableCreateDialogError(state, message) { function setTableCreateDialogSaving(state, isSaving) { state.isSaving = isSaving; + state.modal.busy = isSaving; state.columnList .querySelectorAll("input, select, button") .forEach(function (control) { @@ -2043,8 +2044,7 @@ async function createTableFromDataPreview(state) { var tableUrl = responseData.table_url || fallbackTableUrl(responseData.table || payload.table); - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); if (tableUrl) { location.href = tableUrl; } else { @@ -2118,8 +2118,7 @@ async function saveTableCreateDialog(state) { var tableUrl = responseData.table_url || fallbackTableUrl(responseData.table || payload.table); - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); if (tableUrl) { location.href = tableUrl; } else { @@ -2141,18 +2140,6 @@ function confirmDiscardTableCreateChanges(state) { return window.confirm("Discard this new table?"); } -function closeTableCreateDialogIfConfirmed(state) { - if (!state || state.isSaving) { - return false; - } - if (!confirmDiscardTableCreateChanges(state)) { - return false; - } - state.shouldRestoreFocus = true; - state.dialog.close(); - return true; -} - function ensureTableCreateDialog(manager) { if (tableCreateDialogState) { return tableCreateDialogState; @@ -2161,7 +2148,8 @@ function ensureTableCreateDialog(manager) { return null; } - var dialog = document.createElement("dialog"); + var modal = DatasetteModal.create(); + var dialog = modal.dialog; dialog.id = TABLE_CREATE_DIALOG_ID; dialog.className = "table-create-dialog"; dialog.setAttribute("aria-labelledby", "table-create-title"); @@ -2198,14 +2186,15 @@ function ensureTableCreateDialog(manager) {
Create table from data - - + +
`; - document.body.appendChild(dialog); + document.body.appendChild(modal); tableCreateDialogState = { + modal: modal, dialog: dialog, form: dialog.querySelector(".table-create-form"), title: dialog.querySelector(".modal-title"), @@ -2225,8 +2214,6 @@ function ensureTableCreateDialog(manager) { manualCreateLink: dialog.querySelector(".table-create-manual"), cancelButton: dialog.querySelector(".table-create-cancel"), saveButton: dialog.querySelector(".table-create-save"), - currentButton: null, - shouldRestoreFocus: true, isSaving: false, mode: "manual", dataPreviewRows: null, @@ -2266,7 +2253,7 @@ function ensureTableCreateDialog(manager) { tableCreateDialogState.dataTextarea.focus(); return; } - closeTableCreateDialogIfConfirmed(tableCreateDialogState); + modal.requestClose("cancel"); }); tableCreateDialogState.createFromDataLink.addEventListener( @@ -2364,36 +2351,14 @@ function ensureTableCreateDialog(manager) { updateTableCreateDialogButtons(tableCreateDialogState); }); - dialog.addEventListener("click", function (ev) { - if (ev.target === dialog) { - closeTableCreateDialogIfConfirmed(tableCreateDialogState); - } - }); - - dialog.addEventListener("keydown", function (ev) { - if (ev.key !== "Escape") { - return; - } - ev.preventDefault(); - closeTableCreateDialogIfConfirmed(tableCreateDialogState); - }); - - dialog.addEventListener("cancel", function (ev) { - ev.preventDefault(); - closeTableCreateDialogIfConfirmed(tableCreateDialogState); - }); + modal.beforeClose = function (reason) { + return confirmDiscardTableCreateChanges(tableCreateDialogState); + }; dialog.addEventListener("close", function () { var state = tableCreateDialogState; clearTableCreateDialogError(state); setTableCreateDialogSaving(state, false); - if ( - state.shouldRestoreFocus && - state.currentButton && - document.contains(state.currentButton) - ) { - state.currentButton.focus(); - } }); return tableCreateDialogState; @@ -2414,15 +2379,12 @@ function openTableCreateDialog(button, manager) { menu.open = false; } state.manager = manager; - state.currentButton = button; - state.shouldRestoreFocus = true; + state.title.textContent = "Create a table in " + data.databaseName; clearTableCreateDialogError(state); resetTableCreateDialog(state); loadTableCreateForeignKeyTargets(state); - if (!state.dialog.open) { - state.dialog.showModal(); - } + state.modal.show({ trigger: button }); state.tableName.focus(); } diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 0aa5fd84..63a6f0e6 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1832,3 +1832,38 @@ def test_modal_disconnect_cleans_up_pending_escape(page, datasette_server): page.keyboard.press("Escape") page.wait_for_function("closeAttempts === 1") expect(dialog).to_be_visible() + + +@pytest.mark.playwright +@pytest.mark.parametrize("kind", ["create"]) +def test_schema_modal_escape_confirmation_and_focus(page, datasette_server, kind): + from playwright.sync_api import expect + + path = "data" if kind == "create" else "data/projects" + page.goto(datasette_server + path) + menu = page.locator("details.actions-menu-links") + menu.locator("summary").click() + selector = "data-database-action" if kind == "create" else "data-table-action" + menu.locator(f'button[{selector}="{kind}-table"]').click() + dialog = page.locator(f"#table-{kind}-dialog") + if kind == "create": + dialog.locator('input[name="table"]').fill("unsaved_table") + else: + dialog.locator(".table-alter-add-column").click() + # Real browser confirms, including WebKit, should appear once and stay usable. + confirmations = [] + + def reject(prompt): + confirmations.append(prompt.message) + prompt.dismiss() + + page.on("dialog", reject) + with page.expect_event("dialog"): + page.keyboard.press("Escape") + expect(dialog).to_be_visible() + assert len(confirmations) == 1 + page.remove_listener("dialog", reject) + page.on("dialog", lambda prompt: prompt.accept()) + page.keyboard.press("Escape") + expect(dialog).not_to_be_visible() + expect(menu.locator("summary")).to_be_focused()