From c410ed95554839f617f178470f38d571c58f5813 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:23 -0700 Subject: [PATCH] Refactor navigation search to use the shared modal, refs #2790 --- datasette/static/navigation-search.js | 91 +++------------------------ tests/test_playwright.py | 45 +++++++++++++ 2 files changed, 53 insertions(+), 83 deletions(-) diff --git a/datasette/static/navigation-search.js b/datasette/static/navigation-search.js index ec2d23d8..02136466 100644 --- a/datasette/static/navigation-search.js +++ b/datasette/static/navigation-search.js @@ -15,8 +15,6 @@ class NavigationSearch extends HTMLElement { this.matches = []; this.renderedMatches = []; this.debounceTimer = null; - this.restoreFocusTarget = null; - this.shouldRestoreFocus = true; this.render(); this.setupEventListeners(); @@ -29,38 +27,10 @@ class NavigationSearch extends HTMLElement { display: contents; } - dialog { - border: none; - border-radius: var(--modal-border-radius, 0.75rem); - padding: 0; + dialog.datasette-modal { max-width: 90vw; width: 600px; max-height: 80vh; - 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: slideIn var(--modal-animation-duration, 0.2s) ease-out; - } - - 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: fadeIn var(--modal-animation-duration, 0.2s) ease-out; - } - - @keyframes slideIn { - from { - opacity: 0; - transform: translateY(-20px) scale(0.95); - } - to { - opacity: 1; - transform: translateY(0) scale(1); - } - } - - @keyframes fadeIn { - from { opacity: 0; } - to { opacity: 1; } } .search-container { @@ -255,7 +225,7 @@ class NavigationSearch extends HTMLElement { /* Mobile optimizations */ @media (max-width: 640px) { - dialog { + dialog.datasette-modal { width: 95vw; max-height: 85vh; border-radius: 0.5rem; @@ -280,7 +250,7 @@ class NavigationSearch extends HTMLElement { } - +
@@ -309,7 +279,7 @@ class NavigationSearch extends HTMLElement { Esc Close
- + `; } @@ -355,8 +325,6 @@ class NavigationSearch extends HTMLElement { } else if (e.key === "Enter") { e.preventDefault(); this.selectCurrentItem(); - } else if (e.key === "Escape") { - this.closeMenu(); } }); @@ -380,18 +348,6 @@ class NavigationSearch extends HTMLElement { } }); - // Close on backdrop click - dialog.addEventListener("click", (e) => { - if (e.target === dialog) { - this.closeMenu(); - } - }); - - dialog.addEventListener("cancel", (e) => { - e.preventDefault(); - this.closeMenu(); - }); - dialog.addEventListener("close", () => { this.onMenuClosed(); }); @@ -432,19 +388,6 @@ class NavigationSearch extends HTMLElement { } } - focusRestoreTarget(trigger) { - if (trigger && typeof trigger.focus === "function") { - return trigger; - } - if ( - document.activeElement && - typeof document.activeElement.focus === "function" - ) { - return document.activeElement; - } - return null; - } - setNavigationTriggersExpanded(expanded) { if (typeof document.querySelectorAll !== "function") { return; @@ -854,17 +797,13 @@ class NavigationSearch extends HTMLElement { } openMenu(trigger) { - const dialog = this.shadowRoot.querySelector("dialog"); const input = this.shadowRoot.querySelector(".search-input"); - this.restoreFocusTarget = this.focusRestoreTarget(trigger); - this.shouldRestoreFocus = true; - if (!dialog.open) { - dialog.showModal(); - } + this.shadowRoot + .querySelector("datasette-modal") + .show({ trigger, initialFocus: input }); this.setNavigationTriggersExpanded(true); input.value = ""; - input.focus(); // Reset state, then populate the default jump list. this.matches = []; @@ -874,13 +813,7 @@ class NavigationSearch extends HTMLElement { } closeMenu(options = {}) { - const dialog = this.shadowRoot.querySelector("dialog"); - this.shouldRestoreFocus = options.restoreFocus !== false; - if (dialog.open) { - dialog.close(); - } else { - this.onMenuClosed(); - } + this.shadowRoot.querySelector("datasette-modal").close(options); } onMenuClosed() { @@ -889,14 +822,6 @@ class NavigationSearch extends HTMLElement { this.removeElementAttribute(input, "aria-activedescendant"); this.setNavigationTriggersExpanded(false); this.setStatus(""); - if ( - this.shouldRestoreFocus && - this.restoreFocusTarget && - typeof this.restoreFocusTarget.focus === "function" - ) { - this.restoreFocusTarget.focus(); - } - this.restoreFocusTarget = null; } escapeHtml(text) { diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 969f6edf..32364b0f 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1762,6 +1762,51 @@ def test_modal_lifecycle(page, datasette_server, shadow): expect(page.locator("#after-save")).to_be_focused() +@pytest.mark.playwright +@pytest.mark.parametrize("name", ["jump"]) +def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name): + from playwright.sync_api import expect + + page_errors = [] + page.on("pageerror", lambda error: page_errors.append(str(error))) + if name == "mobile": + page.set_viewport_size({"width": 390, "height": 844}) + page.emulate_media(reduced_motion="reduce") + page.goto(datasette_server + "data/projects") + if name == "jump": + trigger = page.locator("details.nav-menu summary") + trigger.click() + page.locator("[data-navigation-search-open]").click() + dialog = page.locator("navigation-search dialog") + elif name == "columns": + # Open through its public API with a real, focused page control. + trigger = page.locator("details.actions-menu-links summary") + trigger.focus() + page.evaluate( + "document.querySelector('column-chooser').open({columns: ['id', 'title'], selected: ['id']})" + ) + dialog = page.locator("column-chooser dialog") + elif name == "type": + trigger = page.locator("details.actions-menu-links summary") + trigger.focus() + page.evaluate( + "openSetColumnTypeDialog(document.querySelector('th[data-column=title]'))" + ) + dialog = page.locator("#set-column-type-dialog") + else: + trigger = page.locator(".column-actions-mobile") + trigger.click() + dialog = page.locator("#mobile-column-actions-dialog") + expect(dialog).to_be_visible() + expect(dialog).to_have_css("border-radius", "8px" if name == "mobile" else "12px") + expect(dialog).to_have_css("animation-name", "none") + assert dialog.evaluate("node => node.parentElement.localName") == "datasette-modal" + page.keyboard.press("Escape") + expect(dialog).not_to_be_visible() + expect(trigger).to_be_focused() + assert page_errors == [] + + @pytest.mark.playwright def test_modal_disconnect_cleans_up_pending_escape(page, datasette_server): from playwright.sync_api import expect