left4me/docs/superpowers/plans/2026-05-17-files-overlay-rewrite.md
mwiegand d76ee05956
docs(files): errata — script tag lives in overlay_detail.html, not base.html
files-overlay.js is loaded from overlay_detail.html:285 (with defer),
not base.html — the JS activates only when .files-manager exists,
which is only on overlay detail for files-type overlays. Loading from
base.html would pull it onto every page. The plan's first draft had
this wrong in four places (step 1, step 4, step 10, critical files
table). Following the plan verbatim would have moved the script tag
to the wrong template — exactly the failure mode that
feedback_validate_before_implementing memory warns about.

Added an Errata section at the bottom of the plan documenting this.
Also clarified that all new module script tags should use defer to
match the existing pattern (the modules query the DOM at load and
need the body parsed first).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 15:02:21 +02:00

271 lines
23 KiB
Markdown

# files-overlay.js Rewrite + Editor Migration + API Cleanup (handoff-ready)
## Context
`l4d2web/l4d2web/static/js/files-overlay.js` has grown to ~35 KB / 1092 lines and hosts 9+ feature areas in one IIFE: helpers, the legacy inline editor dialog (text + binary + create-new modes), new-folder modal, conflict modal, delete-confirm modal, file-row action dispatch, drag-drop on the file tree, upload queue + progress panel, and document-level delegation for the URL-addressable editor's save/delete. Event-binding patterns are inconsistent — direct-binds, clone-and-rebind anti-patterns, bind-and-remove per-call, and document-level delegation all coexist for similar tasks.
Two flows still use the legacy inline `<dialog id="files-editor-modal">`: **create-new-file** (no URL to deep-link a file that doesn't exist yet) and **binary-replace** (the new URL-addressable template intentionally omitted binary-replace UI per Task 2's pilot scope cut). Migrating both flows to URL-addressable lets us delete the legacy dialog and simplifies the JS editor module dramatically.
Meanwhile `routes/files_routes.py` (640+ lines) duplicates path-resolution and editability checks between `overlay_file_content` (JSON) and `overlay_file_edit_page` (HTML), and the JSON `/files/content` endpoint becomes dead code once create-new + binary-replace migrate away from JS-populated inline dialogs.
**Goal:** rewrite the JS into focused modules with consistent delegation; migrate create-new + binary-replace flows to URL-addressable modals; delete the legacy dialog; clean up `routes/files_routes.py` to share helpers and drop dead endpoints. Behavior visible to the user is unchanged — same features, same UX. Each migration commit leaves the working tree in a known-good Chromium-verified state, so this plan can be paused, resumed, and handed off across sessions.
## Approach: three phases, twelve commits
Three natural pause points for cross-session handoff. Each phase is independently completable and verifiable.
| Phase | Steps | Scope | Where the working tree lands |
|-------|-------|-------|------------------------------|
| **A** | 1-4 | JS rewrite into 4 modules (`core.js`, `editor.js`, `dialogs.js`, `uploads.js`). `editor.js` is dual-purpose at this point (legacy inline dialog + URL-addressable modal). | Old `files-overlay.js` is empty/stub; behavior unchanged; legacy dialog still exists and is still used by create-new + binary-replace. |
| **B** | 5-10 | URL-addressable migration of create-new + binary-replace. Adds `GET /overlays/<id>/files/new?at=<folder>`; binary-file detection in the edit route; binary-replace UI in `overlay_file_editor.html`; JS code-paths for both flows move to URL-addressable. Legacy dialog deleted from `overlay_detail.html`. `editor.js` becomes single-purpose. | Legacy dialog gone; `editor.js` ~200 lines, URL-addressable-only; create-new + binary-replace are URL-addressable. |
| **C** | 11-12 | API cleanup. Extract shared path-resolution + editability helper used by edit/save/content endpoints. Delete `GET /overlays/<id>/files/content` JSON endpoint if confirmed unused. Consolidate save/replace duplication where reasonable. | `routes/files_routes.py` ~450 lines; no dead routes; pytest still green. |
Total estimate: ~12 commits, each independently revertable.
## Decomposition (new JS modules)
| Module | Phase A end | Phase B end | Responsibility |
|--------|-------------|-------------|----------------|
| `static/js/files-overlay/core.js` | ~120 lines | ~120 lines | Helpers (`postJson`, `postForm`, `scheduleRefresh`, `parentOf`, `joinPath`). Manager-element detection, CSRF read. File-row click delegation dispatch. |
| `static/js/files-overlay/editor.js` | ~350 lines (dual-purpose) | ~200 lines (URL-addressable only) | Save / delete / rename-on-save / 409 conflict handling. Filename rename hint. Ctrl+S. After Phase B, only handles URL-addressable modal content via document-level delegation gated on `#modal-content`. |
| `static/js/files-overlay/dialogs.js` | ~180 lines | ~180 lines | New-folder modal, delete-confirm modal, conflict modal (`askConflict` returning a promise). All document-level delegated, no clone-and-rebind. |
| `static/js/files-overlay/uploads.js` | ~280 lines | ~280 lines | Upload queue (concurrency 3), progress panel, per-row cancel via `data-upload-id`. Drag-drop on `treeRoot` kept direct-bound (5 coordinated events, persistent target). |
Total after rewrite: ~780 lines across 4 files (vs 1092 in one).
Direct-bind escape hatches (places we keep direct binding deliberately):
- `editor.js`: `input` / `keydown` on `.files-editor-filename` and `.files-editor-content` (high-frequency, persistent input elements in the swapped-in modal content — re-bound on each `htmx:afterSwap`).
- `uploads.js`: `dragstart` / `dragend` / `dragover` / `dragleave` / `drop` on `treeRoot` (5 coordinated events sharing per-target highlight state; document delegation would obscure the coordination logic).
## Migration sequence
### Phase A — JS rewrite (steps 1-4)
**Step 1: scaffold `files-overlay/core.js`**
Create the new directory. Move helpers (lines ~40-170 of the current file: `postJson`, `postForm`, `scheduleRefresh`, `parentOf`, `joinPath`, byte-count utility, manager-element detection, CSRF read). Add the file-row click delegation that currently lives at line 1054 — same selector match, but instead of switch-casing into local handlers, dispatch through a registry `window.__filesOverlay.handleAction(action, path, actionEl)` that feature modules register handlers into. Old `files-overlay.js` still has its own handlers for now — `core.js` dispatch is unused until subsequent steps wire features into it.
`overlay_detail.html` (NOT base.html): add `<script src="files-overlay/core.js" defer>` BEFORE the existing `files-overlay.js` script tag (currently at `overlay_detail.html:285`, also `defer`). The script tag lives in `overlay_detail.html` because `files-overlay.js` activates only when `.files-manager` exists (line 23-24 of the current file) — loading from `base.html` would pull the JS onto every page unnecessarily. **All new module script tags use `defer`** to match the existing pattern (the modules query the DOM at load time and need the body parsed first).
Verification: existing functionality still works (click any file row → opens editor, +new-file → opens editor, +new-folder → opens new-folder modal, zip download works).
**Step 2: migrate editor handlers to `files-overlay/editor.js`**
Move the editor section (lines 262-591: `editorEls` object, `openEditorTextNew`, `openEditorForFile`, save/delete handlers + sub-element handlers). At the start of `editor.js`'s IIFE, query the legacy dialog once: `const editorDialog = document.getElementById("files-editor-modal")`. If present, register handlers; if absent (i.e., later when the legacy dialog is deleted in Phase B), skip the legacy branch.
For save/delete: convert from direct-bound `editorEls.saveBtn.addEventListener` to document-level delegation scoped via `event.target.closest("#files-editor-modal")`. The URL-addressable modal save/delete delegation already at lines 600-664 moves over too.
For replace-zone drag (lines 449-458): convert to delegation gated on `event.target.closest(".files-editor-replace-zone")` inside `#files-editor-modal`.
Keep direct-bound: `input` on filename, `input` and `keydown` on content textarea (these target persistent inputs inside the persistent legacy dialog).
Register the file-row action handlers (`new-file`, `edit`) into the `core.js` dispatch registry from step 1.
Delete the migrated handlers from the old `files-overlay.js`. The old file shrinks by ~330 lines.
Verification: open editor on text file → edit + save works; rename + save works; rename + 409 conflict alerts and modal stays open; delete works; binary file → opens in binary mode with replace UI; URL-addressable editor flow (file-row click on editable file) still works including rename and 409.
**Step 3: migrate dialogs to `files-overlay/dialogs.js`**
Move new-folder modal (lines 666-722), delete-confirm modal (lines 222-258), conflict modal (`askConflict`, lines 174-211).
Eliminate the clone-and-rebind pattern: register single document-level delegated handlers, scoped to each dialog by id. Per-dialog state (target folder for new-folder, current path for delete-confirm, resolve-callback for conflict) lives in module-scope variables set when the dialog opens and cleared when it closes.
Open the dialogs via `window.modals.openInline(idOrEl)` instead of `dialog.showModal()` directly, completing the inline-modal convention from commit `c51089d`.
Register the file-row action handlers (`new-folder`, `delete`) into `core.js` dispatch registry.
Delete migrated handlers from old `files-overlay.js`. Shrinks another ~150 lines.
Verification: new-folder open + create works; new-folder Enter-in-input creates; delete-confirm open + confirm deletes; upload-conflict prompt (overwrite + keep-both branches both work).
**Step 4: migrate uploads + drag-drop to `files-overlay/uploads.js`**
Move upload queue (lines ~750-900) and drag-drop on `treeRoot` (lines 913-1020).
Keep drag-drop direct-bound to `treeRoot` (deliberate — coordinated state across 5 events).
Convert upload-row cancel buttons (currently direct-bound at row creation, line 753) to document-level delegation: store `data-upload-id="<id>"` on each row and look up the upload at click time.
Register file-row action handlers (`zip`) into `core.js` dispatch registry. (`zip` is just a navigation — could live in `core.js` directly; pick whichever reads cleaner.)
Delete remaining migrated handlers from old `files-overlay.js`. After this step the old file is empty or near-empty. Add `<script src="files-overlay/uploads.js" defer>` etc. to `overlay_detail.html` alongside the (now-empty) `files-overlay.js` script tag (or delete the old `<script>` tag entirely if the file is empty). Initially leave the old file in place to avoid stretching this step further.
Verification: drop a single file onto tree → uploads + appears; drop a folder onto tree → multiple uploads with progress; cancel in-flight upload → stops, row shows cancelled; click Clear → done rows removed; drag a file row to another folder → moves.
End of Phase A: working tree has 4 focused modules + a (near-)empty old file. All current behavior preserved.
### Phase B — URL-addressable migration of create-new + binary-replace (steps 5-10)
**Step 5: extend the edit route to support new-file mode (server + template + tests)**
Add `GET /overlays/<id>/files/new?at=<folder>` to `routes/files_routes.py`. Returns the editor template (`overlay_file_editor.html`) with:
- `rel_path = ""` (empty filename input — user types name)
- `content = ""` (empty editor)
- `byte_count = 0`
- A new context flag `is_new = True`
- The save button label "Create" instead of "Save"
- The delete button hidden
- The target folder rendered as a `data-at-folder="<folder>"` attribute on the textarea so JS save can compose `path = at_folder + "/" + filename` on submit.
Extend `overlay_file_editor.html` to render conditionally based on `is_new`. The existing template has filename, content, save button — just add a `{% if is_new %}Create{% else %}Save{% endif %}` and `{% if not is_new %}<button class="files-editor-delete">{% endif %}`.
Add pytest tests in `tests/test_url_addressable_modals.py`:
- `test_new_route_renders_with_empty_content`
- `test_new_route_renders_with_target_folder_attribute`
- `test_new_route_renders_create_button_not_save`
- `test_new_route_400s_for_invalid_at_path` (path traversal)
- `test_new_route_404s_for_non_files_overlay`
Verification: pytest green; curl the new route, see the editor markup with empty content + "Create" button.
**Step 6: migrate create-new-file JS flow to URL-addressable**
In `editor.js`, the `openEditorTextNew(folder)` path currently populates the legacy inline dialog with an empty filename + content. Change it to call `window.modals.openRouted("/overlays/<id>/files/new?at=" + encodeURIComponent(folder))`.
In the URL-addressable save delegation, detect `is_new` mode (look for `data-at-folder` on the textarea or `value === ""` on the filename input). When new: compose `path = at_folder + "/" + filename.trim()` from the form, send `{path, content}` to `/files/save` (existing endpoint handles creation when the file doesn't exist).
The `core.js` dispatch for `op === "new-file"` (currently calls into the old/legacy flow) is updated to call the new URL-addressable open.
Verification: click + on a folder → URL gains `?modal=/overlays/<id>/files/new?at=foo`; editor opens with empty content + Create button + target folder shown; type filename + content + Create → file appears in tree at the right folder; rename test (type a path-like value into the filename input — should create nested as expected or 422 if invalid).
**Step 7: add binary-file support to the edit route + template (server + template + tests)**
Change `overlay_file_edit_page` in `routes/files_routes.py`: when `is_editable(target)` is False but the file IS readable (size check, no UnicodeDecodeError), instead of returning 415, return the editor template with:
- `is_binary = True`
- `byte_count = target.stat().st_size`
- No `content` (or `content = ""`)
- A `download_url` and `mime_type` (best-effort guess)
Extend `overlay_file_editor.html` to render the binary-replace UI when `is_binary`:
- Hide the CM6 textarea + language dropdown
- Show: file info (name, size), Download button, Replace zone (drag-drop drop zone with browse fallback)
- The Save button is replaced by "Replace" (only enabled when a file is queued)
- The Delete button stays visible
Add pytest tests:
- `test_edit_route_renders_binary_template_for_non_editable`
- `test_edit_route_still_404s_for_missing_file`
- `test_edit_route_still_400s_for_path_traversal`
- `test_binary_template_has_replace_zone`
Verification: navigate to `/overlays/<id>/files/edit?path=image.png` (a real binary file) → page renders with replace UI; navigate via URL-addressable modal → modal opens with same UI.
**Step 8: migrate binary-replace JS flow to URL-addressable**
In `editor.js`, add document-level delegation for the binary-replace zone inside `#modal-content`:
- `dragover`, `dragleave`, `drop` on `.files-editor-replace-zone` (delegated via `event.target.closest(...)`)
- `click` on `.files-editor-replace-browse` and the file input's `change` event for click-to-browse
- `click` on `.files-editor-replace-clear` to clear the queued file
- `click` on `.files-editor-save` when in binary mode → POST `/files/replace` (multipart) with the queued file
The legacy `openEditorForFile(path, false)` branch in `files-overlay.js` (currently called for binary files at line 1083) is replaced by `window.modals.openRouted("/overlays/<id>/files/edit?path=" + encodeURIComponent(path))` — same as for editable files. The server figures out which template branch to render.
Verification: click on a binary file in the tree → URL-addressable modal opens with replace UI; drag a new binary onto the replace zone → queued; click Replace → POST 200; file size updates; file still binary.
**Step 9: delete the legacy `<dialog id="files-editor-modal">` from `overlay_detail.html`**
By this point the legacy dialog has no callers. Delete the block from `overlay_detail.html` (originally lines 165-228, may have shifted).
In `editor.js`, delete all code paths that handle the legacy dialog: the `editorDialog` ref + the document-delegated handlers scoped to `#files-editor-modal` + the `input`/`keydown` direct-binds that were only needed for the legacy persistent inputs. The `editor.js` module shrinks to ~200 lines, single-purpose (URL-addressable modal only).
Add a pytest assertion (in `test_url_addressable_modals.py`) that `id="files-editor-modal"` does NOT appear in the rendered overlay detail page.
Verification: overlay detail page renders without the legacy dialog (DOM inspector); URL-addressable editor still works for text + binary + create-new flows; all existing pytest tests still pass.
**Step 10: delete `files-overlay.js` stub + update base.html**
If `files-overlay.js` is empty or just an IIFE shell, delete it. Update `overlay_detail.html` (NOT base.html) to load only the 4 new modules (`core.js`, `editor.js`, `dialogs.js`, `uploads.js`), all with `defer`. Order doesn't strictly matter for execution (each is an independent IIFE), but `core.js` first makes the registry-of-handlers pattern explicit.
Verification: full re-run of the URL-addressable-modals spec's verification matrix (10 checks); 4 new modules' features all work (editor text + binary + create-new; new-folder; conflict; delete-confirm; drag-drop; uploads cancel + clear).
End of Phase B: legacy dialog gone, `editor.js` single-purpose, all editor flows URL-addressable.
### Phase C — API cleanup (steps 11-12)
**Step 11: extract shared path-resolution + editability helper**
`routes/files_routes.py` has duplication between `overlay_file_content` (lines 203-234, JSON output) and `overlay_file_edit_page` (lines 237-275, HTML output, added in Task 3). Both:
- Read `request.args.get("path", "")`
- Call `_load_files_overlay(overlay_id, user)`
- Call `safe_resolve_for_listing(overlay.path, sub_path)`
- Check `target.exists() and target.is_file()`
- Call `is_editable(target)`
- Try `target.read_text(encoding="utf-8")` with OSError + UnicodeDecodeError fallback
Extract `_load_file_for_editing(overlay_id, sub_path, user) -> (overlay, target_path, content_or_None, is_binary, byte_count) | Response`:
- Returns a tuple on success, a Response on any failure case (404, 415, 400, 403)
- Both routes call this helper and translate the tuple into their respective output shapes
Same path: `is_editable` checks become part of the helper.
Add pytest tests for the helper directly if reasonable, plus confirm existing route tests still pass.
Verification: existing pytest tests stay green (no behavior change); both routes shorter and obviously parallel.
**Step 12: delete `GET /overlays/<id>/files/content` if unused; consolidate save/replace**
Audit: search the codebase for callers of `/files/content` (JSON endpoint). After Phase B, the legacy `openEditorForFile()` is gone, which was its only caller. If grep confirms no other callers, delete the endpoint + its tests.
`overlay_file_save` and `overlay_file_replace` share the rename branch (lines 276-285 of save, lines 322-335 of replace). Extract `_apply_optional_rename(overlay, path, new_path) -> (write_target, echo_path) | Response`. Both endpoints call it.
Verification: pytest stays green; grep confirms no remaining references to `/files/content`; both save/replace routes shorter.
End of Phase C: `routes/files_routes.py` ~450 lines (vs 640), no dead endpoints, shared helpers.
## Critical files
| Path | Phases | Action |
|------|--------|--------|
| `l4d2web/l4d2web/static/js/files-overlay/core.js` | A | New file |
| `l4d2web/l4d2web/static/js/files-overlay/editor.js` | A → B | New file, then shrinks in Phase B |
| `l4d2web/l4d2web/static/js/files-overlay/dialogs.js` | A | New file |
| `l4d2web/l4d2web/static/js/files-overlay/uploads.js` | A | New file |
| `l4d2web/l4d2web/static/js/files-overlay.js` | A → B | Shrinks each step; deleted in step 10 |
| `l4d2web/l4d2web/templates/overlay_detail.html` | A, B | Script tag updates each phase end (currently loads files-overlay.js at line 285 with `defer`) |
| `l4d2web/l4d2web/templates/overlay_file_editor.html` | B | Add new-file + binary-replace branches |
| `l4d2web/l4d2web/templates/overlay_detail.html` | B | Delete legacy `<dialog id="files-editor-modal">` (step 9) |
| `l4d2web/l4d2web/routes/files_routes.py` | B, C | Add `/files/new` route (step 5); extend `/files/edit` for binary (step 7); extract helpers (step 11); delete `/files/content` (step 12) |
| `l4d2web/tests/test_url_addressable_modals.py` | B | Add tests for new + binary modes (steps 5, 7); add legacy-dialog-gone assertion (step 9) |
## Existing functions and utilities to reuse
- `window.modals.openInline(idOrEl)` / `closeInline()` / `openRouted(path)` / `closeRouted()` — at `static/js/modals.js`. New code uses these instead of `dialog.showModal()` directly.
- `window.__editor.initEditors(root)` — at `static/js/editor.js`. CM6 re-init on swapped-in textareas. The new-file flow's empty textarea also needs CM6 mount; this just works because the `htmx:afterSwap` listener already covers it.
- `window.__filesEditor.getValue()` — set by `editor.js` when CM6 mounts in the modal content. Used by `editor.js`'s save delegation.
- `_load_files_overlay`, `safe_resolve_for_listing`, `is_editable`, `_validate_save_content`, `_stream_upload_into` — all in `routes/files_routes.py`. Reused by new routes and the extracted helper.
- Existing Flask routes `/files/{save,delete,replace,upload,move,mkdir,download,download_zip,edit}` — consumed unchanged by the new JS modules. `/files/content` deleted in Phase C.
## Verification
Each step has its own check (above). Phase-end checks:
**Phase A end** (after step 4): all current features still work. 573 backend tests pass. Chromium pass on: file-row click → editor; +new-file → editor (in legacy dialog); +new-folder → new-folder modal; click binary file → editor in binary mode; drag file row → moves; drop file onto tree → uploads.
**Phase B end** (after step 10): legacy dialog gone; create-new + binary-replace are URL-addressable. Full re-run of the URL-addressable-modals spec verification matrix (`docs/superpowers/specs/2026-05-17-url-addressable-modals-design.md`, ## Verification, 10 checks) passes. New checks added: create-new URL deep-link works, binary-replace URL deep-link works.
**Phase C end** (after step 12): `routes/files_routes.py` shorter; 573 backend tests stay green; `grep -rn "/files/content"` returns nothing (or only confirms the deletion).
## Handoff state
After **each** commit, the working tree is in a known-good Chromium-verified state. A future session resumes by:
1. Reading this plan file at `docs/superpowers/plans/2026-05-17-files-overlay-rewrite.md`.
2. Reading `git log --oneline` to see which steps have shipped.
3. Picking up at the next un-committed step.
4. Per `feedback_textarea_editor_v2_run` memory: direct-to-master on left4me, skip subagent middleman for verbatim-from-plan tasks, document plan deviations in commit messages, Chromium verification works in default sandbox via `./scripts/dev-server.py`.
Natural pause points: end of Phase A, end of Phase B, end of Phase C. Each phase delivers value independently — Phase A alone is a useful cleanup if the user doesn't want to do B and C.
## Errata (pre-execution)
- **Script tag location:** the plan's first draft incorrectly referenced `base.html` for the script tag updates. The actual location is `overlay_detail.html:285` (with `defer`). `files-overlay.js` and its replacement modules are page-scoped to overlay detail — loading from `base.html` would pull the JS onto every page when the `.files-manager` element only exists on overlay detail. All script tag references in this plan now correctly say `overlay_detail.html`. This errata is itself an example of the `feedback_validate_before_implementing` memory: probe the live state before trusting a plan claim.
## Out of scope
- Adding a JS test framework (no JS tests in the project today; verification stays Chromium-driven).
- Migrating the other inline modals (rename, delete-overlay, new-folder-overlay, etc.) — unrelated to this file.
- Restructuring the `/files/upload` chunking or progress-event behavior.
- Refactoring `_overlay_file_node.html`, `_overlay_file_tree.html`, or other template partials that this JS doesn't own.