Refactor the alter table dialog to use the shared modal, refs #2790

This commit is contained in:
Simon Willison 2026-09-17 13:28:25 -07:00
commit 814165c8b1
3 changed files with 30 additions and 159 deletions

View file

@ -2542,46 +2542,8 @@ select.table-create-input {
}
dialog.table-alter-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-alter-dialog[open] {
display: flex;
flex-direction: column;
}
dialog.table-alter-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-alter-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-alter-dialog .modal-title {
@ -2589,9 +2551,6 @@ dialog.table-alter-dialog::backdrop {
align-items: center;
min-width: 0;
max-width: 100%;
font-size: 1rem;
font-weight: 600;
color: var(--ink);
}
.table-alter-form {
@ -2949,72 +2908,29 @@ select.table-alter-input {
outline-offset: 1px;
}
.table-alter-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-alter-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-alter-dialog .btn-ghost {
background: transparent;
color: var(--muted);
border: 1px solid var(--rule);
}
.table-alter-dialog .btn-ghost:hover {
background: var(--rule);
color: var(--ink);
}
.table-alter-dialog .btn-danger {
.table-alter-dialog .modal-btn-danger {
background: #b91c1c;
color: #fff;
margin-right: auto;
}
.table-alter-dialog .btn-danger:hover {
.table-alter-dialog .modal-btn-danger:hover {
background: #991b1b;
}
.table-alter-dialog .btn-danger:disabled,
.table-alter-dialog .btn-danger:disabled:hover {
.table-alter-dialog .modal-btn-danger:disabled,
.table-alter-dialog .modal-btn-danger:disabled:hover {
background: #d98c8c;
color: #fff;
}
.table-alter-dialog .btn-primary {
background: var(--accent);
color: #fff;
}
.table-alter-dialog .btn-primary:hover {
background: #1949b8;
}
.table-alter-dialog .btn-primary:disabled,
.table-alter-dialog .btn-primary:disabled:hover {
.table-alter-dialog .modal-btn-primary:disabled,
.table-alter-dialog .modal-btn-primary:disabled:hover {
background: #a0aec0;
color: #fff;
}
.table-alter-dialog .btn:disabled,
.table-alter-dialog .modal-btn:disabled,
.table-alter-add-column:disabled,
.table-alter-icon-button:disabled {
opacity: 0.55;

View file

@ -2656,6 +2656,7 @@ function showTableAlterDialogError(state, message) {
function setTableAlterDialogSaving(state, isSaving) {
state.isSaving = isSaving;
state.modal.busy = isSaving;
state.cancelButton.disabled = isSaving;
state.addColumnButton.disabled = isSaving;
state.backButton.disabled = isSaving;
@ -3791,8 +3792,7 @@ async function applyTableAlterChanges(state, result) {
result.columnTypeAssignments || [],
tableUrl,
);
state.shouldRestoreFocus = false;
state.dialog.close();
state.modal.close({ restoreFocus: false });
if (tableAlterResultRenamesTable(result) && tableUrl) {
window.location.href = tableUrl;
} else {
@ -3853,8 +3853,7 @@ async function dropTableFromAlterDialog(state) {
if (!response.ok || (responseData && responseData.ok === false)) {
throw rowMutationRequestError(response, responseData);
}
state.shouldRestoreFocus = false;
state.dialog.close();
state.modal.close({ restoreFocus: false });
window.location.href = tableAlterDatabaseUrl() || "/";
} catch (error) {
setTableAlterDialogSaving(state, false);
@ -3890,27 +3889,6 @@ function confirmDiscardTableAlterChanges(state) {
return window.confirm("Discard table changes?");
}
function closeTableAlterDialogIfConfirmed(state) {
if (!state || state.isSaving) {
return false;
}
if (!confirmDiscardTableAlterChanges(state)) {
return false;
}
state.shouldRestoreFocus = true;
state.dialog.close();
return true;
}
function closeTableAlterDialog(state) {
if (!state || state.isSaving) {
return false;
}
state.shouldRestoreFocus = true;
state.dialog.close();
return true;
}
function ensureTableAlterDialog(manager) {
if (tableAlterDialogState) {
return tableAlterDialogState;
@ -3919,7 +3897,8 @@ function ensureTableAlterDialog(manager) {
return null;
}
var dialog = document.createElement("dialog");
var modal = DatasetteModal.create();
var dialog = modal.dialog;
dialog.id = TABLE_ALTER_DIALOG_ID;
dialog.className = "table-alter-dialog";
dialog.setAttribute("aria-labelledby", "table-alter-title");
@ -3950,16 +3929,17 @@ function ensureTableAlterDialog(manager) {
</div>
<div class="table-alter-review" hidden></div>
<div class="modal-footer">
<button type="button" class="btn btn-danger table-alter-drop" hidden>Drop table</button>
<button type="button" class="btn btn-ghost table-alter-back" hidden>Back</button>
<button type="button" class="btn btn-ghost table-alter-cancel">Cancel</button>
<button type="submit" class="btn btn-primary table-alter-save">Review changes</button>
<button type="button" class="modal-btn modal-btn-danger table-alter-drop" hidden>Drop table</button>
<button type="button" class="modal-btn modal-btn-ghost table-alter-back" hidden>Back</button>
<button type="button" class="modal-btn modal-btn-ghost table-alter-cancel">Cancel</button>
<button type="submit" class="modal-btn modal-btn-primary table-alter-save">Review changes</button>
</div>
</form>
`;
document.body.appendChild(dialog);
document.body.appendChild(modal);
tableAlterDialogState = {
modal: modal,
dialog: dialog,
form: dialog.querySelector(".table-alter-form"),
title: dialog.querySelector(".modal-title"),
@ -3974,8 +3954,6 @@ function ensureTableAlterDialog(manager) {
dropButton: dialog.querySelector(".table-alter-drop"),
cancelButton: dialog.querySelector(".table-alter-cancel"),
saveButton: dialog.querySelector(".table-alter-save"),
currentButton: null,
shouldRestoreFocus: true,
isSaving: false,
initialSignature: "",
originalTableName: "",
@ -4017,7 +3995,7 @@ function ensureTableAlterDialog(manager) {
});
tableAlterDialogState.cancelButton.addEventListener("click", function () {
closeTableAlterDialog(tableAlterDialogState);
modal.requestClose("cancel");
});
tableAlterDialogState.dropButton.addEventListener("click", function () {
@ -4038,36 +4016,17 @@ function ensureTableAlterDialog(manager) {
}
});
dialog.addEventListener("click", function (ev) {
if (ev.target === dialog) {
closeTableAlterDialogIfConfirmed(tableAlterDialogState);
}
});
dialog.addEventListener("keydown", function (ev) {
if (ev.key !== "Escape") {
return;
}
ev.preventDefault();
closeTableAlterDialogIfConfirmed(tableAlterDialogState);
});
dialog.addEventListener("cancel", function (ev) {
ev.preventDefault();
closeTableAlterDialogIfConfirmed(tableAlterDialogState);
});
modal.beforeClose = function (reason) {
return (
reason === "cancel" ||
confirmDiscardTableAlterChanges(tableAlterDialogState)
);
};
dialog.addEventListener("close", function () {
var state = tableAlterDialogState;
clearTableAlterDialogError(state);
setTableAlterDialogSaving(state, false);
if (
state.shouldRestoreFocus &&
state.currentButton &&
document.contains(state.currentButton)
) {
state.currentButton.focus();
}
});
return tableAlterDialogState;
@ -4088,8 +4047,7 @@ function openTableAlterDialog(button, manager) {
menu.open = false;
}
state.manager = manager;
state.currentButton = button;
state.shouldRestoreFocus = true;
state.title.textContent = "Alter table " + data.tableName;
clearTableAlterDialogError(state);
resetTableAlterDialog(state, data);
@ -4099,9 +4057,7 @@ function openTableAlterDialog(button, manager) {
tableAlterForeignKeyTargetsUrl(),
{ filterByType: false },
);
if (!state.dialog.open) {
state.dialog.showModal();
}
state.modal.show({ trigger: button });
var firstName = state.columnList.querySelector(".table-alter-column-name");
if (firstName) {
firstName.focus();

View file

@ -1033,15 +1033,14 @@ def test_alter_table_cancel_skips_discard_prompt(page, datasette_server):
dialog.locator(".table-alter-add-column").click()
dialog.locator(".table-alter-column-name").last.fill("escape_me")
page.keyboard.press("Escape")
page.wait_for_function("window.__discardConfirmMessages.length === 1")
assert page.evaluate("() => window.__discardConfirmMessages") == [
"Discard table changes?"
]
assert dialog.evaluate("node => node.open") is True
page.evaluate("() => window.__discardConfirmMessages = []")
dialog.evaluate(
"""node => node.dispatchEvent(new MouseEvent("click", {bubbles: true}))"""
)
page.mouse.click(2, 2)
assert page.evaluate("() => window.__discardConfirmMessages") == [
"Discard table changes?"
]
@ -1835,7 +1834,7 @@ def test_modal_disconnect_cleans_up_pending_escape(page, datasette_server):
@pytest.mark.playwright
@pytest.mark.parametrize("kind", ["create"])
@pytest.mark.parametrize("kind", ["create", "alter"])
def test_schema_modal_escape_confirmation_and_focus(page, datasette_server, kind):
from playwright.sync_api import expect