docs(modals): plan errata — 3 verbatim-code defects + 3 inserted tasks

The URL-addressable modals plan shipped in 14 commits. Three places
where the plan's verbatim code didn't survive contact with the codebase
(has_request_context guard, LEFT4ME_ROOT-aware fixture, save-handler
direct-bind) are now documented at the top of the plan, with commit
references for the fixes. Also notes the inserted tasks 8.5/8.5b/9b
and the Task 6 design refinement (close-event single state sink) so a
future re-executor sees the actual shipped pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
mwiegand 2026-05-17 14:03:10 +02:00
parent 55c7856eb1
commit 712ccc9861
No known key found for this signature in database

View file

@ -12,6 +12,35 @@
--- ---
## Errata (post-execution)
The plan shipped via 14 commits between 2026-05-17 and the same day's evening. Three defects in the verbatim plan code were caught by code review during execution; if you re-run this plan, watch for them:
1. **Task 1, Step 4 — context processor needs a `has_request_context()` guard.** Plan code reads `request.headers.get("HX-Modal")` unconditionally, but `tests/test_timeago.py` renders templates inside `app.app_context()` only (no request context). Without the guard the processor crashes with `RuntimeError: Working outside of request context`. Fix: `is_modal = has_request_context() and request.headers.get("HX-Modal") == "1"` (lazy import `from flask import has_request_context` is fine). Shipped in commit `82c3f04`.
2. **Task 3, Step 1 — test fixture must respect `LEFT4ME_ROOT`.** Plan code uses `path=str(overlay_root)` (absolute filesystem path) on the `Overlay` model. The codebase resolves `overlay.path` relative to `LEFT4ME_ROOT` via `validate_overlay_ref` and rejects absolute paths. Fix: `monkeypatch.setenv("LEFT4ME_ROOT", str(tmp_path))`, write files to `tmp_path/overlays/<id>/`, set `overlay.path = str(overlay.id)`. Mirrors `tests/test_overlay_files_routes.py`'s convention. Shipped in commit `60e7968`.
3. **Task 9, Step 2 — "save flow unchanged" was wrong.** The legacy save/delete handlers in `files-overlay.js` are direct-bound to `editorEls.saveBtn` / `editorEls.deleteBtn` (the inline dialog's specific elements), not document-delegated. The new server-rendered modal's identical-class buttons get no handler. Fix: add document-level event delegation for `.files-editor-save` and `.files-editor-delete` clicks gated on `modalContent.contains(btn)`, read `data-rel-path` from the textarea (NOT from a JS var the now-deleted open path used to set), use `window.__filesEditor.getValue()`, POST + `closeModal()` + `scheduleRefresh(parentOf(path))`. Also support rename: read filename input, compose `payload.new_path = parent/filename` when changed, handle 409 with alert + keep modal open. Shipped across commits `64cf203` and `33a2e52`.
## Tasks added during execution
Three tasks were inserted that weren't in the original plan:
- **Task 8.5 (commit `f6b8ecf`)**`overlay_file_editor.html`'s `<dialog open>` nested inside `<dialog id="modal-container">` collapses to 2 px tall in browsers. Replaced with `<div role="document">`. Bundled with CM6 `controller.destroy()` on modal close (memory leak fix — every open/close cycle had been orphaning an `EditorView` and a `matchMedia` listener) and a `mountOne` idempotency guard. CSS broadened: `dialog.modal, div.modal`.
- **Task 8.5b (commit `7829d1c`)** — the broadened CSS caused double-card painting (outer dialog + inner div both matched the `.modal` styling). Dropped `class="modal modal-wide"` and `role="document"` from the inner div; the outer dialog owns the chrome.
- **Task 9b (commit `33a2e52`)** — see defect #3 above for rename-on-save support.
## Design refinement during execution (Task 6 superseded)
Task 6's original "every close source updates state directly" code was replaced with a close-event-centric design: every close source (Esc cancel, backdrop click, `[data-modal-dismiss]`, browser back, `htmx:responseError`, programmatic close) just calls `dialog.close()`, and a single `close`-event listener clears `currentModalPath` and removes `?modal=` from the URL. This kills two latent bugs simultaneously: (a) the legacy `modal.js:31-33` backdrop handler closes `dialog.modal` without clearing URL, and (b) HTMX's `htmx.ajax` resolves on 4xx so plain `.then(() => showModal())` would open a modal on error responses. Shipped in commit `6e66375`. The revised design is in that commit's diff.
## Post-pilot polish (commits 5dc4xx after Task 10)
- Removed dangling `aria-labelledby="modal-content-title"` from `#modal-container` in `base.html` (referenced an id that never existed).
- Renamed the new editor template's outer `<div>` id from `files-editor-modal` to `files-editor-fragment` to resolve a duplicate-id W3C violation with the legacy inline `<dialog id="files-editor-modal">` in `overlay_detail.html`. Updated `editor.js`'s `closest()` to match both selectors so auto-language detection works for both modal pipelines.
---
## File Structure ## File Structure
| Path | New / Modify | Responsibility | | Path | New / Modify | Responsibility |