From e78b8a2e6ac69310c06fdacc6ca0a6ab309ffe0b Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Tue, 1 Sep 2026 09:32:37 -0700 Subject: [PATCH 001/108] Run datasette serve startup and uvicorn on a single event loop (#2886) * Run datasette serve startup and uvicorn on a single event loop * Move the serve-subprocess test plumbing into a conftest fixture * Fix datasette-litestream URL and trim marker-task test comments * Explain why serve_with_plugins needs a subprocess and plugin files * Apply ruff 0.16 and black fixes * Tweaked some comments --- datasette/cli.py | 91 ++++++++++++++----------- pyproject.toml | 2 +- tests/conftest.py | 84 ++++++++++++++++++++++- tests/test_cli_serve_server.py | 117 +++++++++++++++++++++++++++++++++ 4 files changed, 253 insertions(+), 41 deletions(-) diff --git a/datasette/cli.py b/datasette/cli.py index 57db83b6..12024a14 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -663,16 +663,6 @@ def serve( # Private utility mechanism for writing unit tests return ds - # Run async soundness checks before startup hooks, since invoke_startup - # now populates internal tables which requires querying each database - run_sync(lambda: check_databases(ds)) - - # Run the "startup" plugin hooks - try: - run_sync(ds.invoke_startup) - except StartupError as e: - raise click.ClickException(e.args[0]) - if headers and not get: raise click.ClickException("--headers can only be used with --get") @@ -680,6 +670,14 @@ def serve( raise click.ClickException("--token can only be used with --get") if get: + # --get means we don't run Uvicorn at all + run_sync(lambda: check_databases(ds)) + + try: + run_sync(ds.invoke_startup) + except StartupError as e: + raise click.ClickException(e.args[0]) + client = TestClient(ds) request_headers = {} if token: @@ -704,34 +702,51 @@ def serve( sys.exit(exit_code) return - # Start the server - url = None - if root: - ds.root_enabled = True - url = "http://{}:{}{}?token={}".format( - host, port, ds.urls.path("-/auth-token"), ds._root_token - ) - click.echo(url) - if open_browser: - if url is None: - # Figure out most convenient URL - to table, database or homepage - path = run_sync(lambda: initial_path_for_datasette(ds)) - url = f"http://{host}:{port}{path}" - webbrowser.open(url) - uvicorn_kwargs = { - "host": host, - "port": port, - "log_level": "info", - "lifespan": "on", - "workers": 1, - } - if uds: - uvicorn_kwargs["uds"] = uds - if ssl_keyfile: - uvicorn_kwargs["ssl_keyfile"] = ssl_keyfile - if ssl_certfile: - uvicorn_kwargs["ssl_certfile"] = ssl_certfile - uvicorn.run(ds.app(), **uvicorn_kwargs) + # check_databases, invoke_startup() and the uvicorn server all run on a + # single event loop, so that anything a plugin's "startup" hook schedules + # on the loop (asyncio.create_task, Lock/Queue/Event objects, ...) is + # still alive when the server starts handling requests. + async def _serve_async(): + # Populate internal catalog tables before invoke_startup + await check_databases(ds) + + # Run the "startup" plugin hooks + try: + await ds.invoke_startup() + except StartupError as e: + raise click.ClickException(e.args[0]) + + # Start the server + url = None + if root: + ds.root_enabled = True + url = "http://{}:{}{}?token={}".format( + host, port, ds.urls.path("-/auth-token"), ds._root_token + ) + click.echo(url) + if open_browser: + if url is None: + # Figure out most convenient URL - to table, database or homepage + path = await initial_path_for_datasette(ds) + url = f"http://{host}:{port}{path}" + webbrowser.open(url) + uvicorn_kwargs = { + "host": host, + "port": port, + "log_level": "info", + "lifespan": "on", + "workers": 1, + } + if uds: + uvicorn_kwargs["uds"] = uds + if ssl_keyfile: + uvicorn_kwargs["ssl_keyfile"] = ssl_keyfile + if ssl_certfile: + uvicorn_kwargs["ssl_certfile"] = ssl_certfile + server = uvicorn.Server(uvicorn.Config(ds.app(), **uvicorn_kwargs)) + await server.serve() + + asyncio.run(_serve_async()) @cli.command() diff --git a/pyproject.toml b/pyproject.toml index cf5db905..e658955f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,7 @@ dependencies = [ "hupper>=1.9", "httpx>=0.20,<1.0", "pluggy>=1.0", - "uvicorn>=0.11", + "uvicorn>=0.29", "aiofiles>=0.4", "PyYAML>=5.3", "mergedeep>=1.1.1", diff --git a/tests/conftest.py b/tests/conftest.py index a2e6aba2..12dce417 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ import importlib.metadata import os import pathlib import re +import socket import subprocess import sys import tempfile @@ -32,17 +33,31 @@ UNDOCUMENTED_PERMISSIONS = { } -def wait_until_responds(url, timeout=5.0, client=httpx, **kwargs): +def wait_until_responds(url, timeout=5.0, client=httpx, process=None, **kwargs): start = time.time() while time.time() - start < timeout: + # If the server died there is no point waiting out the timeout - fail + # now, with its output, instead of after `timeout` seconds of silence + if process is not None and process.poll() is not None: + raise AssertionError( + "Server exited early with returncode {}\n{}".format( + process.returncode, process.stdout.read().decode("utf-8") + ) + ) try: client.get(url, **kwargs) return - except httpx.ConnectError: + except httpx.TransportError: time.sleep(0.1) raise AssertionError(f"Timed out waiting for {url} to respond") +def find_free_port(): + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + @pytest.fixture def bare_ds(): """ @@ -301,6 +316,71 @@ def ds_unix_domain_socket_server(tmp_path_factory): pass +@pytest.fixture +def serve_with_plugins(tmp_path): + """Factory fixture for starting ``datasette serve`` in a subprocess with + plugins written to a temporary ``--plugins-dir``. + + For tests that need the real serve path: event-loop wiring, exit codes, + signals. The usual in-process ``pm.register`` plugin pattern can't reach + a subprocess, so plugin source is written out as importable files instead. + + Unlike ``ds_localhost_http_server`` this is function-scoped and takes a + fresh port each time, because each test needs its own plugins. Call it as:: + + proc, port = serve_with_plugins({"my_plugin": PLUGIN_SOURCE}) + + ``plugins`` maps module name to Python source. Pass + ``wait_for_startup=False`` when the server is expected to fail during + startup rather than begin serving. Extra CLI arguments are passed through. + Every process started is terminated when the test ends. + """ + processes = [] + + def start(plugins, *extra_args, wait_for_startup=True): + plugins_dir = tmp_path / "plugins" + plugins_dir.mkdir(exist_ok=True) + for module_name, source in plugins.items(): + (plugins_dir / f"{module_name}.py").write_text(source, "utf-8") + port = find_free_port() + proc = subprocess.Popen( + [ + sys.executable, + "-m", + "datasette", + "--memory", + "--plugins-dir", + str(plugins_dir), + "-h", + "127.0.0.1", + "-p", + str(port), + *extra_args, + ], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + # Avoid FileNotFoundError: [Errno 2] No such file or directory: + cwd=tempfile.gettempdir(), + ) + processes.append(proc) + if wait_for_startup: + wait_until_responds( + f"http://127.0.0.1:{port}/-/versions.json", process=proc + ) + return proc, port + + yield start + + for proc in processes: + if proc.poll() is None: + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + + # Import fixtures from fixtures.py to make them available from .fixtures import ( # noqa: F401 TEMP_PLUGIN_SECRET_FILE, diff --git a/tests/test_cli_serve_server.py b/tests/test_cli_serve_server.py index b7604bb8..b76180fd 100644 --- a/tests/test_cli_serve_server.py +++ b/tests/test_cli_serve_server.py @@ -1,4 +1,5 @@ import socket +import time import httpx import pytest @@ -28,3 +29,119 @@ def test_serve_unix_domain_socket(ds_unix_domain_socket_server): "path": "/_memory", "tables": [], }.items() <= response.json().items() + + +# Shaped after datasette-litestream's startup hook, which schedules a +# background task with asyncio.get_running_loop().create_task(...): +# https://github.com/datasette/datasette-litestream +MARKER_TASK_PLUGIN = """ +import asyncio +from datasette import hookimpl +from datasette.utils.asgi import Response + + +@hookimpl +def startup(datasette): + datasette._startup_calls = getattr(datasette, "_startup_calls", 0) + 1 + + async def _mark(): + # Must await before setting the flag: a task with no internal + # await point could finish on the throwaway loop before it + # closed, masking the regression this test guards against. + await asyncio.sleep(0.2) + datasette._marker_task_ran = True + + asyncio.get_running_loop().create_task(_mark()) + + +@hookimpl +def register_routes(): + async def marker_status(datasette): + return Response.json( + { + "marker_task_ran": getattr(datasette, "_marker_task_ran", False), + "startup_calls": getattr(datasette, "_startup_calls", 0), + } + ) + + return [(r"^/-/marker-task-ran$", marker_status)] +""" + + +STARTUP_ERROR_PLUGIN = """ +from datasette import hookimpl +from datasette.utils import StartupError + + +@hookimpl +def startup(datasette): + raise StartupError("boom from plugin") +""" + + +@pytest.mark.serial +def test_startup_hook_background_task_runs_on_serving_loop(serve_with_plugins): + """ + Litestream-shaped regression test: a startup hook that does + asyncio.get_running_loop().create_task(...) must have that task + actually execute before/while the server is handling requests. This + only holds if invoke_startup() and uvicorn.Server.serve() share one + event loop. This test fails against unmodified main, where + invoke_startup() runs on a throwaway loop that is closed before + uvicorn opens its own loop to serve. + """ + _, port = serve_with_plugins({"marker_task_plugin": MARKER_TASK_PLUGIN}) + # The fixture has already waited for the server to answer requests. The + # marker task deliberately awaits before setting its flag, so poll for a + # moment rather than assuming it landed before the first request arrived. + deadline = time.time() + 3.0 + payload = {} + while time.time() < deadline: + payload = httpx.get( + f"http://127.0.0.1:{port}/-/marker-task-ran", timeout=1.0 + ).json() + if payload["marker_task_ran"]: + break + time.sleep(0.05) + assert payload.get("marker_task_ran"), ( + "The startup hook's asyncio.create_task(...) never ran - " + "invoke_startup() and the server are not sharing an event loop" + ) + # Polling above means this test would also pass if the startup hook were + # re-run on the serving loop by the first-request fallback - which would + # hide exactly the bug being tested. invoke_startup() is idempotent today + # so that cannot happen; assert it explicitly so that if the idempotency + # guard is ever removed this test fails loudly instead of silently + # becoming a no-op. + assert payload["startup_calls"] == 1, ( + "startup hook ran {} times - the marker may have been set by a " + "re-run on the serving loop rather than by the original task".format( + payload["startup_calls"] + ) + ) + + +@pytest.mark.serial +def test_startup_error_fails_fast_before_port_binds(serve_with_plugins): + """ + A "startup" plugin hook that raises StartupError must fail fast: print + the message, exit non-zero, and never accept a connection on the port - + the failure must happen before uvicorn.Server binds the socket. + """ + proc, port = serve_with_plugins( + {"startup_error_plugin": STARTUP_ERROR_PLUGIN}, wait_for_startup=False + ) + stdout, _ = proc.communicate(timeout=15) + output = stdout.decode("utf-8") + assert proc.returncode not in (0, None), output + assert "boom from plugin" in output, output + + # Nothing is listening on the port now the process has exited. This + # confirms the socket was not left bound; on its own it cannot prove the + # failure preceded the bind, since a port nothing ever touched also + # refuses connections. + with ( + pytest.raises(OSError), + socket.create_connection(("127.0.0.1", port), timeout=0.2), + ): + pass From 3e018bb1b571cef87c67ae718a5472f23d6b3c6f Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Tue, 1 Sep 2026 09:39:25 -0700 Subject: [PATCH 002/108] Run startup via ASGI lifespan instead of waiting for the first request (#2887) * Run startup via ASGI lifespan instead of waiting for the first request * Ensure immutable table counts still precompute when startup ran first --- datasette/app.py | 47 ++++++-- datasette/cli.py | 7 +- datasette/utils/asgi.py | 42 +++++-- tests/test_lifespan.py | 259 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 337 insertions(+), 18 deletions(-) create mode 100644 tests/test_lifespan.py diff --git a/datasette/app.py b/datasette/app.py index c82ea075..42be7425 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -453,8 +453,10 @@ class Datasette: self.databases = collections.OrderedDict() self.actions = {} # .invoke_startup() will populate this self._column_types = {} # .invoke_startup() will populate this + self._setup_db_done = False try: self._refresh_schemas_lock = asyncio.Lock() + self._startup_lock = asyncio.Lock() except RuntimeError as rex: # Workaround for intermittent test failure, see: # https://github.com/simonw/datasette/issues/1802 @@ -462,6 +464,7 @@ class Datasette: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) self._refresh_schemas_lock = asyncio.Lock() + self._startup_lock = asyncio.Lock() else: raise self.crossdb = crossdb @@ -2803,24 +2806,52 @@ class Datasette: raise RowNotFound(db.name, table_name, pk_values) return ResolvedRow(db, table_name, sql, params, pks, pk_values, results.first()) + async def _startup_sequence(self): + """Idempotently run the full startup sequence: table counts for + immutable databases, then invoke_startup(). Safe to call more than + once and safe to call concurrently - callers block until whichever + call got there first has finished. + + This is the single entry point used by both AsgiLifespan (so + real deployments finish startup before accepting requests) and + AsgiRunOnFirstRequest (the fallback for hosts that never send + lifespan events, e.g. DatasetteClient's httpx.ASGITransport), and + `datasette serve` (cli.py) calls it too. The fast path below checks + both `_startup_invoked` and `_setup_db_done` - not just the former - + so that a bare `await ds.invoke_startup()` made by a caller ahead of + `_startup_sequence()` (which only sets `_startup_invoked`) can't + make this method skip the immutable-database table-count precompute. + """ + if self._startup_invoked and self._setup_db_done: + return + async with self._startup_lock: + if self._startup_invoked and self._setup_db_done: + return + if not self._setup_db_done: + # First time server starts up, calculate table counts for + # immutable databases + for database in self.databases.values(): + if not database.is_mutable: + await database.table_counts(limit=60 * 60 * 1000) + self._setup_db_done = True + await self.invoke_startup() + def app(self): """Returns an ASGI app function that serves the whole of Datasette""" routes = self._routes() - async def setup_db(): - # First time server starts up, calculate table counts for immutable databases - for database in self.databases.values(): - if not database.is_mutable: - await database.table_counts(limit=60 * 60 * 1000) - async def _close_on_shutdown(): self.close() asgi = CrossOriginProtectionMiddleware(DatasetteRouter(self, routes), self) if self.setting("trace_debug"): asgi = AsgiTracer(asgi) - asgi = AsgiLifespan(asgi, on_shutdown=[_close_on_shutdown]) - asgi = AsgiRunOnFirstRequest(asgi, on_startup=[setup_db, self.invoke_startup]) + asgi = AsgiLifespan( + asgi, + on_startup=[self._startup_sequence], + on_shutdown=[_close_on_shutdown], + ) + asgi = AsgiRunOnFirstRequest(asgi, on_startup=[self._startup_sequence]) for wrapper in pm.hook.asgi_wrapper(datasette=self): asgi = wrapper(asgi) return asgi diff --git a/datasette/cli.py b/datasette/cli.py index 12024a14..2694c1f6 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -710,9 +710,12 @@ def serve( # Populate internal catalog tables before invoke_startup await check_databases(ds) - # Run the "startup" plugin hooks + # Run the full startup sequence (immutable-database table-count + # precompute + the "startup" plugin hooks) via the same entry point + # AsgiLifespan/AsgiRunOnFirstRequest use, so it's not skipped when + # uvicorn's lifespan.startup fires moments later. try: - await ds.invoke_startup() + await ds._startup_sequence() except StartupError as e: raise click.ClickException(e.args[0]) diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 812194fd..2614ad02 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -1,3 +1,4 @@ +import asyncio import json import re from http.cookies import Morsel, SimpleCookie @@ -300,12 +301,24 @@ class AsgiLifespan: while True: message = await receive() if message["type"] == "lifespan.startup": - for fn in self.on_startup: - await fn() + try: + for fn in self.on_startup: + await fn() + except Exception as e: # noqa: BLE001 + await send( + {"type": "lifespan.startup.failed", "message": str(e)} + ) + return await send({"type": "lifespan.startup.complete"}) elif message["type"] == "lifespan.shutdown": - for fn in self.on_shutdown: - await fn() + try: + for fn in self.on_shutdown: + await fn() + except Exception as e: # noqa: BLE001 + await send( + {"type": "lifespan.shutdown.failed", "message": str(e)} + ) + return await send({"type": "lifespan.shutdown.complete"}) return else: @@ -624,10 +637,23 @@ class AsgiRunOnFirstRequest: self.asgi = asgi self.on_startup = on_startup self._started = False + # Guards against concurrent early requests interleaving with startup: + # without this, several requests could all observe `_started is + # False` and proceed before any of them finish running the hooks. + self._lock = asyncio.Lock() async def __call__(self, scope, receive, send): - if not self._started: - self._started = True - for hook in self.on_startup: - await hook() + # Leave "lifespan" scope events alone - this shim only exists as a + # fallback for hosts that never send them. It wraps AsgiLifespan, so + # if it ran on_startup here too, a startup exception would escape + # before AsgiLifespan's own try/except got a chance to turn it into + # a lifespan.startup.failed message. + if scope["type"] != "lifespan" and not self._started: + async with self._lock: + # Re-check: another request may have finished startup while + # we were waiting for the lock. + if not self._started: + for hook in self.on_startup: + await hook() + self._started = True return await self.asgi(scope, receive, send) diff --git a/tests/test_lifespan.py b/tests/test_lifespan.py new file mode 100644 index 00000000..3655285e --- /dev/null +++ b/tests/test_lifespan.py @@ -0,0 +1,259 @@ +""" +Tests for wiring Datasette startup (setup_db table counts + invoke_startup) +into the ASGI lifespan protocol. + +These exercise Datasette._startup_sequence() via three different callers: +- AsgiLifespan, by hand-driving lifespan.startup messages (no HTTP request) +- AsgiRunOnFirstRequest, the fallback for hosts that never send lifespan + events (this is what DatasetteClient / plain httpx.ASGITransport uses) +- Both at once, to prove startup hooks run at most once +""" + +import asyncio +import contextlib +import sqlite3 + +import httpx +import pytest + +from datasette import hookimpl +from datasette.app import Datasette +from datasette.database import Database +from datasette.plugins import pm + + +async def _drive_lifespan_startup(app): + """Send a single lifespan.startup message into app's ASGI lifespan loop + and return the list of messages sent back - without ever sending + lifespan.shutdown. Mirrors what a real server does: after startup + completes it parks waiting for the next event. We cancel that wait + once we've observed the startup response, rather than closing the + Datasette instance down with a shutdown message. + """ + messages_sent = [] + startup_responded = asyncio.Event() + delivered = False + + async def receive(): + nonlocal delivered + if not delivered: + delivered = True + return {"type": "lifespan.startup"} + # No further messages: block until the task is cancelled below, + # same as a real server parked waiting for lifespan.shutdown. + await asyncio.Event().wait() + + async def send(message): + messages_sent.append(message) + startup_responded.set() + + task = asyncio.create_task(app({"type": "lifespan"}, receive, send)) + try: + await asyncio.wait_for(startup_responded.wait(), timeout=5) + finally: + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + return messages_sent + + +@pytest.mark.asyncio +async def test_lifespan_startup_runs_before_any_request(): + ds = Datasette(memory=True) + assert ds._startup_invoked is False + app = ds.app() + + messages = await _drive_lifespan_startup(app) + + assert {"type": "lifespan.startup.complete"} in messages + assert ds._startup_invoked is True + # Internal catalog tables should be populated too, entirely without an + # HTTP request having been made. + internal_db = ds.get_internal_database() + databases = await internal_db.execute("select * from catalog_databases") + assert len(databases.rows) >= 1 + + +@pytest.mark.asyncio +async def test_lifespan_startup_failure_reports_lifespan_startup_failed(): + class RaisingStartupPlugin: + __name__ = "RaisingStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + raise RuntimeError("boom from startup hook") + + return inner + + ds = Datasette(memory=True) + pm.register(RaisingStartupPlugin(), name="raising_startup_plugin") + try: + app = ds.app() + messages = await _drive_lifespan_startup(app) + finally: + pm.unregister(name="raising_startup_plugin") + + assert messages == [ + {"type": "lifespan.startup.failed", "message": "boom from startup hook"} + ] + # The exception happened before invoke_startup() got to the end of its + # body, so startup is not considered to have completed. + assert ds._startup_invoked is False + + +@pytest.mark.asyncio +async def test_startup_runs_exactly_once_across_lifespan_and_first_request(): + call_count = {"n": 0} + + class CountingStartupPlugin: + __name__ = "CountingStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + call_count["n"] += 1 + + return inner + + ds = Datasette(memory=True) + pm.register(CountingStartupPlugin(), name="counting_startup_plugin") + try: + # Build the ASGI app once, the way a real deployment does - and + # reuse the SAME app instance for both the lifespan drive and the + # HTTP requests below, since a fresh ds.app() call would reset the + # AsgiRunOnFirstRequest fallback's state. + app = ds.app() + + messages = await _drive_lifespan_startup(app) + assert {"type": "lifespan.startup.complete"} in messages + assert call_count["n"] == 1 + + # A first HTTP request (as if the host never sent lifespan events, + # or lifespan already ran) should not run the hook again. + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + response1 = await client.get("/-/versions.json") + assert response1.status_code == 200 + # ... nor should a second, repeat request. + response2 = await client.get("/-/versions.json") + assert response2.status_code == 200 + finally: + pm.unregister(name="counting_startup_plugin") + + assert call_count["n"] == 1 + + +@pytest.mark.asyncio +async def test_no_lifespan_first_request_still_triggers_startup(): + # Pin today's behavior: a client that never drives ASGI lifespan events + # at all (like httpx.ASGITransport, which DatasetteClient uses) still + # gets startup armed by the AsgiRunOnFirstRequest fallback. + ds = Datasette(memory=True) + assert ds._startup_invoked is False + app = ds.app() + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + response = await client.get("/-/versions.json") + assert response.status_code == 200 + + assert ds._startup_invoked is True + internal_db = ds.get_internal_database() + databases = await internal_db.execute("select * from catalog_databases") + assert len(databases.rows) >= 1 + + +@pytest.mark.asyncio +async def test_datasette_client_first_request_triggers_startup(): + # Same as above, but through the real DatasetteClient (ds.client) that + # plugins and tests actually use, to confirm nothing regressed there. + ds = Datasette(memory=True) + assert ds._startup_invoked is False + response = await ds.client.get("/-/versions.json") + assert response.status_code == 200 + assert ds._startup_invoked is True + + +@pytest.mark.asyncio +async def test_concurrent_first_requests_all_wait_for_slow_startup(): + call_count = {"n": 0} + + class SlowStartupPlugin: + __name__ = "SlowStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + call_count["n"] += 1 + await asyncio.sleep(0.2) + + return inner + + ds = Datasette(memory=True) + pm.register(SlowStartupPlugin(), name="slow_startup_plugin") + try: + app = ds.app() + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + responses = await asyncio.gather( + *[client.get("/-/versions.json") for _ in range(10)] + ) + finally: + pm.unregister(name="slow_startup_plugin") + + # Every one of the 10 simultaneous first requests must have blocked + # until startup actually finished, not raced ahead of it. + assert all(response.status_code == 200 for response in responses) + assert call_count["n"] == 1 + assert ds._startup_invoked is True + + +@pytest.mark.asyncio +async def test_setup_db_still_runs_when_invoke_startup_ran_first(tmp_path, monkeypatch): + # Regression test: `datasette serve` (cli.py _serve_async) calls + # ds.invoke_startup() directly, before uvicorn ever sends a + # lifespan.startup event that drives _startup_sequence(). If + # _startup_sequence()'s fast path only checked `_startup_invoked`, it + # would see startup already done and skip the immutable-database + # table-count precompute (setup_db) entirely - a silent regression + # versus main, where AsgiRunOnFirstRequest ran setup_db unconditionally + # on request #1. + db_path = tmp_path / "immutable.db" + conn = sqlite3.connect(str(db_path)) + conn.execute("create table t (id integer primary key)") + conn.commit() + conn.close() + + ds = Datasette([], immutables=[str(db_path)]) + + call_count = {"n": 0} + original_table_counts = Database.table_counts + + async def counting_table_counts(self, *args, **kwargs): + call_count["n"] += 1 + return await original_table_counts(self, *args, **kwargs) + + monkeypatch.setattr(Database, "table_counts", counting_table_counts) + + # Simulate the CLI path: invoke_startup() runs directly and completes + # BEFORE _startup_sequence() ever gets a chance to run setup_db. + await ds.invoke_startup() + assert ds._startup_invoked is True + assert call_count["n"] == 0 + + # The lifespan/first-request path (or the CLI itself, per the fix) + # calling the shared entry point afterwards must still precompute + # table counts for immutable databases. + await ds._startup_sequence() + assert call_count["n"] == 1 + assert ds._setup_db_done is True + + # Idempotency: a second call must not recompute. + await ds._startup_sequence() + assert call_count["n"] == 1 From bdc973174096cae350ddaa733a10ed8b3ffd970b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 1 Sep 2026 13:37:15 -0700 Subject: [PATCH 003/108] check-latest: true, add 3.15 to test matrix, to test RCs (#2895) See https://simonwillison.net/2026/Sep/1/python-315-rc-2/ --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 751eedfd..2a8c0ae4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,16 +11,17 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14", "3.15"] steps: - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} allow-prereleases: true cache: pip cache-dependency-path: pyproject.toml + check-latest: true - name: Build extension for --load-extension test run: |- (cd tests && gcc ext.c -fPIC -shared -o ext.so) From 7403ae68bb0e1c39f2ff1927953d2775b932b9d3 Mon Sep 17 00:00:00 2001 From: Zain Dana Harper <17142659+HarperZ9@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:22:17 -0700 Subject: [PATCH 004/108] Give each non-blocking write a distinct task id, refs #2860, #2859 execute_write_fn(fn, block=False) is documented to return "a UUID representing the queued task". Two things stopped that being true. _send_to_write_thread() derived the id from uuid.uuid5(NAMESPACE_DNS, "datasette.io"), which is deterministic, so every non-blocking write in every database in every process returned 3f143baa-4e3d-5842-a36f-4fa2f683b72f. A constant cannot identify a particular task. Now uuid4(). Refs #2860. With num_sql_threads=0 there is no write thread, so execute_write_fn took the synchronous branch and `result` was the write function's return value, normally None. The block=False path then unpacked it unconditionally and raised TypeError: cannot unpack non-iterable NoneType object. The non-threaded branch now returns the same (task_id, reply_future) shape, with the future already resolved because the write has finished, so both modes share one code path. Refs #2859. test_execute_write_fn_block_false only asserted isinstance(task_id, uuid.UUID), which a constant satisfies. The new test is parametrized over threaded and non-threaded and asserts two calls return different ids, so either regression fails it. --- datasette/database.py | 11 ++++++++++- tests/test_internals_database.py | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/datasette/database.py b/datasette/database.py index e162d34e..90c4e429 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -354,6 +354,15 @@ class Database: result = fn(self._write_connection) else: result = fn(self._write_connection) + if not block: + # There is no write thread here, so the write has already + # finished. Hand back the same (task_id, reply_future) shape + # _send_to_write_thread() returns, with the future already + # resolved, so the block=False path below is identical in + # both modes. + reply_future = asyncio.get_running_loop().create_future() + reply_future.set_result(result) + result = (uuid.uuid4(), reply_future) else: result = await self._send_to_write_thread( fn, block=block, transaction=transaction @@ -425,7 +434,7 @@ class Database: ) self._write_thread.name = f"_execute_writes for database {self.name}" self._write_thread.start() - task_id = uuid.uuid5(uuid.NAMESPACE_DNS, "datasette.io") + task_id = uuid.uuid4() loop = asyncio.get_running_loop() reply_future = loop.create_future() self._write_queue.put( diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index b1093b1c..97513123 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -705,6 +705,33 @@ async def test_execute_write_fn_block_false(db): assert isinstance(task_id, uuid.UUID) +@pytest.mark.asyncio +@pytest.mark.parametrize("disable_threads", (False, True)) +async def test_execute_write_fn_block_false_returns_uuid(tmp_path, disable_threads): + # block=False is documented to return "a UUID representing the queued task". + # With num_sql_threads=0 there is no write thread, so the non-threaded branch + # has to satisfy the same contract as the threaded one. + settings = {"num_sql_threads": 0} if disable_threads else {} + ds = Datasette([], memory=True, settings=settings) + await ds.invoke_startup() + db = ds.add_memory_database("test_block_false") + await db.execute_write( + "create table if not exists t (id integer primary key, v text)" + ) + + def write_fn(conn): + conn.execute("insert into t (v) values ('a')") + # Returns None, like most write functions. + + task_id = await db.execute_write_fn(write_fn, block=False) + + assert isinstance(task_id, uuid.UUID) + # Distinct per call, so a caller can tell two queued tasks apart. + second = await db.execute_write_fn(write_fn, block=False) + assert isinstance(second, uuid.UUID) + assert second != task_id + + @pytest.mark.asyncio async def test_execute_write_fn_block_true(db): def write_fn(conn): From bdaa8cc76cc69b4016747cc04f0ec50b418fbb7b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:45:03 -0700 Subject: [PATCH 005/108] Disable extension loading once --load-extension extensions are loaded Refs GHSA-2mvv-ffvc-q5p6 Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/app.py | 29 ++++++++++++++++++------- tests/test_load_extensions.py | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 42be7425..b89ab30c 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1532,15 +1532,28 @@ class Datasette: conn.row_factory = sqlite3.Row conn.text_factory = lambda x: str(x, "utf-8", "replace") if self.sqlite_extensions and database != INTERNAL_DB_NAME: + # Extension loading is only enabled for as long as it takes to + # load the configured extensions. Leaving it enabled would let + # anyone who can execute SQL call load_extension() themselves. conn.enable_load_extension(True) - for extension in self.sqlite_extensions: - # "extension" is either a string path to the extension - # or a 2-item tuple that specifies which entrypoint to load. - if isinstance(extension, tuple): - path, entrypoint = extension - conn.execute("SELECT load_extension(?, ?)", [path, entrypoint]) - else: - conn.execute("SELECT load_extension(?)", [extension]) + try: + for extension in self.sqlite_extensions: + # "extension" is either a string path to the extension + # or a 2-item tuple that specifies which entrypoint to load. + if isinstance(extension, tuple): + path, entrypoint = extension + if sys.version_info >= (3, 12): + conn.load_extension(path, entrypoint=entrypoint) + else: + # Connection.load_extension() only gained the + # entrypoint argument in Python 3.12 + conn.execute( + "SELECT load_extension(?, ?)", [path, entrypoint] + ) + else: + conn.load_extension(extension) + finally: + conn.enable_load_extension(False) if self.setting("cache_size_kb"): conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}") # pylint: disable=no-member diff --git a/tests/test_load_extensions.py b/tests/test_load_extensions.py index 61cdb3e0..a7c2bc24 100644 --- a/tests/test_load_extensions.py +++ b/tests/test_load_extensions.py @@ -1,4 +1,5 @@ from pathlib import Path +from unittest import mock import pytest @@ -20,6 +21,29 @@ def has_compiled_ext(): return False +@pytest.mark.parametrize("load_fails", (False, True)) +def test_load_extension_is_disabled(load_fails): + ds = Datasette(sqlite_extensions=[COMPILED_EXTENSION_PATH]) + connection = mock.Mock() + if load_fails: + connection.load_extension.side_effect = RuntimeError + + if load_fails: + with pytest.raises(RuntimeError): + ds._prepare_connection(connection, "data") + else: + ds._prepare_connection(connection, "data") + + # Extensions are loaded using the Python API, never via SQL + assert connection.load_extension.mock_calls == [ + mock.call(COMPILED_EXTENSION_PATH), + ] + assert connection.enable_load_extension.mock_calls == [ + mock.call(True), + mock.call(False), + ] + + @pytest.mark.asyncio @pytest.mark.skipif(not has_compiled_ext(), reason="Requires compiled ext.c") async def test_load_extension_default_entrypoint(): @@ -64,3 +88,20 @@ async def test_load_extension_multiple_entrypoints(): response = await ds.client.get("/_memory/-/query.json?_shape=arrays&sql=select+c()") assert response.status_code == 200 assert response.json()["rows"][0][0] == "c" + + +@pytest.mark.asyncio +@pytest.mark.skipif(not has_compiled_ext(), reason="Requires compiled ext.c") +async def test_sql_cannot_load_additional_extension(): + ds = Datasette(sqlite_extensions=[COMPILED_EXTENSION_PATH]) + + response = await ds.client.get( + "/_memory/-/query.json", + params={ + "sql": "select load_extension(:path, :entrypoint)", + "path": COMPILED_EXTENSION_PATH, + "entrypoint": "sqlite3_ext_b_init", + }, + ) + assert response.status_code == 400 + assert response.json()["error"] == "not authorized" From c7944fc454c9c7014719cfd6dc3dbb76f4841a9b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:27:18 -0700 Subject: [PATCH 006/108] Skip deploy if environment variables are missing --- .github/workflows/deploy-latest.yml | 33 ++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deploy-latest.yml b/.github/workflows/deploy-latest.yml index 3fc83438..46f03b01 100644 --- a/.github/workflows/deploy-latest.yml +++ b/.github/workflows/deploy-latest.yml @@ -14,24 +14,46 @@ jobs: deploy: runs-on: ubuntu-latest steps: + - name: Check deployment prerequisites + id: deployment-prerequisites + env: + GCP_SA_KEY: ${{ secrets.GCP_SA_KEY }} + LATEST_DATASETTE_SECRET: ${{ secrets.LATEST_DATASETTE_SECRET }} + run: | + missing=() + for variable in GCP_SA_KEY LATEST_DATASETTE_SECRET; do + if [[ -z "${!variable:-}" ]]; then + missing+=("$variable") + fi + done + if (( ${#missing[@]} )); then + echo "::notice::Skipping deployment because required environment variables are missing: ${missing[*]}" + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi - name: Check out datasette + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} uses: actions/checkout@v7 - name: Set up Python + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} uses: actions/setup-python@v6 with: python-version: "3.13" cache: pip - name: Install Python dependencies + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | python -m pip install --upgrade pip python -m pip install . --group dev python -m pip install sphinx-to-sqlite==0.1a1 - name: Run tests - if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' && github.ref == 'refs/heads/main' }} run: | pytest -n auto -m "not serial" pytest -m "serial" - name: Build fixtures.db and other files needed to deploy the demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: |- python tests/fixtures.py \ fixtures.db \ @@ -40,13 +62,14 @@ jobs: plugins \ --extra-db-filename extra_database.db - name: Build docs.db - if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' && github.ref == 'refs/heads/main' }} run: |- cd docs DISABLE_SPHINX_INLINE_TABS=1 sphinx-build -b xml . _build sphinx-to-sqlite ../docs.db _build cd .. - name: Set up the alternate-route demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | echo ' from datasette import hookimpl @@ -58,6 +81,7 @@ jobs: ' > plugins/alternative_route.py cp fixtures.db fixtures2.db - name: And the counters writable stored query demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | cat > plugins/counters.py < Date: Thu, 3 Sep 2026 14:35:35 -0700 Subject: [PATCH 007/108] execute-write: Check view-table for every table in a CREATE VIEW Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/utils/sql_analysis.py | 62 +++++++++++++++++++++++++++++- tests/test_queries.py | 68 --------------------------------- 2 files changed, 60 insertions(+), 70 deletions(-) diff --git a/datasette/utils/sql_analysis.py b/datasette/utils/sql_analysis.py index 334545bd..22bb55c4 100644 --- a/datasette/utils/sql_analysis.py +++ b/datasette/utils/sql_analysis.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from typing import Literal +from datasette.utils import escape_sqlite from datasette.utils.sqlite import SQLiteTableType, sqlite3, sqlite_table_type SQLOperation = Literal[ @@ -208,7 +209,9 @@ def analyze_sql_tables( This function is synchronous and connection-based. It temporarily installs a SQLite authorizer, prepares ``EXPLAIN ``, and returns the operation - callbacks observed while SQLite compiles the statement. + callbacks observed while SQLite compiles the statement. ``CREATE VIEW`` is + additionally executed inside a rolled-back savepoint so its source-table reads + can be discovered by analyzing a query against the temporary view. """ operations: dict[OperationKey, set[str]] = {} @@ -532,7 +535,7 @@ def analyze_sql_tables( return None return table_kind_cache[(key.sqlite_schema, key.table)] - return SQLAnalysis( + analysis = SQLAnalysis( operations=tuple( Operation( operation=key.operation, @@ -549,3 +552,58 @@ def analyze_sql_tables( for key, columns in operations.items() ) ) + + # SQLite does not resolve the SELECT body of a view when preparing CREATE + # VIEW, so its authorizer does not report reads from the view's source + # tables. Temporarily create the view, analyze a query against it (which + # does resolve the body), then roll the schema change back. Database-level + # callers use an isolated writable connection for this analysis. + create_view_operations = tuple( + operation + for operation in analysis.operations + if operation.operation == "create" and operation.target_type == "view" + ) + if not create_view_operations: + return analysis + + savepoint = "datasette_analyze_create_view" + conn.execute(f"SAVEPOINT {savepoint}") + try: + conn.execute(sql, params if params is not None else {}) + dependency_reads = [] + for view_operation in create_view_operations: + if view_operation.sqlite_schema is None or view_operation.table is None: + raise sqlite3.OperationalError( + "Could not determine the created view name" + ) + quoted_schema = escape_sqlite(view_operation.sqlite_schema) + quoted_view = escape_sqlite(view_operation.table) + qualified_view = f"{quoted_schema}.{quoted_view}" + view_analysis = analyze_sql_tables( + conn, + f"SELECT * FROM {qualified_view}", + database_name=database_name, + schema_to_database=schema_to_database, + ) + dependency_reads.extend( + operation + for operation in view_analysis.operations + if operation.operation == "read" + and not ( + operation.sqlite_schema == view_operation.sqlite_schema + and operation.table == view_operation.table + ) + ) + finally: + conn.execute(f"ROLLBACK TO {savepoint}") + conn.execute(f"RELEASE {savepoint}") + + existing_operations = set(analysis.operations) + return SQLAnalysis( + operations=analysis.operations + + tuple( + operation + for operation in dependency_reads + if operation not in existing_operations + ) + ) diff --git a/tests/test_queries.py b/tests/test_queries.py index 15b7ad0f..ebe8b832 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -3248,74 +3248,6 @@ async def test_execute_write_create_table_uses_create_table_permission(): assert not await db.table_exists("should_not_exist") -@pytest.mark.asyncio -async def test_execute_write_create_view_uses_create_view_permission(): - ds = Datasette( - memory=True, - default_deny=True, - config={ - "permissions": { - "insert-row": {"id": "row-writer"}, - "update-row": {"id": "row-writer"}, - }, - "databases": { - "data": { - "permissions": { - "view-database": {"id": ["creator", "row-writer"]}, - "execute-write-sql": {"id": ["creator", "row-writer"]}, - "create-view": {"id": "creator"}, - } - } - }, - }, - ) - db = ds.add_memory_database("execute_write_create_view", name="data") - await db.execute_write("create table dogs (id integer primary key, name text)") - await ds.invoke_startup() - - analysis_response = await ds.client.get( - "/data/-/execute-write/analyze", - actor={"id": "creator"}, - params={"sql": "create view dog_names as select id, name from dogs"}, - ) - allowed_response = await ds.client.post( - "/data/-/execute-write", - actor={"id": "creator"}, - json={"sql": "create view dog_names as select id, name from dogs"}, - ) - row_permission_response = await ds.client.post( - "/data/-/execute-write", - actor={"id": "row-writer"}, - json={"sql": "create view should_not_exist as select id from dogs"}, - ) - - assert analysis_response.status_code == 200 - analysis_data = analysis_response.json() - assert analysis_data["ok"] is True - assert analysis_data["execute_disabled"] is False - assert analysis_data["analysis_rows"] == [ - { - "operation": "create", - "database": "data", - "table": "dog_names", - "required_permission": "create-view", - "source": None, - "allowed": True, - } - ] - - assert allowed_response.status_code == 200 - assert allowed_response.json()["ok"] is True - assert allowed_response.json()["message"] == "Query executed" - assert await db.view_exists("dog_names") - - assert row_permission_response.status_code == 403 - assert row_permission_response.json()["errors"] == [ - "Permission denied: need create-view on data" - ] - assert not await db.view_exists("should_not_exist") - - @pytest.mark.parametrize( ( "database_name", From c280c47424e87019376f534fbd349fd1a55d53a3 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:40 -0700 Subject: [PATCH 008/108] POST /db/-/create checks table-level insert/update/alter permissions Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/views/table_create_alter.py | 12 +-- tests/test_api_write.py | 116 ++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 5 deletions(-) diff --git a/datasette/views/table_create_alter.py b/datasette/views/table_create_alter.py index 56b28877..f8f8c31e 100644 --- a/datasette/views/table_create_alter.py +++ b/datasette/views/table_create_alter.py @@ -821,16 +821,18 @@ class TableCreateView(BaseView): ignore = create_request.ignore replace = create_request.replace + table_name = create_request.table + table_exists = await db.table_exists(table_name) + table_resource = TableResource(database=database_name, table=table_name) + # Replacing rows requires update-row permission if replace and not await self.ds.allowed( action="update-row", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error(["Permission denied: need update-row"], 403) - table_name = create_request.table - table_exists = await db.table_exists(table_name) columns = create_request.columns rows = create_request.rows_list @@ -838,7 +840,7 @@ class TableCreateView(BaseView): # Must have insert-row permission if not await self.ds.allowed( action="insert-row", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error(["Permission denied: need insert-row"], 403) @@ -857,7 +859,7 @@ class TableCreateView(BaseView): if create_request.alter: if not await self.ds.allowed( action="alter-table", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error( diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 11ef30de..1c560cf5 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -2745,3 +2745,119 @@ async def test_create_using_alter_against_existing_table( insert_rows_event = ds_write._tracked_events[1] assert insert_rows_event.name == "insert-rows" assert insert_rows_event.num_rows == 1 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("denied_action", "request_body"), + ( + ( + "insert-row", + { + "table": "salaries", + "rows": [{"id": 9, "note": "INJ-VIA-CREATE"}], + }, + ), + ( + "update-row", + { + "table": "salaries", + "rows": [{"id": 1, "note": "REPLACED"}], + "pk": "id", + "replace": True, + }, + ), + ( + "alter-table", + { + "table": "salaries", + "rows": [{"id": 9, "note": "INSERTED", "extra": "NEW"}], + "alter": True, + }, + ), + ), +) +async def test_create_table_existing_table_respects_table_level_denial( + denied_action, request_body +): + # GHSA-53fc-rhfg-h7qp issue 2: POST /db/-/create against an existing table + # inserts rows into it, so insert-row (and update-row / alter-table) must be + # checked against the TableResource, not just the DatabaseResource. + ds = Datasette( + memory=True, + config={ + "databases": { + # id=editor user has each permission at the database level, but + # the selected action is explicitly denied on the salaries table + "data": { + "permissions": { + "create-table": {"id": "editor"}, + "insert-row": {"id": "editor"}, + "update-row": {"id": "editor"}, + "alter-table": {"id": "editor"}, + }, + "tables": { + "salaries": {"permissions": {denied_action: False}}, + }, + } + } + }, + ) + db = ds.add_memory_database( + f"create_table_existing_table_denied_{denied_action}", name="data" + ) + await db.execute_write("create table salaries (id integer primary key, note text)") + await db.execute_write("insert into salaries values (1, 'TOPSECRET-A')") + await ds.invoke_startup() + + if denied_action == "insert-row": + # Sanity: direct insert into salaries is denied for this actor + direct = await ds.client.post( + "/data/salaries/-/insert", + actor={"id": "editor"}, + json={"row": {"id": 9, "note": "INJ-DIRECT"}}, + ) + assert direct.status_code == 403 + + response = await ds.client.post( + "/data/-/create", + actor={"id": "editor"}, + json=request_body, + ) + assert response.status_code == 403, response.json() + assert response.json()["errors"] == [f"Permission denied: need {denied_action}"] + rows = (await db.execute("select id, note from salaries order by id")).rows + assert [tuple(r) for r in rows] == [(1, "TOPSECRET-A")] + assert await db.table_columns("salaries") == ["id", "note"] + + +@pytest.mark.asyncio +async def test_create_table_respects_predeclared_table_level_denial(): + ds = Datasette( + memory=True, + config={ + "databases": { + "data": { + "permissions": { + "create-table": {"id": "editor"}, + "insert-row": {"id": "editor"}, + }, + "tables": { + "planned_table": {"permissions": {"insert-row": False}}, + }, + } + } + }, + ) + db = ds.add_memory_database("create_table_predeclared_denial", name="data") + await ds.invoke_startup() + + response = await ds.client.post( + "/data/-/create", + actor={"id": "editor"}, + json={"table": "planned_table", "rows": [{"id": 1}]}, + ) + + assert response.status_code == 403, response.json() + assert response.json()["errors"] == ["Permission denied: need insert-row"] + assert not await db.table_exists("planned_table") From 577aeb73f06ec48df630e75af47713bf029fc0c8 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:45 -0700 Subject: [PATCH 009/108] Disallow ?_through= if user lacks view-table permission Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/filters.py | 7 ++++++- tests/test_table_api.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/datasette/filters.py b/datasette/filters.py index 3cfb36e5..af922eda 100644 --- a/datasette/filters.py +++ b/datasette/filters.py @@ -2,7 +2,7 @@ import json from typing import ClassVar from datasette import hookimpl -from datasette.resources import DatabaseResource +from datasette.resources import DatabaseResource, TableResource from datasette.utils.asgi import BadRequest from datasette.views.base import DatasetteError @@ -135,6 +135,11 @@ def through_filters(request, database, table, datasette): through_table = through_data["table"] other_column = through_data["column"] value = through_data["value"] + await datasette.ensure_permission( + action="view-table", + resource=TableResource(database=database, table=through_table), + actor=request.actor, + ) db = datasette.get_database(database) outgoing_foreign_keys = await db.foreign_keys_for_table(through_table) fk_to_us = next( diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 6c0c021b..ec4a1368 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -1778,3 +1778,34 @@ async def test_next_url_included_by_default(ds_client): data = response.json() assert data["next"] is None assert data["next_url"] is None + + +@pytest.mark.asyncio +async def test_table_through_requires_view_table_on_through_table(): + # GHSA-53fc-rhfg-h7qp issue 3: ?_through= runs a sub-select against the + # caller-supplied through table, so the actor must be allowed to view it. + # Otherwise it is an equality oracle over any column of a denied table. + from datasette.app import Datasette + + ds = Datasette( + memory=True, + config={"databases": {"data": {"tables": {"salaries": {"allow": False}}}}}, + ) + db = ds.add_memory_database("table_through_denied", name="data") + await db.execute_write("create table people (id integer primary key, name text)") + await db.execute_write( + "create table salaries (id integer primary key, " + "person_id integer references people(id), note text)" + ) + await db.execute_write("insert into people values (1, 'alice'), (2, 'bob')") + await db.execute_write("insert into salaries values (1, 1, 'TOPSECRET-A')") + await ds.invoke_startup() + + # Sanity: anonymous cannot read salaries directly + assert (await ds.client.get("/data/salaries.json")).status_code == 403 + + response = await ds.client.get( + "/data/people.json?_shape=array" + '&_through={"table":"salaries","column":"note","value":"TOPSECRET-A"}' + ) + assert response.status_code == 403, response.text From f8e8e65af7403666f227bb6f0d523bcf2d1e11aa Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:48 -0700 Subject: [PATCH 010/108] actor cookie respects expire_after Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/app.py | 2 +- tests/test_auth.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/datasette/app.py b/datasette/app.py index b89ab30c..6683d4dc 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2462,7 +2462,7 @@ class Datasette: ): data = {"a": actor} if expire_after: - expires_at = int(time.time()) + (24 * 60 * 60) + expires_at = int(time.time()) + expire_after data["e"] = baseconv.base62.encode(expires_at) response.set_cookie("ds_actor", self.sign(data, "actor")) diff --git a/tests/test_auth.py b/tests/test_auth.py index e7a5402e..6024e3bb 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -524,3 +524,25 @@ async def test_root_without_root_enabled_no_special_permissions(ds_client): ) is not True ), "Root without root_enabled should not automatically get set-column-type" + + +@pytest.mark.parametrize("expire_after", (1, 300, 3600, 30 * 24 * 60 * 60)) +def test_set_actor_cookie_honours_expire_after(expire_after): + # GHSA-53fc-rhfg-h7qp issue 4: expire_after is documented as a number of + # seconds, but every value was being replaced with 24 hours. + from datasette.app import Datasette + from datasette.utils.asgi import Response + + ds = Datasette(memory=True) + response = Response.text("") + before = int(time.time()) + ds.set_actor_cookie(response, {"id": "test"}, expire_after=expire_after) + after = int(time.time()) + + (header,) = response._set_cookie_headers + assert header.startswith("ds_actor=") + value = header[len("ds_actor=") :].split(";", 1)[0] + data = ds.unsign(value, "actor") + assert data["a"] == {"id": "test"} + expires_at = baseconv.base62.decode(data["e"]) + assert before + expire_after <= expires_at <= after + expire_after From 435e55ff0a254a77f700a06f5c31bb9f3bf31764 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:53 -0700 Subject: [PATCH 011/108] Remove JSON syntax highlighting Refs GHSA-hp2x-vx2r-6vxg Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- .../static/json-format-highlight-1.0.1.js | 56 ------------------- datasette/templates/api_explorer.html | 5 +- datasette/templates/debug_allowed.html | 5 +- datasette/templates/debug_check.html | 5 +- datasette/templates/debug_rules.html | 5 +- 5 files changed, 8 insertions(+), 68 deletions(-) delete mode 100644 datasette/static/json-format-highlight-1.0.1.js diff --git a/datasette/static/json-format-highlight-1.0.1.js b/datasette/static/json-format-highlight-1.0.1.js deleted file mode 100644 index 0e6e2c29..00000000 --- a/datasette/static/json-format-highlight-1.0.1.js +++ /dev/null @@ -1,56 +0,0 @@ -/* -https://github.com/luyilin/json-format-highlight -From https://unpkg.com/json-format-highlight@1.0.1/dist/json-format-highlight.js -MIT Licensed -*/ -(function (global, factory) { - typeof exports === "object" && typeof module !== "undefined" - ? (module.exports = factory()) - : typeof define === "function" && define.amd - ? define(factory) - : (global.jsonFormatHighlight = factory()); -})(this, function () { - "use strict"; - - var defaultColors = { - keyColor: "dimgray", - numberColor: "lightskyblue", - stringColor: "lightcoral", - trueColor: "lightseagreen", - falseColor: "#f66578", - nullColor: "cornflowerblue", - }; - - function index(json, colorOptions) { - if (colorOptions === void 0) colorOptions = {}; - - if (!json) { - return; - } - if (typeof json !== "string") { - json = JSON.stringify(json, null, 2); - } - var colors = Object.assign({}, defaultColors, colorOptions); - json = json.replace(/&/g, "&").replace(//g, ">"); - return json.replace( - /("(\\u[a-zA-Z0-9]{4}|\\[^u]|[^\\"])*"(\s*:)?|\b(true|false|null)\b|-?\d+(?:\.\d*)?(?:[eE][+]?\d+)?)/g, - function (match) { - var color = colors.numberColor; - if (/^"/.test(match)) { - color = /:$/.test(match) ? colors.keyColor : colors.stringColor; - } else { - color = /true/.test(match) - ? colors.trueColor - : /false/.test(match) - ? colors.falseColor - : /null/.test(match) - ? colors.nullColor - : color; - } - return '' + match + ""; - }, - ); - } - - return index; -}); diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 4927cb8d..32686af1 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -3,7 +3,6 @@ {% block title %}API Explorer{% endblock %} {% block extra_head %} - {% endblock %} {% block content %} @@ -126,7 +125,7 @@ getForm.addEventListener("submit", (ev) => { document.getElementById('response-status').textContent = response.status; return response.json(); }).then((data) => { - output.querySelector('pre').innerHTML = jsonFormatHighlight(data); + output.querySelector('pre').textContent = JSON.stringify(data, null, 2); errorList.style.display = 'none'; }).catch((error) => { alert(error); @@ -174,7 +173,7 @@ postForm.addEventListener("submit", (ev) => { } else { errorList.style.display = 'none'; } - output.querySelector('pre').innerHTML = jsonFormatHighlight(data); + output.querySelector('pre').textContent = JSON.stringify(data, null, 2); output.style.display = 'block'; }).catch(err => { alert("Error: " + err); diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html index 80249d9c..c73cdfb7 100644 --- a/datasette/templates/debug_allowed.html +++ b/datasette/templates/debug_allowed.html @@ -3,7 +3,6 @@ {% block title %}Allowed Resources{% endblock %} {% block extra_head %} - {% include "_permission_ui_styles.html" %} {% include "_debug_common_functions.html" %} {% endblock %} @@ -198,7 +197,7 @@ function displayResults(data) { } // Update raw JSON - document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data); + document.getElementById('raw-json').textContent = JSON.stringify(data, null, 2); } function displayError(data) { @@ -208,7 +207,7 @@ function displayError(data) { resultsContent.innerHTML = `
Error: ${escapeHtml(data.error || 'Unknown error')}
`; - document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data); + document.getElementById('raw-json').textContent = JSON.stringify(data, null, 2); } // Disable child input if parent is empty diff --git a/datasette/templates/debug_check.html b/datasette/templates/debug_check.html index b9fc636a..c0081c66 100644 --- a/datasette/templates/debug_check.html +++ b/datasette/templates/debug_check.html @@ -3,7 +3,6 @@ {% block title %}Explain a permission decision{% endblock %} {% block extra_head %} - {% include "_permission_ui_styles.html" %} {% include "_debug_common_functions.html" %} - +
@@ -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 From 328b2e6c6f77cba22e8c6536afe7a6ec5e4b8953 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:24 -0700 Subject: [PATCH 092/108] Refactor the column chooser to use the shared modal, refs #2790 --- datasette/static/column-chooser.js | 137 +++++------------------------ tests/test_playwright.py | 2 +- 2 files changed, 22 insertions(+), 117 deletions(-) diff --git a/datasette/static/column-chooser.js b/datasette/static/column-chooser.js index c3d5796c..f0fac0ec 100644 --- a/datasette/static/column-chooser.js +++ b/datasette/static/column-chooser.js @@ -41,74 +41,22 @@ class ColumnChooser extends HTMLElement { * { box-sizing: border-box; margin: 0; padding: 0; } - dialog { - border: none; - border-radius: var(--modal-border-radius, 0.75rem); - padding: 0; - margin: auto; + dialog.datasette-modal { width: 100%; max-width: 420px; max-height: min(640px, 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: slideIn var(--modal-animation-duration, 0.2s) ease-out; - overflow: hidden; - font-family: system-ui, -apple-system, sans-serif; - background: var(--card); -webkit-user-select: none; -webkit-touch-callout: none; -webkit-tap-highlight-color: transparent; } - dialog[open] { - display: flex; - flex-direction: column; + dialog.datasette-modal[open] { height: min(640px, calc(100vh - 32px)); } - 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; } - } - .modal-header { padding: 20px 24px 16px; - border-bottom: 1px solid var(--rule); - display: flex; - align-items: center; justify-content: space-between; - flex-shrink: 0; - } - - .modal-title { - font-size: 1rem; - font-weight: 600; - } - - .modal-meta { - font-family: ui-monospace, monospace; - font-size: 0.7rem; - color: var(--muted); - background: var(--paper); - padding: 3px 9px; - border-radius: 20px; } .list-toolbar { @@ -299,47 +247,10 @@ class ColumnChooser extends HTMLElement { 50% { transform: translateX(-50%) scale(1.5); opacity: 0.07; } } - .modal-footer { - padding: 14px 20px; - border-top: 1px solid var(--rule); - display: flex; - align-items: center; - gap: 10px; - flex-shrink: 0; - background: var(--paper); - } - - .footer-info { - flex: 1; - font-family: ui-monospace, monospace; - font-size: 0.68rem; - color: var(--muted); - } - - .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; - } - - .btn-primary { - background: var(--accent); + .modal-btn-primary { color: white; } - .btn-primary:hover { background: #1448c0; } - - .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); - } - .btn-ghost:hover { background: var(--rule); color: var(--ink); } + .modal-btn-primary:hover { background: #1448c0; } .list-wrap::-webkit-scrollbar { width: 5px; } .list-wrap::-webkit-scrollbar-track { background: transparent; } @@ -348,7 +259,7 @@ class ColumnChooser extends HTMLElement { input, textarea { -webkit-user-select: auto; user-select: auto; } - +
Choose columns @@ -364,14 +275,14 @@ class ColumnChooser extends HTMLElement {
- - + +
- + `; // DOM refs - this._dialog = this.shadowRoot.querySelector("dialog"); + this._modal = this.shadowRoot.querySelector("datasette-modal"); this._listWrap = this.shadowRoot.getElementById("listWrap"); this._dragList = this.shadowRoot.getElementById("dragList"); this._pulseTop = this.shadowRoot.getElementById("pulseTop"); @@ -386,15 +297,17 @@ class ColumnChooser extends HTMLElement { // Event listeners this._selectAllBtn.addEventListener("click", () => this._selectAll()); this._deselectAllBtn.addEventListener("click", () => this._deselectAll()); - this._cancelBtn.addEventListener("click", () => this._close()); + this._cancelBtn.addEventListener("click", () => + this._modal.requestClose("cancel"), + ); this._applyBtn.addEventListener("click", () => this._apply()); - this._dialog.addEventListener("click", (e) => { - if (e.target === this._dialog) this._close(); - }); - this._dialog.addEventListener("cancel", (e) => { - e.preventDefault(); - this._close(); - }); + this._modal.beforeClose = () => { + this._items = this._savedItems ? [...this._savedItems] : this._items; + this._checked = this._savedChecked + ? new Set(this._savedChecked) + : this._checked; + return true; + }; } /** @@ -414,19 +327,11 @@ class ColumnChooser extends HTMLElement { this._savedChecked = new Set(this._checked); this._render(); - this._dialog.showModal(); + this._modal.show(); } // ── Internal methods ── - _close() { - this._items = this._savedItems ? [...this._savedItems] : this._items; - this._checked = this._savedChecked - ? new Set(this._savedChecked) - : this._checked; - this._dialog.close(); - } - _selectAll() { this._items.forEach((col) => this._checked.add(col)); this._dragList.querySelectorAll('input[type="checkbox"]').forEach((cb) => { @@ -445,7 +350,7 @@ class ColumnChooser extends HTMLElement { _apply() { const selected = this._items.filter((col) => this._checked.has(col)); - this._dialog.close(); + this._modal.close(); if (this._onApply) { this._onApply(selected); } diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 32364b0f..a76bd8ec 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1763,7 +1763,7 @@ def test_modal_lifecycle(page, datasette_server, shadow): @pytest.mark.playwright -@pytest.mark.parametrize("name", ["jump"]) +@pytest.mark.parametrize("name", ["jump", "columns"]) def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name): from playwright.sync_api import expect From 17b19b4d27087a1866b521b9008c29a6dd0cf4ce Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:24 -0700 Subject: [PATCH 093/108] Refactor mobile column actions to use the shared modal, refs #2790 --- datasette/static/app.css | 88 ----------------------- datasette/static/mobile-column-actions.js | 40 +++-------- tests/test_playwright.py | 2 +- 3 files changed, 9 insertions(+), 121 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 70150f10..f8f033bb 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -982,61 +982,13 @@ p.zero-results { } dialog.mobile-column-actions-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(420px, calc(100vw - 32px)); - max-width: 95vw; max-height: min(640px, 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.mobile-column-actions-dialog[open] { - display: flex; - flex-direction: column; -} - -dialog.mobile-column-actions-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; } .mobile-column-actions-dialog .modal-header { padding: 20px 24px 16px; - border-bottom: 1px solid var(--rule); - display: flex; - align-items: center; justify-content: space-between; - gap: 12px; - flex-shrink: 0; -} - -.mobile-column-actions-dialog .modal-title { - font-size: 1rem; - font-weight: 600; - color: var(--ink); -} - -.mobile-column-actions-dialog .modal-meta { - font-family: ui-monospace, monospace; - font-size: 0.7rem; - color: var(--muted); - background: var(--paper); - padding: 3px 9px; - border-radius: 20px; } .mobile-column-actions-dialog .list-wrap { @@ -1169,46 +1121,6 @@ dialog.mobile-column-actions-dialog::backdrop { font-size: 0.85em; } -.mobile-column-actions-dialog .modal-footer { - padding: 14px 20px; - border-top: 1px solid var(--rule); - display: flex; - align-items: center; - gap: 10px; - flex-shrink: 0; - background: var(--paper); -} - -.mobile-column-actions-dialog .footer-info { - flex: 1; - font-family: ui-monospace, monospace; - font-size: 0.68rem; - color: var(--muted); -} - -.mobile-column-actions-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; -} - -.mobile-column-actions-dialog .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -.mobile-column-actions-dialog .btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - dialog.set-column-type-dialog { --ink: #0f0f0f; --paper: #eef6ff; diff --git a/datasette/static/mobile-column-actions.js b/datasette/static/mobile-column-actions.js index a386b1fc..29082d4e 100644 --- a/datasette/static/mobile-column-actions.js +++ b/datasette/static/mobile-column-actions.js @@ -66,7 +66,8 @@ function initMobileColumnActions(manager) { return; } - var dialog = document.createElement("dialog"); + var modal = DatasetteModal.create(); + var dialog = modal.dialog; dialog.className = "mobile-column-actions-dialog"; dialog.id = MOBILE_COLUMN_DIALOG_ID; dialog.setAttribute("aria-labelledby", MOBILE_COLUMN_DIALOG_TITLE_ID); @@ -78,10 +79,10 @@ function initMobileColumnActions(manager) {
Tap a column to reveal actions. - +
`; - document.body.appendChild(dialog); + document.body.appendChild(modal); triggerButton.setAttribute("aria-haspopup", "dialog"); triggerButton.setAttribute("aria-controls", MOBILE_COLUMN_DIALOG_ID); @@ -91,7 +92,6 @@ function initMobileColumnActions(manager) { var listWrap = dialog.querySelector(".mobile-column-list"); var doneButton = dialog.querySelector(".mobile-column-actions-done"); var expandedSectionId = null; - var shouldRestoreFocus = true; function updateExpandedSection() { Array.from(dialog.querySelectorAll(".col-header")).forEach((button) => { @@ -128,16 +128,7 @@ function initMobileColumnActions(manager) { } function closeDialog(options) { - options = options || {}; - shouldRestoreFocus = options.restoreFocus !== false; - if (dialog.open) { - dialog.close(); - } else { - triggerButton.setAttribute("aria-expanded", "false"); - if (shouldRestoreFocus) { - triggerButton.focus(); - } - } + modal.close(options); } function renderDialog() { @@ -166,7 +157,8 @@ function initMobileColumnActions(manager) { topActions.className = "mobile-column-top-actions"; var showAllColumns = document.createElement("a"); - showAllColumns.className = "btn btn-ghost mobile-column-top-action"; + showAllColumns.className = + "modal-btn modal-btn-ghost mobile-column-top-action"; showAllColumns.href = manager.columnActions.showAllColumnsUrl(); showAllColumns.textContent = "Show all columns"; @@ -265,9 +257,7 @@ function initMobileColumnActions(manager) { if (!renderDialog()) { return; } - if (!dialog.open) { - dialog.showModal(); - } + modal.show({ trigger: triggerButton }); triggerButton.setAttribute("aria-expanded", "true"); var focusTarget = dialog.querySelector(".mobile-column-top-action") || @@ -288,22 +278,8 @@ function initMobileColumnActions(manager) { closeDialog(); }); - dialog.addEventListener("click", function (ev) { - if (ev.target === dialog) { - closeDialog(); - } - }); - - dialog.addEventListener("cancel", function (ev) { - ev.preventDefault(); - closeDialog(); - }); - dialog.addEventListener("close", function () { triggerButton.setAttribute("aria-expanded", "false"); - if (shouldRestoreFocus) { - triggerButton.focus(); - } }); window.addEventListener("resize", function () { diff --git a/tests/test_playwright.py b/tests/test_playwright.py index a76bd8ec..bc582c8a 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1763,7 +1763,7 @@ def test_modal_lifecycle(page, datasette_server, shadow): @pytest.mark.playwright -@pytest.mark.parametrize("name", ["jump", "columns"]) +@pytest.mark.parametrize("name", ["jump", "columns", "mobile"]) def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name): from playwright.sync_api import expect From de37f1451f70c46092077c8ec2919a4a3ad48eb6 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:24 -0700 Subject: [PATCH 094/108] Refactor the column type dialog to use the shared modal, refs #2790 --- datasette/static/app.css | 104 --------------------------- datasette/static/table.js | 145 +++++++++++++++++++------------------- tests/test_playwright.py | 2 +- 3 files changed, 72 insertions(+), 179 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index f8f033bb..32f1a84f 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1122,61 +1122,11 @@ dialog.mobile-column-actions-dialog { } dialog.set-column-type-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(520px, calc(100vw - 32px)); - max-width: 95vw; max-height: min(720px, 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.set-column-type-dialog[open] { - display: flex; - flex-direction: column; -} - -dialog.set-column-type-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; } .set-column-type-dialog .modal-header { - padding: 20px 24px 12px; - border-bottom: 1px solid var(--rule); - display: flex; - align-items: center; justify-content: space-between; - gap: 12px; - flex-shrink: 0; -} - -.set-column-type-dialog .modal-title { - font-size: 1rem; - font-weight: 600; - color: var(--ink); -} - -.set-column-type-dialog .modal-meta { - font-family: ui-monospace, monospace; - font-size: 0.7rem; - color: var(--muted); - background: var(--paper); - padding: 3px 9px; - border-radius: 20px; } .set-column-type-status, @@ -1241,60 +1191,6 @@ dialog.set-column-type-dialog::backdrop { font-size: 0.9rem; } -.set-column-type-dialog .modal-footer { - padding: 14px 20px; - border-top: 1px solid var(--rule); - display: flex; - align-items: center; - gap: 10px; - flex-shrink: 0; - background: var(--paper); -} - -.set-column-type-dialog .footer-info { - flex: 1; - font-family: ui-monospace, monospace; - font-size: 0.68rem; - color: var(--muted); -} - -.set-column-type-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; -} - -.set-column-type-dialog .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -.set-column-type-dialog .btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - -.set-column-type-dialog .btn-primary { - background: var(--accent); - color: #fff; -} - -.set-column-type-dialog .btn-primary:hover { - background: #1949b8; -} - -.set-column-type-dialog .btn:disabled { - opacity: 0.65; - cursor: wait; -} - .row-mutation-status { margin: 0 0 0.75rem; padding: 8px 10px; diff --git a/datasette/static/table.js b/datasette/static/table.js index 143e976f..f1e05604 100644 --- a/datasette/static/table.js +++ b/datasette/static/table.js @@ -157,6 +157,7 @@ function createSetColumnTypeOption(value, name, description, checked) { function setSetColumnTypeDialogBusy(state, isBusy) { state.isBusy = isBusy; + state.modal.busy = isBusy; state.saveButton.disabled = isBusy; state.cancelButton.disabled = isBusy; Array.from( @@ -185,7 +186,8 @@ function ensureSetColumnTypeDialog() { return null; } - var dialog = document.createElement("dialog"); + var modal = DatasetteModal.create(); + var dialog = modal.dialog; dialog.id = SET_COLUMN_TYPE_DIALOG_ID; dialog.className = "set-column-type-dialog"; dialog.setAttribute("aria-labelledby", "set-column-type-title"); @@ -199,13 +201,14 @@ function ensureSetColumnTypeDialog() {
- - + +
`; - document.body.appendChild(dialog); + document.body.appendChild(modal); setColumnTypeDialogState = { + modal: modal, dialog: dialog, meta: dialog.querySelector(".modal-meta"), status: dialog.querySelector(".set-column-type-status"), @@ -220,21 +223,7 @@ function ensureSetColumnTypeDialog() { }; setColumnTypeDialogState.cancelButton.addEventListener("click", function () { - if (!setColumnTypeDialogState.isBusy) { - dialog.close(); - } - }); - - dialog.addEventListener("click", function (ev) { - if (ev.target === dialog && !setColumnTypeDialogState.isBusy) { - dialog.close(); - } - }); - - dialog.addEventListener("cancel", function (ev) { - if (setColumnTypeDialogState.isBusy) { - ev.preventDefault(); - } + modal.requestClose("cancel"); }); dialog.addEventListener("close", function () { @@ -242,49 +231,52 @@ function ensureSetColumnTypeDialog() { setSetColumnTypeDialogBusy(setColumnTypeDialogState, false); }); - setColumnTypeDialogState.saveButton.addEventListener("click", async function () { - var state = setColumnTypeDialogState; - var selected = state.dialog.querySelector( - 'input[name="set-column-type-choice"]:checked', - ); - var selectedType = selected ? selected.value : ""; - var currentType = state.currentConfig.current - ? state.currentConfig.current.type - : ""; + setColumnTypeDialogState.saveButton.addEventListener( + "click", + async function () { + var state = setColumnTypeDialogState; + var selected = state.dialog.querySelector( + 'input[name="set-column-type-choice"]:checked', + ); + var selectedType = selected ? selected.value : ""; + var currentType = state.currentConfig.current + ? state.currentConfig.current.type + : ""; - if (selectedType === currentType) { - state.dialog.close(); - return; - } - - clearSetColumnTypeDialogError(state); - setSetColumnTypeDialogBusy(state, true); - - var payload = { - column: state.currentColumn, - column_type: selectedType ? { type: selectedType } : null, - }; - - try { - var response = await fetch(getSetColumnTypeData().path, { - method: "POST", - headers: { - "Content-Type": "application/json", - Accept: "application/json", - }, - body: JSON.stringify(payload), - }); - var data = await response.json(); - if (!response.ok || data.ok === false) { - var message = (data.errors || ["Request failed"]).join(" "); - throw new Error(message); + if (selectedType === currentType) { + state.modal.close(); + return; } - location.reload(); - } catch (error) { - setSetColumnTypeDialogBusy(state, false); - showSetColumnTypeDialogError(state, error.message || "Request failed"); - } - }); + + clearSetColumnTypeDialogError(state); + setSetColumnTypeDialogBusy(state, true); + + var payload = { + column: state.currentColumn, + column_type: selectedType ? { type: selectedType } : null, + }; + + try { + var response = await fetch(getSetColumnTypeData().path, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json", + }, + body: JSON.stringify(payload), + }); + var data = await response.json(); + if (!response.ok || data.ok === false) { + var message = (data.errors || ["Request failed"]).join(" "); + throw new Error(message); + } + location.reload(); + } catch (error) { + setSetColumnTypeDialogBusy(state, false); + showSetColumnTypeDialogError(state, error.message || "Request failed"); + } + }, + ); return setColumnTypeDialogState; } @@ -341,9 +333,7 @@ function openSetColumnTypeDialog(th) { state.optionsWrap.appendChild(emptyState); } - if (!state.dialog.open) { - state.dialog.showModal(); - } + state.modal.show(); var selectedOption = state.dialog.querySelector( 'input[name="set-column-type-choice"]:checked', ); @@ -367,9 +357,10 @@ function shouldShowShowAllColumns() { function hasMultipleVisibleColumns(manager) { return ( - Array.from(document.querySelectorAll(manager.selectors.tableHeaders)).filter( - (th) => th.dataset.column && th.dataset.isLinkColumn !== "1", - ).length > 1 + Array.from( + document.querySelectorAll(manager.selectors.tableHeaders), + ).filter((th) => th.dataset.column && th.dataset.isLinkColumn !== "1") + .length > 1 ); } @@ -649,10 +640,12 @@ function filterRowNumberFromName(name) { } function nextFilterRowNumber(manager) { - return filterRowsWithControls(manager).reduce((max, row) => { - var column = row.querySelector("select"); - return Math.max(max, filterRowNumberFromName(column && column.name)); - }, 0) + 1; + return ( + filterRowsWithControls(manager).reduce((max, row) => { + var column = row.querySelector("select"); + return Math.max(max, filterRowNumberFromName(column && column.name)); + }, 0) + 1 + ); } function setFilterRowNumber(row, number) { @@ -679,9 +672,11 @@ function updateFilterRowButtons(manager) { if (addButton) { addButton.hidden = index !== rows.length - 1 || !column.value; } - var visibleButtonCount = [removeButton, addButton].filter(function (button) { - return button && !button.hidden; - }).length; + var visibleButtonCount = [removeButton, addButton].filter( + function (button) { + return button && !button.hidden; + }, + ).length; row.classList.toggle( "filter-controls-row-has-buttons", visibleButtonCount > 0, @@ -703,7 +698,9 @@ function cloneFilterRow(row) { clone.querySelector(".filter-op select").name = "_filter_op"; clone.querySelector("input.filter-value").name = "_filter_value"; resetFilterRow(clone); - clone.querySelectorAll(".filter-row-icon").forEach((button) => button.remove()); + clone + .querySelectorAll(".filter-row-icon") + .forEach((button) => button.remove()); return clone; } diff --git a/tests/test_playwright.py b/tests/test_playwright.py index bc582c8a..0aa5fd84 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1763,7 +1763,7 @@ def test_modal_lifecycle(page, datasette_server, shadow): @pytest.mark.playwright -@pytest.mark.parametrize("name", ["jump", "columns", "mobile"]) +@pytest.mark.parametrize("name", ["jump", "columns", "type", "mobile"]) def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name): from playwright.sync_api import expect From c82a98c88a0263a051a5fd1eaa39ded62bde452e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:25 -0700 Subject: [PATCH 095/108] 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() From 814165c8b1fb0f5e9786285f125239e8b1f655be Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:25 -0700 Subject: [PATCH 096/108] Refactor the alter table dialog to use the shared modal, refs #2790 --- datasette/static/app.css | 98 +++------------------------------- datasette/static/edit-tools.js | 84 +++++++---------------------- tests/test_playwright.py | 7 ++- 3 files changed, 30 insertions(+), 159 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index ce24cb5b..cadc5eb3 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -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; diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index 1e9b1aca..eb0144a7 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -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) {
- - - - + + + +
`; - 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(); diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 63a6f0e6..b14a2359 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -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 From 90f543327e193a8043022f521e930e0290fc0068 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:26 -0700 Subject: [PATCH 097/108] Refactor row deletion to use the shared modal, refs #2790 --- datasette/static/app.css | 82 ---------------------------------- datasette/static/edit-tools.js | 62 +++++-------------------- tests/test_playwright.py | 2 +- 3 files changed, 12 insertions(+), 134 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index cadc5eb3..9a57ad90 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1224,46 +1224,11 @@ button.table-insert-row svg { } dialog.row-delete-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(440px, calc(100vw - 32px)); - max-width: 95vw; - 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.row-delete-dialog[open] { - display: flex; - flex-direction: column; -} - -dialog.row-delete-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; } .row-delete-dialog .modal-header { - padding: 20px 24px 12px; - border-bottom: 1px solid var(--rule); - display: flex; - align-items: center; justify-content: flex-start; - gap: 12px; - flex-shrink: 0; - min-width: 0; } .row-delete-dialog .modal-title { @@ -1272,9 +1237,6 @@ dialog.row-delete-dialog::backdrop { gap: 0.35rem; min-width: 0; max-width: 100%; - font-size: 1rem; - font-weight: 600; - color: var(--ink); } .row-delete-message, @@ -1306,53 +1268,9 @@ dialog.row-delete-dialog::backdrop { .row-delete-dialog .modal-footer { padding: 18px 20px 14px; - border-top: 1px solid var(--rule); - display: flex; - align-items: center; - justify-content: flex-end; - gap: 10px; - flex-shrink: 0; - background: var(--paper); margin-top: 18px; } -.row-delete-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; -} - -.row-delete-dialog .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -.row-delete-dialog .btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - -.row-delete-dialog .btn-primary { - background: var(--accent); - color: #fff; -} - -.row-delete-dialog .btn-primary:hover { - background: #1949b8; -} - -.row-delete-dialog .btn:disabled { - opacity: 0.65; - cursor: wait; -} - dialog.row-edit-dialog { --ink: #0f0f0f; --paper: #eef6ff; diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index eb0144a7..402af956 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -2410,6 +2410,7 @@ function initTableCreateActions(manager) { function setRowDeleteDialogBusy(state, isBusy) { state.isBusy = isBusy; + state.modal.busy = isBusy; state.confirmButton.disabled = isBusy; state.cancelButton.disabled = isBusy; state.confirmButton.textContent = isBusy ? "Deleting..." : "Delete row"; @@ -4360,7 +4361,8 @@ function ensureRowDeleteDialog(manager) { return null; } - var dialog = document.createElement("dialog"); + var modal = DatasetteModal.create(); + var dialog = modal.dialog; dialog.id = ROW_DELETE_DIALOG_ID; dialog.className = "row-delete-dialog"; dialog.setAttribute("aria-labelledby", "row-delete-title"); @@ -4372,13 +4374,14 @@ function ensureRowDeleteDialog(manager) {

Delete row ?

- - + +
`; - document.body.appendChild(dialog); + document.body.appendChild(modal); rowDeleteDialogState = { + modal: modal, dialog: dialog, title: dialog.querySelector(".modal-title"), message: dialog.querySelector(".row-delete-message"), @@ -4391,21 +4394,10 @@ function ensureRowDeleteDialog(manager) { currentPkPath: null, manager: manager, isBusy: false, - shouldRestoreFocus: true, }; rowDeleteDialogState.cancelButton.addEventListener("click", function () { - if (!rowDeleteDialogState.isBusy) { - rowDeleteDialogState.shouldRestoreFocus = true; - dialog.close(); - } - }); - - dialog.addEventListener("click", function (ev) { - if (ev.target === dialog && !rowDeleteDialogState.isBusy) { - rowDeleteDialogState.shouldRestoreFocus = true; - dialog.close(); - } + modal.requestClose("cancel"); }); dialog.addEventListener("keydown", function (ev) { @@ -4417,25 +4409,6 @@ function ensureRowDeleteDialog(manager) { if (!rowDeleteDialogState.isBusy) { rowDeleteDialogState.confirmButton.click(); } - return; - } - if (ev.key !== "Escape") { - return; - } - if (rowDeleteDialogState.isBusy) { - ev.preventDefault(); - return; - } - ev.preventDefault(); - rowDeleteDialogState.shouldRestoreFocus = true; - dialog.close(); - }); - - dialog.addEventListener("cancel", function (ev) { - if (rowDeleteDialogState.isBusy) { - ev.preventDefault(); - } else { - rowDeleteDialogState.shouldRestoreFocus = true; } }); @@ -4443,13 +4416,6 @@ function ensureRowDeleteDialog(manager) { var state = rowDeleteDialogState; clearRowDeleteDialogError(state); setRowDeleteDialogBusy(state, false); - if ( - state.shouldRestoreFocus && - state.currentButton && - document.contains(state.currentButton) - ) { - state.currentButton.focus(); - } }); rowDeleteDialogState.confirmButton.addEventListener( @@ -4476,8 +4442,7 @@ function ensureRowDeleteDialog(manager) { throw rowMutationRequestError(response, data); } if (data && data.redirect) { - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); location.href = data.redirect; return; } @@ -4489,8 +4454,7 @@ function ensureRowDeleteDialog(manager) { var statusMessage = state.currentPkPath ? "Deleted row " + state.currentPkPath + "." : "Deleted row."; - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); state.currentRow.remove(); showRowMutationStatus(state.manager, statusMessage, false); if (focusTarget && document.contains(focusTarget)) { @@ -4519,11 +4483,9 @@ function openRowDeleteDialog(button, manager) { } state.manager = manager; - state.currentButton = button; state.currentRow = row; state.currentDeleteUrl = rowDeleteUrl(row); state.currentPkPath = rowDisplayLabel(row); - state.shouldRestoreFocus = true; clearRowDeleteDialogError(state); setRowDeleteDialogBusy(state, false); @@ -4535,9 +4497,7 @@ function openRowDeleteDialog(button, manager) { ); state.rowId.textContent = state.currentPkPath || "this row"; - if (!state.dialog.open) { - state.dialog.showModal(); - } + state.modal.show({ trigger: button }); state.confirmButton.focus(); } diff --git a/tests/test_playwright.py b/tests/test_playwright.py index b14a2359..d7f6fa5c 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1603,7 +1603,7 @@ def test_delete_row_flow_removes_row(page, datasette_server): dialog = page.locator("#row-delete-dialog") dialog.wait_for() assert "Delete row 1" in dialog.inner_text() - dialog.locator(".row-delete-confirm").click() + dialog.locator(".row-delete-confirm").press("Enter") page.locator(".row-mutation-status", has_text="Deleted row 1").wait_for() page.locator('tr[data-row="1"]').wait_for(state="detached") From 3b013b7ea392aeb06d4b69713e75d95235a2a2fc Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 13:28:26 -0700 Subject: [PATCH 098/108] Refactor row editing and insertion to use the shared modal, refs #2790 --- datasette/static/app.css | 88 +---------------------- datasette/static/edit-tools.js | 128 +++++++-------------------------- tests/test_playwright.py | 61 ++++++++++++++++ 3 files changed, 88 insertions(+), 189 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 9a57ad90..6971635b 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1272,46 +1272,8 @@ dialog.row-delete-dialog { } dialog.row-edit-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(720px, 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.row-edit-dialog[open] { - display: flex; - flex-direction: column; -} - -dialog.row-edit-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; -} - -.row-edit-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; } .row-edit-dialog .modal-title { @@ -1320,9 +1282,6 @@ dialog.row-edit-dialog::backdrop { gap: 0.35rem; min-width: 0; max-width: 100%; - font-size: 1rem; - font-weight: 600; - color: var(--ink); } .row-edit-dialog .modal-title .row-dialog-action, @@ -1692,7 +1651,7 @@ textarea.row-edit-input { justify-content: flex-start; } -.row-edit-bulk-actions .btn { +.row-edit-bulk-actions .modal-btn { padding-left: 12px; padding-right: 12px; } @@ -1936,17 +1895,6 @@ datasette-autocomplete input[type="text"], max-width: 46rem; } -.row-edit-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); -} - .row-edit-mode-link { color: var(--accent); font-size: 0.9rem; @@ -1957,39 +1905,7 @@ datasette-autocomplete input[type="text"], display: none; } -.row-edit-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; -} - -.row-edit-dialog .btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -.row-edit-dialog .btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - -.row-edit-dialog .btn-primary { - background: var(--accent); - color: #fff; -} - -.row-edit-dialog .btn-primary:hover { - background: #1949b8; -} - -.row-edit-dialog .btn:disabled { +.row-edit-dialog .modal-btn:disabled { opacity: 0.55; cursor: not-allowed; } diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index 402af956..edb00c7f 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -5572,6 +5572,7 @@ function setRowEditDialogLoading(state, isLoading) { function setRowEditDialogSaving(state, isSaving) { state.isSaving = isSaving; + state.modal.busy = isSaving; updateRowEditDialogButtons(state); } @@ -5789,18 +5790,6 @@ function confirmDiscardRowEditChanges(state) { return window.confirm(message); } -function closeRowEditDialogIfConfirmed(state) { - if (!state || state.isSaving) { - return false; - } - if (!confirmDiscardRowEditChanges(state)) { - return false; - } - state.shouldRestoreFocus = true; - state.dialog.close(); - return true; -} - function setRowInsertDialogTitle(state) { var insertData = tableInsertData() || {}; var title = rowEditIsMultipleInsert(state) @@ -6626,38 +6615,6 @@ async function insertBulkPreviewRows(state) { } } -function scheduleCloseRowEditDialogIfConfirmed(state) { - // Fix for an issue in Safari where hitting Esc would show - // the confirm() prompt asking if state should be discarded - // but the Esc key press would then cancel that dialog too. - // Wait for keyup, then move the confirm() to a fresh timer tick. - if (!state || state.isSaving || state.isClosePending) { - return false; - } - if (!rowEditDialogHasChanges(state)) { - state.shouldRestoreFocus = true; - state.dialog.close(); - return true; - } - state.isClosePending = true; - var closeAfterKeyup = function () { - if (!state.isClosePending) { - return; - } - state.isClosePending = false; - closeRowEditDialogIfConfirmed(state); - }; - var onKeyup = function (ev) { - if (ev.key !== "Escape") { - return; - } - document.removeEventListener("keyup", onKeyup, true); - setTimeout(closeAfterKeyup, 0); - }; - document.addEventListener("keyup", onKeyup, true); - return true; -} - function findDataRowElement(root, rowId) { var elements = root.querySelectorAll("[data-row]"); for (var i = 0; i < elements.length; i += 1) { @@ -6747,9 +6704,8 @@ async function saveRowEditDialog(state) { } var formValues = collectRowFormValues(state); if (state.mode === "edit" && !Object.keys(formValues).length) { - state.shouldRestoreFocus = true; hideRowMutationStatus(); - state.dialog.close(); + state.modal.close(); return; } var payload = @@ -6782,9 +6738,8 @@ async function saveRowEditDialog(state) { insertedRowData, insertData.primaryKeys || [], ); - state.shouldRestoreFocus = false; if (!insertedRowId) { - state.dialog.close(); + state.modal.close({ restoreFocus: false }); var missingIdStatus = showRowMutationStatus( state.manager, "Inserted row. Refresh the page to see it.", @@ -6800,7 +6755,7 @@ async function saveRowEditDialog(state) { try { insertedRow = await fetchUpdatedRowElement(state); } catch (_error) { - state.dialog.close(); + state.modal.close({ restoreFocus: false }); var refreshFailedStatus = showRowMutationStatus( state.manager, "Inserted row, but could not refresh the table row. Refresh the page to see it.", @@ -6815,7 +6770,7 @@ async function saveRowEditDialog(state) { rowTitleLabel(insertedRow), ); var addedRow = addInsertedRowToPage(insertedRow); - state.dialog.close(); + state.modal.close({ restoreFocus: false }); showRowMutationStatus(state.manager, insertedStatusMessage, false); if (addedRow) { var insertedFocusTarget = @@ -6824,7 +6779,7 @@ async function saveRowEditDialog(state) { insertedFocusTarget.focus(); } } else { - state.dialog.close(); + state.modal.close({ restoreFocus: false }); var filteredStatus = showRowMutationStatus( state.manager, "Inserted row. It does not match the current filters.", @@ -6836,8 +6791,7 @@ async function saveRowEditDialog(state) { } if (isRowPage()) { - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); location.reload(); return; } @@ -6873,8 +6827,7 @@ async function saveRowEditDialog(state) { ); } - state.shouldRestoreFocus = false; - state.dialog.close(); + state.modal.close({ restoreFocus: false }); if (focusTarget && document.contains(focusTarget)) { focusTarget.focus(); } @@ -7018,7 +6971,8 @@ function ensureRowEditDialog(manager) { return null; } - var dialog = document.createElement("dialog"); + var modal = DatasetteModal.create(); + var dialog = modal.dialog; dialog.id = ROW_EDIT_DIALOG_ID; dialog.className = "row-edit-dialog"; dialog.setAttribute("aria-labelledby", "row-edit-title"); @@ -7048,7 +7002,7 @@ function ensureRowEditDialog(manager) {
- + You can paste the template into Google Sheets or Excel.Paste into Google Sheets or Excel
@@ -7061,14 +7015,15 @@ function ensureRowEditDialog(manager) {
- - + +
`; - document.body.appendChild(dialog); + document.body.appendChild(modal); rowEditDialogState = { + modal: modal, dialog: dialog, form: dialog.querySelector(".row-edit-form"), title: dialog.querySelector(".modal-title"), @@ -7099,7 +7054,6 @@ function ensureRowEditDialog(manager) { singleInsertLink: dialog.querySelector(".row-edit-single-insert"), cancelButton: dialog.querySelector(".row-edit-cancel"), saveButton: dialog.querySelector(".row-edit-save"), - currentButton: null, currentRow: null, currentRowId: null, currentPkPath: null, @@ -7127,9 +7081,7 @@ function ensureRowEditDialog(manager) { manager: manager, isLoading: false, isSaving: false, - isClosePending: false, hasLoaded: false, - shouldRestoreFocus: true, }; rowEditDialogState.form.addEventListener("submit", function (ev) { @@ -7149,10 +7101,7 @@ function ensureRowEditDialog(manager) { rowEditDialogState.bulkInsertTextarea.focus(); return; } - if (!rowEditDialogState.isSaving) { - rowEditDialogState.shouldRestoreFocus = true; - dialog.close(); - } + modal.requestClose("cancel"); }); rowEditDialogState.bulkInsertLink.addEventListener("click", function (ev) { @@ -7271,31 +7220,17 @@ function ensureRowEditDialog(manager) { }, ); - dialog.addEventListener("click", function (ev) { - if (ev.target === dialog) { - closeRowEditDialogIfConfirmed(rowEditDialogState); - } - }); - - dialog.addEventListener("keydown", function (ev) { - if (ev.key !== "Escape") { - return; - } - ev.preventDefault(); - scheduleCloseRowEditDialogIfConfirmed(rowEditDialogState); - }); - - dialog.addEventListener("cancel", function (ev) { - ev.preventDefault(); - scheduleCloseRowEditDialogIfConfirmed(rowEditDialogState); - }); + modal.beforeClose = function (reason) { + return ( + reason === "cancel" || confirmDiscardRowEditChanges(rowEditDialogState) + ); + }; dialog.addEventListener("close", function () { var state = rowEditDialogState; var shouldReloadOnClose = state.shouldReloadOnClose; var redirectOnCloseUrl = state.redirectOnCloseUrl; state.loadId += 1; - state.isClosePending = false; state.bulkInsertLiveValidationError = null; state.shouldReloadOnClose = false; state.redirectOnCloseUrl = null; @@ -7308,13 +7243,6 @@ function ensureRowEditDialog(manager) { destroyRowEditFields(state); setRowEditDialogLoading(state, false); setRowEditDialogSaving(state, false); - if ( - state.shouldRestoreFocus && - state.currentButton && - document.contains(state.currentButton) - ) { - state.currentButton.focus(); - } if (shouldReloadOnClose) { if (redirectOnCloseUrl) { location.href = redirectOnCloseUrl; @@ -7339,7 +7267,6 @@ async function openRowEditDialog(button, manager) { state.manager = manager; state.mode = "edit"; - state.currentButton = button; state.currentRow = row; state.currentRowId = row.getAttribute("data-row") || ""; state.currentPkPath = rowDisplayLabel(row); @@ -7356,7 +7283,7 @@ async function openRowEditDialog(button, manager) { } else { state.form.removeAttribute("action"); } - state.shouldRestoreFocus = true; + state.hasLoaded = false; state.loadId += 1; var loadId = state.loadId; @@ -7375,9 +7302,7 @@ async function openRowEditDialog(button, manager) { state.summary.textContent = ""; syncRowEditInsertModeUi(state); - if (!state.dialog.open) { - state.dialog.showModal(); - } + state.modal.show({ trigger: button }); state.cancelButton.focus(); try { @@ -7417,7 +7342,6 @@ function openRowInsertDialog(button, manager) { state.manager = manager; state.mode = "insert"; - state.currentButton = button; state.currentRow = null; state.currentRowId = null; state.currentPkPath = null; @@ -7432,7 +7356,7 @@ function openRowInsertDialog(button, manager) { state.shouldReloadOnClose = false; state.redirectOnCloseUrl = null; resetBulkInsertPreview(state); - state.shouldRestoreFocus = true; + state.hasLoaded = false; state.loadId += 1; @@ -7454,9 +7378,7 @@ function openRowInsertDialog(button, manager) { state.summary.textContent = ""; syncRowEditInsertModeUi(state); - if (!state.dialog.open) { - state.dialog.showModal(); - } + state.modal.show({ trigger: button }); renderRowInsertFields(state, insertData); } diff --git a/tests/test_playwright.py b/tests/test_playwright.py index d7f6fa5c..753691e4 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1761,6 +1761,67 @@ def test_modal_lifecycle(page, datasette_server, shadow): expect(page.locator("#after-save")).to_be_focused() +@pytest.mark.playwright +def test_modal_nested_escape_and_cleanup(page, datasette_server): + from playwright.sync_api import expect + + page.goto(datasette_server + "data/projects") + trigger = page.locator('tr[data-row="1"] button[data-row-action="edit"]') + trigger.click() + dialog = page.locator("#row-edit-dialog") + field = dialog.locator('input[name="title"]') + expect(field).to_be_visible() + field.fill("Unsaved title") + page.evaluate("""() => { + window.confirmations = []; + window.confirm = message => { confirmations.push(message); return false; }; + }""") + # Plugin controls can consume Escape without closing their containing form. + field.evaluate("""node => node.addEventListener('keydown', event => { + if (event.key === 'Escape') event.preventDefault(); + }, {once: true})""") + field.press("Escape") + assert page.evaluate("confirmations") == [] + expect(dialog).to_be_visible() + field.press("Escape") + page.wait_for_function("confirmations.length === 1") + assert page.evaluate("confirmations") == ["Discard unsaved changes to this row?"] + + # A nested native modal closes independently, then returns focus to its field. + field.evaluate("""node => { + node.focus(); + window.nestedModal = DatasetteModal.create(); + nestedModal.dialog.setAttribute('aria-label', 'Nested picker'); + nestedModal.dialog.innerHTML = ''; + node.closest('dialog').append(nestedModal); + nestedModal.show(); + }""") + nested = page.get_by_role("dialog", name="Nested picker") + page.keyboard.press("Escape") + expect(nested).not_to_be_visible() + expect(dialog).to_be_visible() + expect(field).to_be_focused() + assert page.evaluate("confirmations.length") == 1 + + # Closing before keyup cancels the pending confirmation, including on reopen. + page.keyboard.down("Escape") + dialog.locator(".row-edit-cancel").click() + expect(dialog).not_to_be_visible() + trigger.click() + page.keyboard.up("Escape") + expect(field).to_be_visible() + assert page.evaluate("confirmations.length") == 1 + expect(dialog).to_be_visible() + # Native cancel (e.g. an accessibility action) does not wait for keyboard input. + field.fill("Another edit") + dialog.evaluate( + "node => node.dispatchEvent(new Event('cancel', {cancelable: true}))" + ) + assert page.evaluate("confirmations.length") == 2 + dialog.locator(".row-edit-cancel").click() + expect(trigger).to_be_focused() + + @pytest.mark.playwright @pytest.mark.parametrize("name", ["jump", "columns", "type", "mobile"]) def test_modal_consumers_dismiss_and_restore_focus(page, datasette_server, name): From e60d1bfe1c406d58b97a265a2f9d5e89da854d9c Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:10:44 -0700 Subject: [PATCH 099/108] Render navigation search without shadow DOM, refs #2790 --- datasette/static/app.css | 228 ++++++++++++++++++++++ datasette/static/navigation-search.js | 271 +++----------------------- tests/test_playwright.py | 35 ++++ 3 files changed, 286 insertions(+), 248 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 6971635b..f825b273 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -981,6 +981,234 @@ p.zero-results { display: none; } +/* navigation-search */ +navigation-search { + display: contents; +} + +navigation-search dialog.datasette-modal { + max-width: 90vw; + width: 600px; + max-height: 80vh; +} + +navigation-search .search-container { + display: flex; + flex-direction: column; +} + +navigation-search .search-input-wrapper { + padding: 1.25rem; + border-bottom: 1px solid #e5e7eb; + display: flex; + gap: 0.5rem; + align-items: center; +} + +navigation-search .search-input { + width: 100%; + flex: 1; + min-width: 0; + padding: 0.75rem 1rem; + font-size: 1rem; + border: 2px solid #e5e7eb; + border-radius: 0.5rem; + outline: none; + transition: border-color 0.2s; + box-sizing: border-box; +} + +navigation-search .search-input:focus { + border-color: #2563eb; +} + +navigation-search .close-search { + background: transparent; + border: 1px solid transparent; + border-radius: 0.375rem; + color: #4b5563; + cursor: pointer; + flex: 0 0 auto; + font: inherit; + font-size: 1.5rem; + height: 2.75rem; + line-height: 1; + width: 2.75rem; +} + +navigation-search .close-search:hover, +navigation-search .close-search:focus { + background-color: #f3f4f6; + border-color: #d1d5db; +} + +navigation-search .results-container { + box-sizing: content-box; + overflow-y: auto; + height: calc(80vh - 180px); + padding: 0.5rem; +} + +navigation-search .results-list:empty { + display: none; +} + +navigation-search .result-item { + padding: 0.875rem 1rem; + cursor: pointer; + border-radius: 0.5rem; + transition: background-color 0.15s; + display: flex; + align-items: center; + gap: 0.75rem; +} + +navigation-search .result-item:hover { + background-color: #f3f4f6; +} + +navigation-search .result-item.selected { + background-color: #dbeafe; +} + +navigation-search .result-item > div { + flex: 1; + min-width: 0; +} + +navigation-search .jump-start-content { + border-bottom: 1px solid #e5e7eb; + margin-bottom: 0.5rem; + padding: 0.5rem 0.5rem 1rem; +} + +navigation-search .jump-start-content:empty { + display: none; +} + +navigation-search .result-name { + font-weight: 500; + color: #111827; +} + +navigation-search .result-label { + font-size: 0.875rem; + color: #4b5563; +} + +navigation-search .result-type { + color: #4b5563; + font-size: 0.75rem; + font-weight: 600; + text-transform: uppercase; +} + +navigation-search .result-url { + font-size: 0.875rem; + color: #6b7280; +} + +navigation-search .result-description { + color: #374151; + display: -webkit-box; + font-size: 0.8125rem; + line-height: 1.35; + margin-top: 0.35rem; + overflow: hidden; + -webkit-box-orient: vertical; + -webkit-line-clamp: 2; +} + +navigation-search .results-heading { + color: #4b5563; + font-size: 0.75rem; + font-weight: 600; + letter-spacing: 0; + padding: 0.5rem 1rem 0.25rem; + text-transform: uppercase; +} + +navigation-search .recent-actions { + padding: 0.25rem 1rem 0.75rem; +} + +navigation-search .clear-recent { + background: transparent; + border: 0; + color: #2563eb; + cursor: pointer; + font: inherit; + font-size: 0.875rem; + padding: 0; +} + +navigation-search .clear-recent:hover { + text-decoration: underline; +} + +navigation-search .no-results { + padding: 2rem; + text-align: center; + color: #6b7280; +} + +navigation-search .hint-text { + padding: 0.75rem 1.25rem; + font-size: 0.875rem; + color: #6b7280; + border-top: 1px solid #e5e7eb; + display: flex; + gap: 1rem; + flex-wrap: wrap; +} + +navigation-search .hint-text kbd { + background: #f3f4f6; + padding: 0.125rem 0.375rem; + border-radius: 0.25rem; + font-size: 0.75rem; + border: 1px solid #d1d5db; + font-family: monospace; +} + +navigation-search .visually-hidden { + border: 0; + clip: rect(0 0 0 0); + height: 1px; + margin: -1px; + overflow: hidden; + padding: 0; + position: absolute; + white-space: nowrap; + width: 1px; +} + +@media (max-width: 640px) { + navigation-search dialog.datasette-modal { + width: 95vw; + max-height: 85vh; + border-radius: 0.5rem; + } + + navigation-search .search-input-wrapper { + padding: 1rem; + } + + navigation-search .search-input { + font-size: 16px; + } + + navigation-search .result-item { + padding: 1rem 0.75rem; + } + + navigation-search .hint-text { + font-size: 0.8rem; + padding: 0.5rem 1rem; + } +} + + dialog.mobile-column-actions-dialog { width: min(420px, calc(100vw - 32px)); max-height: min(640px, calc(100vh - 32px)); diff --git a/datasette/static/navigation-search.js b/datasette/static/navigation-search.js index 02136466..2f9b723c 100644 --- a/datasette/static/navigation-search.js +++ b/datasette/static/navigation-search.js @@ -10,247 +10,22 @@ class NavigationSearch extends HTMLElement { this.recentHeadingId = `navigation-search-recent-${this.instanceId}`; this.statusId = `navigation-search-status-${this.instanceId}`; this.titleId = `navigation-search-title-${this.instanceId}`; - this.attachShadow({ mode: "open" }); this.selectedIndex = -1; this.matches = []; this.renderedMatches = []; this.debounceTimer = null; + } + connectedCallback() { + if (this._initialized) return; + this._initialized = true; this.render(); this.setupEventListeners(); } render() { - this.shadowRoot.innerHTML = ` - - - + this.innerHTML = ` +
@@ -284,11 +59,10 @@ class NavigationSearch extends HTMLElement { } setupEventListeners() { - const dialog = this.shadowRoot.querySelector("dialog"); - const input = this.shadowRoot.querySelector(".search-input"); - const closeButton = this.shadowRoot.querySelector(".close-search"); - const resultsContainer = - this.shadowRoot.querySelector(".results-container"); + const dialog = this.querySelector("dialog"); + const input = this.querySelector(".search-input"); + const closeButton = this.querySelector(".close-search"); + const resultsContainer = this.querySelector(".results-container"); // Global keyboard listener for "/" document.addEventListener("keydown", (e) => { @@ -408,8 +182,8 @@ class NavigationSearch extends HTMLElement { } updateComboboxState() { - const dialog = this.shadowRoot.querySelector("dialog"); - const input = this.shadowRoot.querySelector(".search-input"); + const dialog = this.querySelector("dialog"); + const input = this.querySelector(".search-input"); const matches = this.renderedMatches || []; this.setElementAttribute( input, @@ -434,7 +208,7 @@ class NavigationSearch extends HTMLElement { } setStatus(message) { - const status = this.shadowRoot.querySelector(`#${this.statusId}`); + const status = this.querySelector(`#${this.statusId}`); if (status) { status.textContent = message || ""; } @@ -644,7 +418,7 @@ class NavigationSearch extends HTMLElement { section.render(node, { navigationSearch: this, container, - input: this.shadowRoot.querySelector(".search-input"), + input: this.querySelector(".search-input"), }); }); } @@ -683,8 +457,8 @@ class NavigationSearch extends HTMLElement { } renderResults() { - const container = this.shadowRoot.querySelector(".results-container"); - const input = this.shadowRoot.querySelector(".search-input"); + const container = this.querySelector(".results-container"); + const input = this.querySelector(".search-input"); const showStartContent = !input.value.trim(); const jumpSections = showStartContent ? this.jumpSections() : []; const startBlock = showStartContent @@ -797,11 +571,12 @@ class NavigationSearch extends HTMLElement { } openMenu(trigger) { - const input = this.shadowRoot.querySelector(".search-input"); + const input = this.querySelector(".search-input"); - this.shadowRoot - .querySelector("datasette-modal") - .show({ trigger, initialFocus: input }); + this.querySelector("datasette-modal").show({ + trigger, + initialFocus: input, + }); this.setNavigationTriggersExpanded(true); input.value = ""; @@ -813,11 +588,11 @@ class NavigationSearch extends HTMLElement { } closeMenu(options = {}) { - this.shadowRoot.querySelector("datasette-modal").close(options); + this.querySelector("datasette-modal").close(options); } onMenuClosed() { - const input = this.shadowRoot.querySelector(".search-input"); + const input = this.querySelector(".search-input"); this.setElementAttribute(input, "aria-expanded", "false"); this.removeElementAttribute(input, "aria-activedescendant"); this.setNavigationTriggersExpanded(false); diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 753691e4..890ba076 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1083,6 +1083,41 @@ def test_navigation_search_renders_jump_sections_from_javascript_plugins( page.wait_for_url("**/-/playwright-agent") +@pytest.mark.playwright +def test_navigation_search_created_from_javascript(page, datasette_server): + from playwright.sync_api import expect + + page.goto(datasette_server) + page.evaluate("""() => { + const search = document.createElement('navigation-search'); + search.id = 'additional-search'; + search.setAttribute('items', JSON.stringify([ + {name: 'Projects', url: '/data/projects'} + ])); + document.body.append(search); + const unrelated = document.createElement('div'); + unrelated.className = 'search-container'; + unrelated.id = 'outside-search'; + document.body.append(unrelated); + search.openMenu(); + }""") + search = page.locator("#additional-search") + dialog = search.get_by_role("dialog", name="Jump to", exact=True) + expect(dialog).to_be_visible() + # Page styles and ordinary DOM queries can reach the component's controls. + page.add_style_tag( + content="#additional-search .search-input { border-top-color: rgb(1, 2, 3); }" + ) + field = dialog.get_by_role("combobox", name="Jump to", exact=True) + expect(field).to_have_css("border-top-color", "rgb(1, 2, 3)") + assert field.evaluate("node => document.getElementById(node.id) === node") + expect(page.locator("#outside-search")).to_have_css("display", "block") + field.fill("projects") + expect(dialog.get_by_role("option")).to_contain_text("Projects") + field.press("Enter") + page.wait_for_url("**/data/projects") + + @pytest.mark.playwright def test_insert_row_flow_uses_custom_column_field(page, datasette_server): page.add_init_script(""" From 15d511e2da61a8ad22aec280279129ceccdfbc65 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:10:45 -0700 Subject: [PATCH 100/108] Render the column chooser without shadow DOM, refs #2790 --- datasette/static/app.css | 306 +++++++++++++++++++++++++++++ datasette/static/column-chooser.js | 290 +++------------------------ tests/test_playwright.py | 51 +++++ 3 files changed, 389 insertions(+), 258 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index f825b273..4a4b0d5c 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1209,6 +1209,312 @@ navigation-search .visually-hidden { } +/* column-chooser */ +column-chooser { + display: contents; + --ink: #0f0f0f; + --paper: #eef6ff; + --muted: #6b6b6b; + --rule: #d8e6f5; + --accent: #1a56db; + --accent-light: #e8effd; + --card: #ffffff; +} + +column-chooser * { + box-sizing: border-box; + margin: 0; + padding: 0; +} + +column-chooser dialog.datasette-modal { + width: 100%; + max-width: 420px; + max-height: min(640px, calc(100vh - 32px)); + -webkit-user-select: none; + -webkit-touch-callout: none; + -webkit-tap-highlight-color: transparent; +} + +column-chooser dialog.datasette-modal[open] { + height: min(640px, calc(100vh - 32px)); +} + +column-chooser .modal-header { + padding: 20px 24px 16px; + justify-content: space-between; +} + +column-chooser .list-toolbar { + padding: 6px 24px; + border-bottom: 1px solid var(--rule); + display: flex; + gap: 12px; + flex-shrink: 0; +} + +column-chooser .list-toolbar button { + background: var(--accent-light); + border: 1px solid var(--rule); + border-radius: 4px; + font-family: inherit; + font-size: 0.75rem; + color: var(--accent); + cursor: pointer; + padding: 3px 10px; + transition: + background 0.12s, + color 0.12s; +} + +column-chooser .list-toolbar button:hover { + background: var(--accent); + color: white; +} + +column-chooser .list-wrap { + flex: 1; + overflow-y: auto; + overflow-x: hidden; + position: relative; + overscroll-behavior: contain; + -webkit-overflow-scrolling: touch; +} + +column-chooser .list-wrap::before, +column-chooser .list-wrap::after { + content: ""; + position: sticky; + display: block; + left: 0; + right: 0; + height: 20px; + pointer-events: none; + z-index: 5; + transition: opacity 0.2s; +} + +column-chooser .list-wrap::before { + top: 0; + background: linear-gradient( + to bottom, + rgba(255, 255, 255, 0.9), + transparent + ); +} + +column-chooser .list-wrap::after { + bottom: 0; + background: linear-gradient(to top, rgba(255, 255, 255, 0.9), transparent); + margin-top: -20px; +} + +column-chooser .scroll-zone { + position: absolute; + left: 0; + right: 0; + height: 72px; + pointer-events: none; + z-index: 10; +} + +column-chooser .scroll-zone-top { + top: 0; +} + +column-chooser .scroll-zone-bot { + bottom: 0; +} + +column-chooser .drag-list { + list-style: none; + padding: 4px 0; +} + +column-chooser .drag-item { + display: flex; + align-items: center; + background: white; + border-bottom: 1px solid var(--rule); + user-select: none; + -webkit-user-select: none; + -webkit-touch-callout: none; + position: relative; + transition: background 0.08s; +} + +column-chooser .drag-item:last-child { + border-bottom: none; +} + +column-chooser .drag-handle { + display: flex; + align-items: center; + justify-content: center; + width: 48px; + height: 48px; + flex-shrink: 0; + cursor: grab; + color: #c8c4bc; + touch-action: none; + transition: color 0.15s; +} + +column-chooser .drag-handle:hover { + color: var(--accent); +} + +column-chooser .drag-handle svg { + pointer-events: none; + display: block; +} + +column-chooser .drag-item-content { + display: flex; + align-items: center; + flex: 1; + min-width: 0; + cursor: pointer; +} + +column-chooser .drag-item-check { + display: flex; + align-items: center; + width: 32px; + height: 48px; + flex-shrink: 0; +} + +column-chooser .drag-item-check input[type="checkbox"] { + width: 16px; + height: 16px; + accent-color: var(--accent); + cursor: pointer; +} + +column-chooser .drag-item-label { + flex: 1; + font-size: 0.9rem; + line-height: 48px; + padding-right: 16px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + cursor: default; +} + +column-chooser .drag-item.is-dragging { + opacity: 0; +} + +column-chooser .drop-indicator { + position: absolute; + left: 48px; + right: 0; + height: 2px; + background: var(--accent); + border-radius: 99px; + pointer-events: none; + z-index: 20; + display: none; +} + +column-chooser .drop-indicator.top { + top: -1px; + display: block; +} + +column-chooser .drop-indicator.bottom { + bottom: -1px; + display: block; +} + +column-chooser .drag-ghost { + position: fixed; + pointer-events: none; + z-index: 9999; + background: white; + border-radius: 6px; + box-shadow: + 0 8px 32px rgba(0, 0, 0, 0.18), + 0 2px 8px rgba(0, 0, 0, 0.1); + display: flex; + align-items: center; + border: 1.5px solid var(--accent-light); + opacity: 0.97; + will-change: transform; + font-family: + system-ui, + -apple-system, + sans-serif; +} + +column-chooser .scroll-pulse { + position: absolute; + left: 50%; + transform: translateX(-50%); + width: 32px; + height: 32px; + border-radius: 50%; + background: var(--accent); + opacity: 0; + pointer-events: none; + z-index: 10; + transition: opacity 0.15s; +} + +column-chooser .scroll-pulse.top { + top: 8px; +} + +column-chooser .scroll-pulse.bot { + bottom: 8px; +} + +column-chooser .scroll-pulse.active { + opacity: 0.18; + animation: column-chooser-pulse 0.8s ease-in-out infinite; +} + +@keyframes column-chooser-pulse { + 0%, + 100% { + transform: translateX(-50%) scale(1); + opacity: 0.18; + } + 50% { + transform: translateX(-50%) scale(1.5); + opacity: 0.07; + } +} + +column-chooser .modal-btn-primary { + color: white; +} + +column-chooser .modal-btn-primary:hover { + background: #1448c0; +} + +column-chooser .list-wrap::-webkit-scrollbar { + width: 5px; +} + +column-chooser .list-wrap::-webkit-scrollbar-track { + background: transparent; +} + +column-chooser .list-wrap::-webkit-scrollbar-thumb { + background: var(--rule); + border-radius: 99px; +} + +column-chooser input, +column-chooser textarea { + -webkit-user-select: auto; + user-select: auto; +} + dialog.mobile-column-actions-dialog { width: min(420px, calc(100vw - 32px)); max-height: min(640px, calc(100vh - 32px)); diff --git a/datasette/static/column-chooser.js b/datasette/static/column-chooser.js index f0fac0ec..c1d25dfa 100644 --- a/datasette/static/column-chooser.js +++ b/datasette/static/column-chooser.js @@ -1,7 +1,9 @@ +let columnChooserInstanceCounter = 0; + class ColumnChooser extends HTMLElement { constructor() { super(); - this.attachShadow({ mode: "open" }); + this.titleId = `column-chooser-title-${++columnChooserInstanceCounter}`; // State this._items = []; @@ -26,273 +28,45 @@ class ColumnChooser extends HTMLElement { // Bound handlers this._onMove = this._onMove.bind(this); this._onUp = this._onUp.bind(this); + } - this.shadowRoot.innerHTML = ` - - - + connectedCallback() { + if (this._modal) return; + this.innerHTML = ` +
- Choose columns - + Choose columns +
- - + +
-
-
-
-
    +
    +
    +
    +
      - - - + + +
      `; // DOM refs - this._modal = this.shadowRoot.querySelector("datasette-modal"); - this._listWrap = this.shadowRoot.getElementById("listWrap"); - this._dragList = this.shadowRoot.getElementById("dragList"); - this._pulseTop = this.shadowRoot.getElementById("pulseTop"); - this._pulseBot = this.shadowRoot.getElementById("pulseBot"); - this._selectAllBtn = this.shadowRoot.getElementById("selectAllBtn"); - this._deselectAllBtn = this.shadowRoot.getElementById("deselectAllBtn"); - this._cancelBtn = this.shadowRoot.getElementById("cancelBtn"); - this._applyBtn = this.shadowRoot.getElementById("applyBtn"); - this._countEl = this.shadowRoot.getElementById("selectedCount"); - this._footerEl = this.shadowRoot.getElementById("footerInfo"); + this._modal = this.querySelector("datasette-modal"); + this._listWrap = this.querySelector(".list-wrap"); + this._dragList = this.querySelector(".drag-list"); + this._pulseTop = this.querySelector(".scroll-pulse.top"); + this._pulseBot = this.querySelector(".scroll-pulse.bot"); + this._selectAllBtn = this.querySelector(".select-all"); + this._deselectAllBtn = this.querySelector(".deselect-all"); + this._cancelBtn = this.querySelector(".modal-btn-ghost"); + this._applyBtn = this.querySelector(".modal-btn-primary"); + this._countEl = this.querySelector(".modal-meta"); + this._footerEl = this.querySelector(".footer-info"); // Event listeners this._selectAllBtn.addEventListener("click", () => this._selectAll()); @@ -416,7 +190,7 @@ class ColumnChooser extends HTMLElement { this._ghostOffX = e.clientX - rect.left; this._ghostOffY = e.clientY - rect.top; - // Build ghost inside shadow DOM + // Keep the drag preview inside the dialog so it stays above the backdrop. this._ghost = document.createElement("div"); this._ghost.className = "drag-ghost"; this._ghost.style.width = rect.width + "px"; @@ -425,7 +199,7 @@ class ColumnChooser extends HTMLElement { this._ghost.querySelector(".drop-indicator")?.remove(); const h = this._ghost.querySelector(".drag-handle"); if (h) h.style.color = "var(--accent)"; - this.shadowRoot.appendChild(this._ghost); + this._modal.dialog.appendChild(this._ghost); srcEl.classList.add("is-dragging"); this._positionGhost(e.clientX, e.clientY); diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 890ba076..5404d94f 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1118,6 +1118,57 @@ def test_navigation_search_created_from_javascript(page, datasette_server): page.wait_for_url("**/data/projects") +@pytest.mark.playwright +def test_column_chooser_selection_and_drag_in_document(page, datasette_server): + from playwright.sync_api import expect + + page.goto(datasette_server + "data/projects") + page.emulate_media(reduced_motion="reduce") + page.evaluate("""() => { + const chooser = document.createElement('column-chooser'); + chooser.id = 'additional-chooser'; + document.body.append(chooser); + window.appliedColumns = null; + chooser.open({ + columns: ['title', 'notes', 'score'], + selected: ['title', 'notes'], + onApply: columns => { window.appliedColumns = columns; } + }); + }""") + chooser = page.locator("#additional-chooser") + dialog = chooser.get_by_role("dialog", name="Choose columns") + expect(dialog).to_be_visible() + assert dialog.evaluate("""node => { + const id = node.getAttribute('aria-labelledby'); + return document.querySelectorAll(`#${id}`).length === 1 && + node.contains(document.getElementById(id)); + }""") + expect(dialog.locator(".modal-meta")).to_have_text("2 of 3 selected") + dialog.get_by_role("button", name="Deselect all", exact=True).click() + expect(dialog.locator(".modal-meta")).to_have_text("0 of 3 selected") + dialog.get_by_role("button", name="Select all", exact=True).click() + expect(dialog.locator(".modal-meta")).to_have_text("3 of 3 selected") + # Move title after score using the same pointer events as mouse/touch dragging. + handle = dialog.locator(".drag-handle").first.bounding_box() + target = dialog.locator(".drag-item").last.bounding_box() + page.mouse.move(handle["x"] + handle["width"] / 2, handle["y"] + 24) + page.mouse.down() + page.mouse.move(target["x"] + 24, target["y"] + target["height"] - 4, steps=5) + expect(dialog.locator(".drag-ghost")).to_be_visible() + page.mouse.up() + expect(dialog.locator(".drag-item-label")).to_have_text(["notes", "score", "title"]) + dialog.get_by_role("button", name="Apply", exact=True).click() + expect(dialog).not_to_be_visible() + assert page.evaluate("appliedColumns") == ["notes", "score", "title"] + chooser.evaluate( + "node => node.open({columns: ['title', 'notes'], selected: ['title']})" + ) + dialog.get_by_role("button", name="Deselect all", exact=True).click() + dialog.get_by_role("button", name="Cancel", exact=True).click() + expect(dialog).not_to_be_visible() + assert page.evaluate("appliedColumns") == ["notes", "score", "title"] + + @pytest.mark.playwright def test_insert_row_flow_uses_custom_column_field(page, datasette_server): page.add_init_script(""" From 71600f1c0a0738ecbf8013376fc5789180a75da7 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:10:45 -0700 Subject: [PATCH 101/108] Simplify shared modals now that all dialogs use the document, refs #2790 --- datasette/static/app.css | 2 +- datasette/static/modal.css | 2 +- datasette/static/modal.js | 28 +++------------------------- datasette/templates/base.html | 2 +- docs/contributing.rst | 4 ++-- docs/javascript_plugins.rst | 4 ++-- tests/test_playwright.py | 14 +++++--------- 7 files changed, 15 insertions(+), 41 deletions(-) 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() From 8220413a8a6c6bed7a7bef66fc49e76277cea60d Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:13:17 -0700 Subject: [PATCH 102/108] Keep modal documentation in the JavaScript plugin docs, refs #2790 --- docs/contributing.rst | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 57643f64..692f94c8 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -132,19 +132,6 @@ If you are not using ``just``, the equivalent ``uv run`` commands are: uv run --group playwright playwright install chromium uv run --group playwright pytest tests/test_playwright.py --playwright --browser chromium -.. _contributing_modals: - -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``. 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 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: Using fixtures From 2474c45f10c0d4b83ac17a7cb757522f40859223 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:17:35 -0700 Subject: [PATCH 103/108] Move shared modal styles into app.css, refs #2790 --- datasette/static/app.css | 135 ++++++++++++++++++++++++++++++++++ datasette/static/modal.css | 134 --------------------------------- datasette/templates/base.html | 1 - 3 files changed, 135 insertions(+), 135 deletions(-) delete mode 100644 datasette/static/modal.css diff --git a/datasette/static/app.css b/datasette/static/app.css index 3b0546e3..f04e6356 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1,3 +1,138 @@ +/* Shared modal styles. */ +datasette-modal { + display: contents; +} + +dialog.datasette-modal { + --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(520px, calc(100vw - 32px)); + max-width: 95vw; + max-height: calc(100dvh - 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.datasette-modal[open] { + display: flex; + flex-direction: column; +} + +dialog.datasette-modal::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; +} + +@keyframes datasette-modal-slide-in { + from { opacity: 0; transform: translateY(-20px) scale(0.95); } + to { opacity: 1; transform: translateY(0) scale(1); } +} + +@keyframes datasette-modal-fade-in { + from { opacity: 0; } + to { opacity: 1; } +} + +:where(.datasette-modal) .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; +} + +:where(.datasette-modal) .modal-title { + font-size: 1rem; + font-weight: 600; + color: var(--ink); +} + +:where(.datasette-modal) .modal-meta { + font-family: ui-monospace, monospace; + font-size: 0.7rem; + color: var(--muted); + background: var(--paper); + padding: 3px 9px; + border-radius: 20px; +} + +:where(.datasette-modal) .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); +} + +:where(.datasette-modal) .footer-info { + flex: 1; + font-family: ui-monospace, monospace; + font-size: 0.68rem; + color: var(--muted); +} + +:where(.datasette-modal) .modal-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; +} + +:where(.datasette-modal) .modal-btn-ghost { + background: transparent; + color: var(--muted); + border: 1px solid var(--rule); +} + +:where(.datasette-modal) .modal-btn-ghost:hover { + background: var(--rule); + color: var(--ink); +} + +:where(.datasette-modal) .modal-btn-primary { + background: var(--accent); + color: #fff; +} + +:where(.datasette-modal) .modal-btn-primary:hover { + background: #1949b8; +} + +:where(.datasette-modal) .modal-btn:disabled { + opacity: 0.65; + cursor: wait; +} + +@media (prefers-reduced-motion: reduce) { + dialog.datasette-modal, + dialog.datasette-modal::backdrop { + animation: none; + } +} + /* Reset and Page Setup ==================================================== */ /* Reset from http://meyerweb.com/eric/tools/css/reset/ diff --git a/datasette/static/modal.css b/datasette/static/modal.css deleted file mode 100644 index aeec561f..00000000 --- a/datasette/static/modal.css +++ /dev/null @@ -1,134 +0,0 @@ -/* Shared modal styles. */ -datasette-modal { - display: contents; -} - -dialog.datasette-modal { - --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(520px, calc(100vw - 32px)); - max-width: 95vw; - max-height: calc(100dvh - 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.datasette-modal[open] { - display: flex; - flex-direction: column; -} - -dialog.datasette-modal::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; -} - -@keyframes datasette-modal-slide-in { - from { opacity: 0; transform: translateY(-20px) scale(0.95); } - to { opacity: 1; transform: translateY(0) scale(1); } -} - -@keyframes datasette-modal-fade-in { - from { opacity: 0; } - to { opacity: 1; } -} - -:where(.datasette-modal) .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; -} - -:where(.datasette-modal) .modal-title { - font-size: 1rem; - font-weight: 600; - color: var(--ink); -} - -:where(.datasette-modal) .modal-meta { - font-family: ui-monospace, monospace; - font-size: 0.7rem; - color: var(--muted); - background: var(--paper); - padding: 3px 9px; - border-radius: 20px; -} - -:where(.datasette-modal) .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); -} - -:where(.datasette-modal) .footer-info { - flex: 1; - font-family: ui-monospace, monospace; - font-size: 0.68rem; - color: var(--muted); -} - -:where(.datasette-modal) .modal-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; -} - -:where(.datasette-modal) .modal-btn-ghost { - background: transparent; - color: var(--muted); - border: 1px solid var(--rule); -} - -:where(.datasette-modal) .modal-btn-ghost:hover { - background: var(--rule); - color: var(--ink); -} - -:where(.datasette-modal) .modal-btn-primary { - background: var(--accent); - color: #fff; -} - -:where(.datasette-modal) .modal-btn-primary:hover { - background: #1949b8; -} - -:where(.datasette-modal) .modal-btn:disabled { - opacity: 0.65; - cursor: wait; -} - -@media (prefers-reduced-motion: reduce) { - dialog.datasette-modal, - dialog.datasette-modal::backdrop { - animation: none; - } -} diff --git a/datasette/templates/base.html b/datasette/templates/base.html index b11d14f5..e5aa46f3 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -2,7 +2,6 @@ {% block title %}{% endblock %} - {% for url in extra_css_urls %} From 0ad118ba267f0ca630f1284544931900b3651037 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:18:22 -0700 Subject: [PATCH 104/108] Clarify focus restoration when reopening a modal, refs #2790 --- docs/javascript_plugins.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/javascript_plugins.rst b/docs/javascript_plugins.rst index 7a21b5a9..10077296 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. ``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()`` 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")`` 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. From 269c043da30d8bcf4202fd8ec4e4639861b98e22 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 17 Sep 2026 14:42:14 -0700 Subject: [PATCH 105/108] Share scrolling dialog body styles with modal-body, refs #2790 --- datasette/static/app.css | 24 ++++++++--------------- datasette/static/column-chooser.js | 2 +- datasette/static/edit-tools.js | 10 +++++----- datasette/static/mobile-column-actions.js | 2 +- datasette/static/navigation-search.js | 2 +- datasette/static/table.js | 2 +- docs/javascript_plugins.rst | 7 +++++-- 7 files changed, 22 insertions(+), 27 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index f04e6356..0297371c 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -71,6 +71,12 @@ dialog.datasette-modal::backdrop { border-radius: 20px; } +:where(.datasette-modal) .modal-body { + min-height: 0; + overflow: auto; + padding: 16px 24px 24px; +} + :where(.datasette-modal) .modal-footer { padding: 14px 20px; border-top: 1px solid var(--rule); @@ -1179,7 +1185,6 @@ navigation-search .close-search:focus { navigation-search .results-container { box-sizing: content-box; - overflow-y: auto; height: calc(80vh - 180px); padding: 0.5rem; } @@ -1409,7 +1414,7 @@ column-chooser .list-toolbar button:hover { column-chooser .list-wrap { flex: 1; - overflow-y: auto; + padding: 0; overflow-x: hidden; position: relative; overscroll-behavior: contain; @@ -1662,8 +1667,7 @@ dialog.mobile-column-actions-dialog { .mobile-column-actions-dialog .list-wrap { flex: 1 1 auto; - min-height: 0; - overflow-y: auto; + padding: 0; overflow-x: hidden; position: relative; overscroll-behavior: contain; @@ -1817,8 +1821,6 @@ dialog.set-column-type-dialog { } .set-column-type-options { - padding: 16px 24px 24px; - overflow-y: auto; display: grid; gap: 12px; } @@ -2018,8 +2020,6 @@ dialog.row-edit-dialog { .row-edit-fields { display: grid; gap: 14px; - padding: 16px 24px 24px; - overflow-y: auto; } .row-edit-fields[hidden], @@ -2299,8 +2299,6 @@ textarea.row-edit-input { .row-edit-bulk { display: grid; gap: 8px; - padding: 16px 24px 24px; - overflow-y: auto; } .row-edit-bulk-editor { @@ -2616,8 +2614,6 @@ dialog.table-create-dialog { .table-create-fields { display: grid; gap: 18px; - padding: 16px 24px 24px; - overflow-y: auto; } .table-create-field { @@ -3081,8 +3077,6 @@ dialog.table-alter-dialog { .table-alter-fields { display: grid; gap: 18px; - padding: 16px 24px 24px; - overflow-y: auto; } .table-alter-table-options { @@ -3116,8 +3110,6 @@ dialog.table-alter-dialog { .table-alter-review { display: grid; gap: 12px; - overflow-y: auto; - padding: 16px 24px 24px; } .table-alter-review[hidden] { diff --git a/datasette/static/column-chooser.js b/datasette/static/column-chooser.js index c1d25dfa..29729f27 100644 --- a/datasette/static/column-chooser.js +++ b/datasette/static/column-chooser.js @@ -42,7 +42,7 @@ class ColumnChooser extends HTMLElement {
      -
      +
        diff --git a/datasette/static/edit-tools.js b/datasette/static/edit-tools.js index edb00c7f..2eeb6b21 100644 --- a/datasette/static/edit-tools.js +++ b/datasette/static/edit-tools.js @@ -2159,7 +2159,7 @@ function ensureTableCreateDialog(manager) {
        -
        +
        @@ -3909,7 +3909,7 @@ function ensureTableAlterDialog(manager) {
        -
        +
        - +
        @@ -6984,8 +6984,8 @@ function ensureRowEditDialog(manager) {

        Loading row...

        -
        -