diff --git a/datasette/static/app.css b/datasette/static/app.css index 4a4b0d5c..3b0546e3 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -63,7 +63,7 @@ em { } /* end reset */ -/* Modal CSS variables (shared by web components via Shadow DOM) */ +/* Shared modal CSS variables */ :root { --modal-backdrop-bg: rgba(0, 0, 0, 0.5); --modal-backdrop-blur: blur(4px); diff --git a/datasette/static/modal.css b/datasette/static/modal.css index 8590adae..aeec561f 100644 --- a/datasette/static/modal.css +++ b/datasette/static/modal.css @@ -1,4 +1,4 @@ -/* Shared by light-DOM dialogs and dialogs inside existing shadow roots. */ +/* Shared modal styles. */ datasette-modal { display: contents; } diff --git a/datasette/static/modal.js b/datasette/static/modal.js index 36e7a7b4..b252af21 100644 --- a/datasette/static/modal.js +++ b/datasette/static/modal.js @@ -1,8 +1,5 @@ -// Shared modal shell. Content stays in the caller's DOM, including plugin -// controls and their form/ARIA relationships. The native dialog owns modality. +// Shared lifecycle for native modal dialogs. (() => { - const stylesheet = document.currentScript.dataset.stylesheet; - class DatasetteModal extends HTMLElement { constructor() { super(); @@ -39,18 +36,6 @@ const dialog = this.dialog; if (!dialog) return; dialog.classList.add("datasette-modal"); - // The same CSS is used in the document and in existing web components. - const root = this.getRootNode(); - if ( - root instanceof ShadowRoot && - !root.querySelector("link[data-datasette-modal]") - ) { - const link = document.createElement("link"); - link.rel = "stylesheet"; - link.href = stylesheet; - link.dataset.datasetteModal = ""; - root.prepend(link); - } this._listeners?.abort(); this._listeners = new AbortController(); const options = { signal: this._listeners.signal }; @@ -86,11 +71,7 @@ (event) => { if (event.key !== "Escape" || event.defaultPrevented) return; // A nested native dialog or plugin picker gets first refusal. - if ( - event.composedPath().find((node) => node.localName === "dialog") !== - dialog - ) - return; + if (event.target.closest("dialog") !== dialog) return; event.preventDefault(); if (this.busy || this._escapeCleanup || this._escapeTimer !== null) return; @@ -158,10 +139,7 @@ const dialog = this.dialog; if (!dialog.open) { this._clearPendingClose(); - let active = this.ownerDocument.activeElement; - while (active?.shadowRoot?.activeElement) - active = active.shadowRoot.activeElement; - this._trigger = trigger || active; + this._trigger = trigger || this.ownerDocument.activeElement; this._restoreFocus = true; dialog.showModal(); } diff --git a/datasette/templates/base.html b/datasette/templates/base.html index 43911ee3..b11d14f5 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -9,7 +9,7 @@ {% endfor %} - + {% for url in extra_js_urls %} diff --git a/docs/contributing.rst b/docs/contributing.rst index 35d6443c..57643f64 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -139,11 +139,11 @@ Modal dialogs Core dialogs use the same ```` component available to plugins. See :ref:`javascript_plugins_modals` for examples, lifecycle methods, dismissal guards and shared styles. -The implementation lives in ``datasette/static/modal.js`` and ``datasette/static/modal.css``. The wrapper keeps each native ```` and its content in the caller's DOM tree, preserving form associations, accessible labels and plugin controls. Components such as ```` use the same wrapper and stylesheet inside their shadow roots. +The implementation lives in ``datasette/static/modal.js`` and ``datasette/static/modal.css``. Dialogs are part of the main document, including those in ```` and ````. Scope component-specific styles in ``app.css`` to the component or dialog. Keep focus restoration, backdrop hit testing, busy-state dismissal guards and the Safari Escape/confirmation workaround in the shared component. Each consumer owns its content, submission logic, discard-confirmation policy and cleanup. In particular, preserve the intentional differences between Cancel and Escape in the editing dialogs. -Add lifecycle coverage to ``tests/test_playwright.py`` when changing the shared component. Exercise both light DOM and shadow roots, focus restoration, busy state, nested controls consuming Escape, backdrop clicks and disconnect cleanup. Run these checks in Chromium, Firefox and WebKit; keyboard changes should include real confirmation prompts in WebKit. +Add lifecycle coverage to ``tests/test_playwright.py`` when changing the shared component. Exercise focus restoration, busy state, nested controls consuming Escape, backdrop clicks and disconnect cleanup. Run these checks in Chromium, Firefox and WebKit; keyboard changes should include real confirmation prompts in WebKit. .. _contributing_using_fixtures: diff --git a/docs/javascript_plugins.rst b/docs/javascript_plugins.rst index 00c64714..7a21b5a9 100644 --- a/docs/javascript_plugins.rst +++ b/docs/javascript_plugins.rst @@ -536,7 +536,7 @@ 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, including inside an open shadow root. ``initialFocus`` can be an element to focus or a function that focuses a custom control. Without it, the browser chooses initial focus. Calling ``show()`` while the dialog is already open preserves the original return-focus target. + 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()`` while the dialog is already open preserves the original return-focus target. ``modal.requestClose(reason = "cancel")`` 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. @@ -615,7 +615,7 @@ You can customize layout and sizing without adding extra classes. For example, t Long content should have a container with ``overflow: auto`` and ``min-height: 0`` so it can scroll while the header and footer remain visible. Keep these styles scoped to your dialog. -The dialog shell also uses the CSS custom properties ``--modal-border-radius``, ``--modal-shadow``, ``--modal-backdrop-bg``, ``--modal-backdrop-blur`` and ``--modal-animation-duration``. These work for dialogs in both the document and shadow roots. The shared animations respect the user's reduced-motion preference. +The dialog shell also uses the CSS custom properties ``--modal-border-radius``, ``--modal-shadow``, ``--modal-backdrop-bg``, ``--modal-backdrop-blur`` and ``--modal-animation-duration``. The shared animations respect the user's reduced-motion preference. .. _javascript_datasette_manager_selectors: diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 5404d94f..158b2cb7 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1739,20 +1739,16 @@ def test_count_all_error_retry(page, datasette_server): @pytest.mark.playwright -@pytest.mark.parametrize("shadow", [False, True]) -def test_modal_lifecycle(page, datasette_server, shadow): +def test_modal_lifecycle(page, datasette_server): from playwright.sync_api import expect page.goto(datasette_server) page.evaluate( - """shadow => { - const host = document.createElement('div'); - document.body.append(host); - const root = shadow ? host.attachShadow({mode: 'open'}) : host; + """() => { const trigger = document.createElement('button'); trigger.id = 'modal-trigger'; trigger.textContent = 'Open test modal'; - root.append(trigger); + document.body.append(trigger); window.testModal = DatasetteModal.create(); const dialog = testModal.dialog; dialog.id = 'test-modal'; @@ -1764,7 +1760,7 @@ def test_modal_lifecycle(page, datasette_server, shadow): `; // Padding is part of the dialog, never a backdrop dismissal. dialog.style.padding = '30px'; - root.append(testModal); + document.body.append(testModal); window.closeReasons = []; testModal.beforeClose = reason => { closeReasons.push(reason); @@ -1776,7 +1772,6 @@ def test_modal_lifecycle(page, datasette_server, shadow): }); dialog.querySelector('button').onclick = () => testModal.requestClose('cancel'); }""", - shadow, ) trigger = page.locator("#modal-trigger") trigger.click() @@ -1947,6 +1942,7 @@ def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name) 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" + assert dialog.evaluate("node => node.getRootNode() === document") page.keyboard.press("Escape") expect(dialog).not_to_be_visible() expect(trigger).to_be_focused()