feat(ui): documents open in an almost-fullscreen modal instead of a new page — same-page overlay on chat + Sources, /document.html kept as the no-JS/direct-link fallback
This commit is contained in:
@@ -0,0 +1,44 @@
|
||||
# Phase 26 — Document Modal Viewer
|
||||
|
||||
**Source:** `TODO.md L4 — "New documents should open in an almost-fullscreen modal, not in a new page"`
|
||||
**Story:** `.agent/user_stories/document-modal.md`
|
||||
**Context:** Phase 10 added the separate `/document.html` viewer page; phase 19 added the shared header bar that now lives on every page. The document content is served by the stateless `GET /api/documents/content` endpoint (PLAN §4).
|
||||
|
||||
## Objective
|
||||
Stop opening cited documents in a new page/tab. Clicking a source chip or a Sources-table path link now opens the document in an **almost-fullscreen modal overlay** on the current page, fed by the same `/api/documents/content` endpoint. The existing `/document.html` page stays as the no-JS / direct-link fallback and its behaviour is unchanged.
|
||||
|
||||
## Dependencies
|
||||
- `10_story_document_viewer` (complete) — the `/document.html` page, the `document.js` renderer, the `renderMarkdown` escape-first renderer in `markdown.js`, and the `#doc-content` / `.doc-md` / `.doc-raw` markup this phase reuses inside the modal.
|
||||
- `19_shared_header` (complete) — the shared header bar the modal sits under; the modal must not disturb the header.
|
||||
- `08_story_dark_tech_theme` (complete) — the Phase-08 tokens and the ≥4.5:1 contrast / `prefers-reduced-motion` contract the modal must honour.
|
||||
|
||||
## Tasks
|
||||
1. `01_modal_css_and_html.md` — the modal CSS (overlay, backdrop, close button, scrollable content area) + inject the modal skeleton into `index.html`
|
||||
2. `02_app_js_modal_intercept.md` — intercept document links in `app.js` + `sources.js`, fetch content via `/api/documents/content`, render inside the modal
|
||||
3. `03_document_js_modal_mode.md` — adapt `document.js` to optionally render in modal mode (reuse the same API call) for the direct-link fallback path
|
||||
4. `04_e2e_regression_suite.md` — update `test_document_viewer.py` to verify modal behaviour; the story gate, run in isolation
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: none required for the modal itself (frontend-only); the `/api/documents/content` endpoint is unchanged (no `app/` change → no coverage delta).
|
||||
- Coverage: frontend-only; the >90% `app/` gate is unaffected.
|
||||
- E2E: `tests/e2e/test_document_viewer.py` rewritten for the modal contract (task 4), green **in isolation** (prereq `podman compose up -d db`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Clicking a source chip (chat) or a Sources-table path link opens the document in an almost-fullscreen modal on the **same page** (no new tab, no navigation).
|
||||
- [ ] The modal renders the same content the `/document.html` page renders: md/markdown via the shared renderer (`.doc-md`), other formats in `<pre class="doc-raw">`, source/format/path/indexed/chunks meta.
|
||||
- [ ] The modal has a visible close control, closes on Escape, closes on backdrop click, and keeps the dark theme + a11y frame (skip-link, focus trap, `:focus-visible`, aria-label).
|
||||
- [ ] The existing `/document.html` page still works unchanged (direct link, back button, XSS-safe rendering, not-found state).
|
||||
- [ ] No CDN tags on any touched page; every asset reference is same-origin or `data:`.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL ≥ pre-change number (gate >90%).
|
||||
- [ ] `uv run pytest tests/e2e/test_document_viewer.py -v --no-cov` green in isolation.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] UI Structure Check (AGENTS.md rule 5): modal content uses the standard centered column width for md; backdrop behind content; no 360px overflow.
|
||||
- [ ] `.agent/user_stories/document-modal.md` exists.
|
||||
- [ ] One `--no-gpg-sign` commit staging only this phase's files; `.agent/phases/todo/26_document_modal_viewer/` moved to `.agent/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **No backend change** — the modal reuses `GET /api/documents/content` unchanged (A10 untouched: the API stays stateless).
|
||||
- **A11 untouched** — vanilla HTML/CSS/JS, no CDN, zero new packages, no new assets, system font stack; the modal is pure CSS + JS.
|
||||
- **No anchor revised** — this is a UI-behaviour change (PLAN §7.5 gains `#doc-modal`, `#doc-modal-backdrop`, `#doc-modal-close`, `#doc-modal-content`); the `/document.html` page and its story are unchanged.
|
||||
- **A16 honoured** — one story E2E suite (rewritten) + adapted regressions.
|
||||
- **A17 honoured** — one atomic `--no-gpg-sign` commit.
|
||||
@@ -0,0 +1,62 @@
|
||||
# Task 01 — Modal CSS + HTML skeleton
|
||||
|
||||
**Phase:** `26_document_modal_viewer` · **Source:** `TODO.md:4 — "New documents should open in an almost-fullscreen modal, not in a new page"`
|
||||
**Story:** `.agent/user_stories/document-modal.md`
|
||||
|
||||
## Objective
|
||||
Add the modal markup to `index.html` and the CSS that styles an almost-fullscreen overlay (backdrop + panel + close button + scrollable content) using the Phase-08 tokens.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html` — insert the modal skeleton just before the closing `</body>` (after the existing script tags, or before them — order doesn't matter for a static skeleton). The skeleton:
|
||||
```html
|
||||
<div class="doc-modal" id="doc-modal" hidden>
|
||||
<div class="doc-modal-backdrop" id="doc-modal-backdrop" aria-hidden="true"></div>
|
||||
<div class="doc-modal-panel" role="dialog" aria-modal="true" aria-labelledby="doc-modal-title" aria-describedby="doc-modal-desc">
|
||||
<header class="doc-modal-header">
|
||||
<h2 class="doc-modal-title" id="doc-modal-title">Loading…</h2>
|
||||
<div class="doc-modal-actions">
|
||||
<a class="doc-modal-open" id="doc-modal-open" target="_blank" rel="noopener" hidden aria-label="Open in full page">
|
||||
<svg aria-hidden="true" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.7" stroke-linecap="round"><path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"/><path d="M15 3h6v6"/><path d="M10 14 21 3"/></svg>
|
||||
<span>Full page</span>
|
||||
</a>
|
||||
<button type="button" class="doc-modal-close" id="doc-modal-close" aria-label="Close document">
|
||||
<svg aria-hidden="true" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.7" stroke-linecap="round"><path d="M18 6 6 18M6 6l12 12"/></svg>
|
||||
</button>
|
||||
</div>
|
||||
</header>
|
||||
<div class="doc-modal-meta" id="doc-modal-meta" aria-live="polite"></div>
|
||||
<p class="visually-hidden" id="doc-modal-desc" role="status">Document content is loading.</p>
|
||||
<main class="doc-modal-content" id="doc-modal-content" tabindex="-1">
|
||||
<p class="doc-modal-loading" role="status">Loading document…</p>
|
||||
</main>
|
||||
</div>
|
||||
</div>
|
||||
```
|
||||
The `hidden` attribute keeps the modal off until JS opens it. The `#doc-modal-open` "Full page" link points at the same `/document.html?source=…&path=…&modal=…` URL the modal will build so a user can still open the dedicated page if JS is off.
|
||||
2. `frontend/assets/styles.css` — add a `--doc-modal-*` block (Phase-08 tokens). Styling contract:
|
||||
- `.doc-modal` — `position: fixed; inset: 0; z-index: 1000;` (above the shared header and every page layer, below the phase-25 background which is `z-index: -1`); the panel is flex, column; the backdrop + panel fill the viewport.
|
||||
- `.doc-modal-backdrop` — `position: fixed; inset: 0; background: rgba(10,14,23,0.82);` backdrop blur is **not** used (phase-08 no-blur perf anchor); `opacity` transition 120ms.
|
||||
- `.doc-modal-panel` — `display: flex; flex-direction: column; width: min(1100px, 96vw); height: 92vh; margin: auto; background: var(--surface, #121a2e); border: 1px solid var(--line, #232b52); border-radius: 12px; box-shadow: 0 24px 80px rgba(0,0,0,.55);` — "almost-fullscreen" = 96vw × 92vh, centered.
|
||||
- `.doc-modal-header` — sticky top, same height/spacing as the doc header (64px / 58px pins from phase 12); title uses `--ink`; close button ≥44px target, focus-visible ring.
|
||||
- `.doc-modal-content` — `flex: 1; overflow: auto;` (vertical scroll inside the panel, not the viewport); padding; the md content reuses `.doc-md` (≤46rem centered column) — the modal just provides the scroll container. For wide raw formats the `.doc-raw` pre already has `overflow-x: auto`.
|
||||
- `.doc-modal-meta` — reuses the `.doc-meta` styling already defined for the viewer page (source/format/path/indexed/chunks badges); keep it compact (single row, wrap).
|
||||
- `.doc-modal-close` — icon-only button, `aria-label` kept, `:focus-visible` 3px ring.
|
||||
- Transitions respect `prefers-reduced-motion: reduce` (no opacity/transform animation, or `animation: none` under the reduced-motion media query — same pattern as the phase-25 background layers).
|
||||
- `.doc-modal[hidden]` — `display: none` (the `hidden` IDL attribute default already hides it; add the rule to be explicit and testable).
|
||||
- Ensure the modal panel does not add horizontal width at 360px (no `box-sizing` surprises; the panel is `96vw` ≤ viewport).
|
||||
3. Verify the new CSS classes do not collide with any existing selector in `styles.css` (grep for `.doc-modal`, `.doc-modal-`).
|
||||
|
||||
## ASSUMPTIONS
|
||||
- The modal panel is `96vw × 92vh` ("almost-fullscreen"). If the owner wants a different fraction, that's a follow-up.
|
||||
- The "Full page" link is admin-agnostic (it just opens `/document.html`); it is shown for everyone since the viewer is public.
|
||||
- The modal uses the existing `.doc-meta` badge classes already defined for the viewer page (no duplicate styling).
|
||||
|
||||
## Testing & Quality
|
||||
- No unit/integration test for static CSS/HTML.
|
||||
- Coverage: frontend-only; the >90% `app/` gate is unaffected.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `index.html` contains the `.doc-modal` skeleton with the documented ids (`#doc-modal`, `#doc-modal-backdrop`, `#doc-modal-panel`, `#doc-modal-close`, `#doc-modal-title`, `#doc-modal-meta`, `#doc-modal-content`, `#doc-modal-open`).
|
||||
- [ ] The modal CSS block is present, uses Phase-08 tokens, has no `filter: blur`/`backdrop-filter`, and the panel is `96vw × 92vh` centered.
|
||||
- [ ] No selector collision (grep clean).
|
||||
- [ ] `prefers-reduced-motion` stills any modal transition.
|
||||
@@ -0,0 +1,40 @@
|
||||
# Task 02 — Intercept document links → modal
|
||||
|
||||
**Phase:** `26_document_modal_viewer` · **Source:** `TODO.md:4 — "New documents should open in an almost-fullscreen modal, not in a new page"`
|
||||
**Story:** `.agent/user_stories/document-modal.md`
|
||||
|
||||
## Objective
|
||||
Intercept document links on the chat page (`app.js` source chips) and the Sources page (`sources.js` table links): instead of navigating to `/document.html` in a new tab, fetch the document via `GET /api/documents/content` and render it inside the modal from task 01.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/document.js` — extract the rendering logic into a reusable, importable function so both the standalone page (task 03) and the modal share the exact same renderer. Specifically:
|
||||
- Export `renderDocument(doc, { containerEl, metaEl, titleEl })` that populates a title element, a meta element (the `.doc-meta` badges), and a content element (`.doc-md` for markdown via `renderMarkdown`, `<pre class="doc-raw">` otherwise). Keep the escape-first XSS-safety contract (`innerHTML` only through `renderMarkdown`; `textContent` for raw + badges).
|
||||
- The standalone `document.html` page keeps its own `load()` that calls `renderDocument` with its page elements (task 03 wires this).
|
||||
2. `frontend/assets/app.js` — add a `openDocumentModal(source, path)` helper:
|
||||
- Build the modal URL: `/api/documents/content?source=…&path=…` (same encoding the chips already use).
|
||||
- Show the modal: set `#doc-modal.hidden = false`, set the loading state, move focus into `#doc-modal-content` (a11y — the panel is `tabindex="-1"`).
|
||||
- `fetch(contentUrl)` → on `!r.ok` render a short "document not found" line in the content area; on success call `renderDocument` with `#doc-modal-title`, `#doc-modal-meta`, `#doc-modal-content`.
|
||||
- The "Full page" link (`#doc-modal-open`) is set to the `/document.html?source=…&path=…` URL on open.
|
||||
- Keep the existing `documentUrl()` builder for the "Full page" link (unchanged output).
|
||||
- Add modal close behaviour: `#doc-modal-close` click → `closeDocumentModal()`; backdrop click → close; `Escape` key → close; closing restores focus to the link that opened the modal (best-effort — store the triggering element).
|
||||
3. `frontend/assets/app.js` — wire the source chips: replace `chip.target = "_blank"` navigation with `chip.addEventListener("click", e => { e.preventDefault(); e.stopPropagation(); openDocumentModal(s.source, s.path, chip); })`. Keep the `title`/aria-label truncation logic the chips already have. The chip keeps its `href` too (no-JS fallback would navigate to `/document.html`).
|
||||
4. `frontend/assets/sources.js` — wire the table links the same way: the `.doc-link` click is intercepted, `preventDefault`, and `openDocumentModal(d.source, d.path, link)` is called. Since `openDocumentModal` lives in `app.js` (the chat page module) and `sources.js` is a separate module, **export** `openDocumentModal` from `app.js` and import it in `sources.js` — but `app.js` is loaded as a module on the chat page only. To avoid a second module instance, move the shared modal logic into a small new module `frontend/assets/document-modal.js` (task 02 step 1 refined below) and have both `app.js` and `sources.js` import it.
|
||||
- **Refined split:** create `frontend/assets/document-modal.js` exporting `openDocumentModal(source, path, triggerEl)` and `closeDocumentModal()`. This module owns the modal DOM wiring (close on ESC / backdrop / button, focus management) and the `fetch` + `renderDocument` call. `app.js` and `sources.js` just call `openDocumentModal(...)` from their click handlers. This is the cleanest single-implementation approach (mirrors how `header.js` is the single owner of the shared header).
|
||||
- `document.js` (standalone page) also imports `renderDocument` from itself (or a shared `document-render.js`) — keep the standalone page self-contained; it doesn't need the modal module.
|
||||
|
||||
## ASSUMPTIONS
|
||||
- The modal module (`document-modal.js`) is a classic or module script loaded on both `index.html` and `sources.html`. It's a module (imports `renderDocument` from `document.js`), so both pages must load it via `<script type="module">`. `document.js` will export `renderDocument`.
|
||||
- Close-on-`Escape` and close-on-backdrop are modal UX standards; the owner's item says "modal, not a new page", which implies standard modal affordances.
|
||||
- The "Full page" link remains for users who want the dedicated viewer; it is optional and doesn't interfere with the modal.
|
||||
|
||||
## Testing & Quality
|
||||
- No unit/integration test (frontend-only).
|
||||
- Coverage: frontend-only; the >90% `app/` gate is unaffected.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Clicking a source chip or a Sources-table path link opens the modal and renders the document (md via `.doc-md`, other formats via `.doc-raw`).
|
||||
- [ ] The modal closes on button click, on backdrop click, and on `Escape`; focus returns to the triggering control.
|
||||
- [ ] No new tab opens from either link type.
|
||||
- [ ] The "Full page" link still navigates to `/document.html` (unchanged).
|
||||
- [ ] XSS-safe rendering preserved (markdown escaped, raw set via `textContent`).
|
||||
- [ ] Both pages load the modal module without a duplicate-module error.
|
||||
@@ -0,0 +1,29 @@
|
||||
# Task 03 — Standalone viewer page reuses the shared renderer
|
||||
|
||||
**Phase:** `26_document_modal_viewer` · **Source:** `TODO.md:4 — "New documents should open in an almost-fullscreen modal, not in a new page"`
|
||||
**Story:** `.agent/user_stories/document-modal.md`
|
||||
|
||||
## Objective
|
||||
Keep `/document.html` working exactly as before (it is the no-JS / direct-link fallback) but refactor its `document.js` so the markdown/raw rendering lives in a shared function the modal module can reuse. No behavioural change to the standalone page.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/document.js` — split the current inline renderer into an exported `renderDocument(doc, { titleEl, metaEl, contentEl })` function (the escape-first contract: markdown → `renderMarkdown` into a `.doc-md` div; other formats → `<pre class="doc-raw">` via `textContent`; badges via `textContent`). The page's existing `load()` IIFE now calls `renderDocument` with the page's `#doc-title`, `#doc-meta`, `#doc-content` elements. Everything else in `document.js` (query-param parsing, `back` target, not-found card, shared header wiring, New Chat button, `mainEl.focus()`) is **unchanged**.
|
||||
2. `frontend/assets/document-modal.js` (new) — imports `renderDocument` from `./document.js`. Owns `openDocumentModal(source, path, triggerEl)` and `closeDocumentModal()` (see task 02). On open it fetches `/api/documents/content` and calls `renderDocument(doc, { titleEl: #doc-modal-title, metaEl: #doc-modal-meta, contentEl: #doc-modal-content })`. It also sets `#doc-modal-open.href` to the `/document.html?source=…&path=…` URL.
|
||||
3. `frontend/index.html` — load `document-modal.js` as a module (add `<script type="module" src="/assets/document-modal.js"></script>` alongside the existing `app.js` module script). `index.html` already loads `markdown.js` as a classic script (needed by `renderDocument`).
|
||||
4. `frontend/sources.html` — load `document-modal.js` as a module (it needs `document.js`'s `renderDocument`, so both `document.js` and `document-modal.js` must be module scripts; `markdown.js` classic script stays). The Sources page currently loads `sources.js` as a module; add the modal module script next to it.
|
||||
5. Verify the no-CDN integration test (`tests/integration/test_api.py::test_index_html_served_locally`) still passes — the new module scripts are same-origin `<script src>`, so they satisfy the "local asset" rule. If the test counts script tags, update the expected count.
|
||||
|
||||
## ASSUMPTIONS
|
||||
- `renderDocument` depends on `renderMarkdown` (from `markdown.js`), which is a classic script — so `document.js` (module) importing nothing but using the global `renderMarkdown` is fine, and `document-modal.js` (module) importing `renderDocument` from `document.js` also relies on the global `renderMarkdown` being present. Both pages load `markdown.js` before the module scripts (hoisting guarantees module scripts run after classic scripts already on the page).
|
||||
- The standalone page's `document.js` no longer needs to be a module for its own rendering — but it stays a module because it imports `header.js` (shared header). Keep it a module.
|
||||
|
||||
## Testing & Quality
|
||||
- No unit/integration test for the refactor itself.
|
||||
- Coverage: frontend-only; the >90% `app/` gate is unaffected.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `/document.html?source=…&path=…` still renders title/meta/content exactly as before (md in `.doc-md`, raw in `<pre.doc-raw>`).
|
||||
- [ ] The not-found state, `back` target, shared header, and New Chat button on `/document.html` are unchanged.
|
||||
- [ ] `document-modal.js` is loaded on `index.html` and `sources.html`; `document.js` exports `renderDocument`.
|
||||
- [ ] No-CDN test still passes (new scripts are same-origin).
|
||||
- [ ] No console errors on any of the three pages.
|
||||
@@ -0,0 +1,33 @@
|
||||
# Task 04 — E2E story suite (modal) + regressions
|
||||
|
||||
**Phase:** `26_document_modal_viewer` · **Source:** `TODO.md:4 — "New documents should open in an almost-fullscreen modal, not in a new page"`
|
||||
**Story:** `.agent/user_stories/document-modal.md`
|
||||
|
||||
## Objective
|
||||
Rewrite the phase-10 E2E suite to assert the **modal** contract (open in a modal on the same page, no new tab; close on button/Escape/backdrop; dark theme; no CDN; a11y frame), and confirm the standalone `/document.html` page still works.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/test_document_viewer.py` — rewrite for the modal contract (the seeding harness from the phase-10 file — `_import_fixtures` / `_reset_db` / `_run_in_thread` — stays identical; only the assertions change):
|
||||
- `test_source_chip_opens_modal` — from the chat page, ask the QUESTION, wait for the `kubernetes.md` source chip, click it (no `target=_blank` click → `expect_popup`); assert the modal `.doc-modal` is visible, NOT hidden; `#doc-modal-title` = "Kubernetes Homelab Cluster"; `#doc-content`/`.doc-md` present; content text "Talos Linux on three nodes". Assert the page URL is unchanged (still `/`).
|
||||
- `test_sources_row_opens_modal` — log in, find the `gitlab-compose.yaml` row link, click it; assert the modal is open with the yaml rendered in `<pre.doc-raw>` containing "gitlab/gitlab-ce:17.2.1-ce.0", mono font.
|
||||
- `test_modal_closes_on_button_escape_and_backdrop` — open the modal, click `#doc-modal-close` → hidden; re-open, click backdrop → hidden; re-open, press Escape → hidden.
|
||||
- `test_modal_focus_and_a11y` — on open, focus is inside `#doc-modal-content`; the panel has `role="dialog"` + `aria-modal="true"`; the close button has `aria-label`.
|
||||
- `test_modal_xss_safe` — seed an XSS fixture doc, open via modal, assert the `<script>` shows as escaped text and no dialog fires (same as the phase-10 test but inside the modal).
|
||||
- `test_standalone_page_still_works` — the phase-10 assertions for `/document.html` (title/content/format badge, not-found state, dark theme, no-CDN, a11y frame, `#doc-content .doc-md` ≤ 736px) are **kept** — the dedicated page must not regress.
|
||||
- `test_modal_theme_and_no_cdn` — dark theme (document background `rgb(10,14,23)`), and the modal panel uses Phase-08 surface colour.
|
||||
2. `tests/integration/test_api.py` — if the no-CDN test counts `<script>` tags on `index.html` / `sources.html`, bump the expected count to include `document-modal.js` (and confirm `document.html` count is unchanged).
|
||||
3. Regressions to run green in isolation after the change: `test_document_back_navigation.py` (source chips now open a modal; verify the back-navigation story doesn't assert a new tab — if it does, adapt), `test_header_consistency.py` (new module scripts don't disturb the header), `test_smoke.py`.
|
||||
4. `.agent/user_stories/document-modal.md` — write the story file mapping the modal behaviour to the E2E scenarios above.
|
||||
|
||||
## ASSUMPTIONS
|
||||
- The phase-10 `expect_popup` calls are removed (no new tab); the modal opens in-page.
|
||||
- The standalone page test is kept to guard the no-JS / direct-link fallback.
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: `tests/e2e/test_document_viewer.py` rewritten — the story gate, green **in isolation** (prereq `podman compose up -d db`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_document_viewer.py -v --no-cov` green in isolation.
|
||||
- [ ] `test_document_back_navigation.py`, `test_header_consistency.py`, `test_smoke.py` green in isolation (adapted if they asserted a new tab).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL ≥ pre-change number.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
Reference in New Issue
Block a user