From 0d3554f75e75d91e0e6d86022c45d279dd9d7d96 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 3 Oct 2026 13:56:35 -0700 Subject: [PATCH 1/3] feat(annotate): open local image links in an HTML page in the image lightbox A link to a local image inside an annotated HTML page (a thumbnail linking to its full-size render) used to toast "Can't open". It now opens in the image lightbox the markdown Viewer already has, extracted to a shared ImageLightbox component. - resolveHtmlLinkIntent gains an image intent: png jpg jpeg gif webp svg avif bmp ico apng (the code-review preview set), resolved to the page's own /api/html-assets// route and only inside the page's folder. Out-of-folder images and pages with no asset route keep a toast that says why. - The lightbox takes focus (out of the iframe) and swallows keys on the window capture phase, so Esc closes it before the HTML Esc ladder and Mod+Enter cannot submit behind it (also fixes the markdown lightbox). - Armed clicks still pin; Interact opens the lightbox, same as doc links. - The asset route also serves .bmp (shared table, vendored to Pi). --- .github/workflows/test.yml | 1 + AGENTS.md | 2 +- packages/editor/App.tsx | 32 +++++- packages/shared/html-assets.ts | 1 + packages/ui/components/ImageLightbox.test.tsx | 85 ++++++++++++++ packages/ui/components/ImageLightbox.tsx | 66 +++++++++++ packages/ui/components/Viewer.tsx | 29 +---- packages/ui/utils/htmlLinkNavigation.test.ts | 104 ++++++++++++++++++ packages/ui/utils/htmlLinkNavigation.ts | 80 +++++++++++++- 9 files changed, 366 insertions(+), 34 deletions(-) create mode 100644 packages/ui/components/ImageLightbox.test.tsx create mode 100644 packages/ui/components/ImageLightbox.tsx diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e9d7a1753b..986c10c8a9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -118,6 +118,7 @@ jobs: packages/ui/components/ThemeProvider.favicon.test.tsx packages/ui/components/MermaidBlock.theme.test.tsx packages/ui/components/DiagramBlock.anchor.test.tsx + packages/ui/components/ImageLightbox.test.tsx packages/ui/components/blocks/QuestionBlock.test.tsx packages/ui/components/blocks/QuestionBlock.host.test.tsx packages/ui/hooks/useAnnotationHighlighter.questionSkip.test.tsx diff --git a/AGENTS.md b/AGENTS.md index e48c87da6b..02443acd55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1449,7 +1449,7 @@ These surfaces are **comment-only**. `redline` (auto-DELETION) and `quickLabel` **HTML Refresh (#1232).** A local rendered-HTML session can re-read its file from disk without reloading the tab, for the loop where an agent edits the page while the reviewer keeps annotating. The header **Refresh** button (left of the eye, `data-html-refresh`, titled "Refresh HTML from disk") fetches the active document through `/api/doc`, hands the bytes to the app, and remounts the viewer under a bumped `reloadGeneration` key (`packages/editor/App.tsx`, viewer `key`). The engine is the published `useHtmlRefresh` (`packages/ui/hooks/useHtmlRefresh.ts`: superseded and cross-document fetches are dropped, one restore acknowledgement per generation) and Plannotator's binding over `fetchHtmlDocumentSnapshot` is `packages/editor/hooks/useHtmlRefresh.ts` (toasts for refreshed, missing, and unavailable). Committed annotations survive on their durable anchors: the remounted viewer re-resolves every element selector and text snapshot against the new page, and the ones it cannot re-anchor are reported once (`onUnanchoredChange` to `reportAnnotationRestore`), toasted, and marked with an **Unanchored** chip in the annotations panel (`htmlUnanchoredIds` in App, cleared when the document changes); their comments stay in the panel and still export. A refresh keeps the version diff: for the root document `/api/doc` carries `previousPlan`/`versionInfo`/`diffHtml` recomputed against the bytes just read (see the annotate `/api/plan` row), `applyRefreshedHtml` sets them and resets `isPlanDiffActive`, so the view returns to normal mode with "Show changes" still available; a tab reload converges on the same state because `/api/plan` serves the current bytes and recomputes the same diff. `/api/share-html` shares the current bytes too. Only local files refresh: `canRefresh` is false for `http(s)` paths and live-app sessions, and the control is absent on read-only (archive) documents. The compact touch shell renders no header controls (`HtmlSurfaceControls` returns null when `compact`), so its Options menu offers "Refresh from disk" beside the Show/Hide tools and Interact/Annotate actions (`compactDocumentActions` in App, disabled while a refresh is in flight); a host that passes `canRefresh` and `onRefresh` to `HtmlSurfaceControls` gets the Refresh button with or without the eye. -**Links between local HTML documents.** A srcdoc document has no URL of its own — its base URL is the PARENT page's, which is the Plannotator server — so an ordinary `` used to resolve onto the server, hit the catch-all, and render the whole editor inside the annotated frame; an in-page `#section` link navigated for the same reason. The bridge therefore **never lets the srcdoc frame navigate itself**: a capture-phase click handler registered before the pinpoint handler (and never stopping propagation, so armed clicks still pin the link element) `preventDefault`s every link click, scrolls in-page `#fragment`s locally, and posts the RAW href to the parent as `link-click`. `javascript:` hrefs are left to the page. The parent is the trust boundary: `parseBridgeMessage` bounds the href (2048 chars, no control characters) and `resolveHtmlLinkIntent` (`packages/ui/utils/htmlLinkNavigation.ts`, pure) decides what it means — a relative or `../` path resolves against the CURRENT document's directory and opens as a **linked document** through `/api/doc` exactly like a relative markdown link, a root-relative path (and the same path spelled with the server's own origin, the shape an author writes as `http://localhost:/01-entry-point.html`) resolves against the directory the session was opened from, another origin opens in a **new tab** (`noopener,noreferrer`, the frame never navigates), a local file outside the annotatable set (`.pdf`, `.zip`) raises a toast and is never fetched, and every other scheme (`data:`, `file:`, `mailto:`, `blob:`) is dropped. The query string is stripped and the fragment is kept: it rides to the new document as `HtmlViewer`'s `initialFragment` and is replayed once that document's bridge is ready (`scroll-to-fragment`). Folder sessions route the open through the file-browser selection handler so the active file, the sidebar and the linked doc stay in step. Annotations stay per document (the `useLinkedDoc` cache), and the Interact/Annotate state and the chrome rules keep applying. **Navigating between HTML documents never opens the sidebar**: the page owns the viewport on this surface (it opens with the tools hidden and the sidebar closed), so a link click leaves the sidebar exactly as the reviewer had it — closed stays closed, open stays on its tab. `useLinkedDoc.open` / `openLoaded` take `revealSidebar` for this (default `true`, so every markdown caller is unchanged), and App passes `false` only when the target renders as HTML — `resolveHtmlLinkIntent`'s `rendersHtml`, which is false for markdown targets and for `.html` under `--markdown`, so a markdown target keeps the markdown convention of opening the Contents tab. The way back is therefore a **Back control in the header** (`data-html-back`, leftmost of the HTML surface controls, before Refresh/eye/pen), rendered only while a linked HTML document is open and named after the document it returns to ("Back to index.html"; `useLinkedDoc` keeps one root snapshot rather than a stack, so Back from any depth lands on the session's root). It deliberately claims **no keyboard shortcut** — `Alt`+`Left` and the browser's own Back belong to the user — and the compact touch shell offers the same action in its Options menu. The sidebar's "Viewing / Back to …" header still works for anyone who opens the sidebar, and `TableOfContents` now renders it even when the document has no headings at all, the normal case for raw HTML. The chrome restore-on-entry effect does not re-run on html→html navigation (`shouldRestoreHtmlChrome` in `packages/ui/utils/htmlChrome.ts` is false whenever the previous surface was HTML), so following a link cannot slam the tools back to hidden or re-apply a remembered sidebar state mid-session. **Armed pinpoint clicks still annotate**: navigation is suppressed in both states, but only Interact follows the link (`Esc`, the header pen, or `Mod+Shift+A` gets there). Live app sessions (`mode: "annotate-app"`) are excluded outright — the interceptor lives inside the bridge's `!LIVE` guard — because they navigate the proxied app for real. Known limitation, pre-existing and unchanged for LINKED documents (`/api/doc` mints a token per HTML file's own directory and refuses `..`, so `sub/page.html` opened as a linked document loads assets below it but not ones it reaches with `../`); **embedded** documents are not affected, because they load from the embedding page's token root — see the next paragraph. +**Links between local HTML documents.** A srcdoc document has no URL of its own — its base URL is the PARENT page's, which is the Plannotator server — so an ordinary `` used to resolve onto the server, hit the catch-all, and render the whole editor inside the annotated frame; an in-page `#section` link navigated for the same reason. The bridge therefore **never lets the srcdoc frame navigate itself**: a capture-phase click handler registered before the pinpoint handler (and never stopping propagation, so armed clicks still pin the link element) `preventDefault`s every link click, scrolls in-page `#fragment`s locally, and posts the RAW href to the parent as `link-click`. `javascript:` hrefs are left to the page. The parent is the trust boundary: `parseBridgeMessage` bounds the href (2048 chars, no control characters) and `resolveHtmlLinkIntent` (`packages/ui/utils/htmlLinkNavigation.ts`, pure) decides what it means — a relative or `../` path resolves against the CURRENT document's directory and opens as a **linked document** through `/api/doc` exactly like a relative markdown link, a root-relative path (and the same path spelled with the server's own origin, the shape an author writes as `http://localhost:/01-entry-point.html`) resolves against the directory the session was opened from, another origin opens in a **new tab** (`noopener,noreferrer`, the frame never navigates), a link to a local **image** (`png jpg jpeg gif webp svg avif bmp ico apng`, the set code review previews, `isReviewImagePath`) opens in the shared `ImageLightbox` (`packages/ui/components/ImageLightbox.tsx`, the same overlay the markdown Viewer uses), read through the page's OWN `/api/html-assets//…` route taken from the `` the server installed (`htmlAssetRouteFromDocument`) so nothing becomes readable that the page could not already load, and only when the image resolves inside the current page's directory (one reached by `../` or a root-relative path outside it toasts "Images open only from this page's own folder"; a page with no asset route, such as a share link, toasts too). The lightbox shows the image only through `` (an SVG never runs), takes focus when it opens (pulling it out of the iframe), and swallows every key on the window's capture phase while open, so `Esc` closes it before the HTML Esc ladder runs and `Mod+Enter` cannot submit a decision behind it. Any other local file outside the annotatable set (`.pdf`, `.zip`) raises a toast and is never fetched, and every other scheme (`data:`, `file:`, `mailto:`, `blob:`) is dropped. The query string is stripped and the fragment is kept: it rides to the new document as `HtmlViewer`'s `initialFragment` and is replayed once that document's bridge is ready (`scroll-to-fragment`). Folder sessions route the open through the file-browser selection handler so the active file, the sidebar and the linked doc stay in step. Annotations stay per document (the `useLinkedDoc` cache), and the Interact/Annotate state and the chrome rules keep applying. **Navigating between HTML documents never opens the sidebar**: the page owns the viewport on this surface (it opens with the tools hidden and the sidebar closed), so a link click leaves the sidebar exactly as the reviewer had it — closed stays closed, open stays on its tab. `useLinkedDoc.open` / `openLoaded` take `revealSidebar` for this (default `true`, so every markdown caller is unchanged), and App passes `false` only when the target renders as HTML — `resolveHtmlLinkIntent`'s `rendersHtml`, which is false for markdown targets and for `.html` under `--markdown`, so a markdown target keeps the markdown convention of opening the Contents tab. The way back is therefore a **Back control in the header** (`data-html-back`, leftmost of the HTML surface controls, before Refresh/eye/pen), rendered only while a linked HTML document is open and named after the document it returns to ("Back to index.html"; `useLinkedDoc` keeps one root snapshot rather than a stack, so Back from any depth lands on the session's root). It deliberately claims **no keyboard shortcut** — `Alt`+`Left` and the browser's own Back belong to the user — and the compact touch shell offers the same action in its Options menu. The sidebar's "Viewing / Back to …" header still works for anyone who opens the sidebar, and `TableOfContents` now renders it even when the document has no headings at all, the normal case for raw HTML. The chrome restore-on-entry effect does not re-run on html→html navigation (`shouldRestoreHtmlChrome` in `packages/ui/utils/htmlChrome.ts` is false whenever the previous surface was HTML), so following a link cannot slam the tools back to hidden or re-apply a remembered sidebar state mid-session. **Armed pinpoint clicks still annotate**: navigation is suppressed in both states, but only Interact follows the link (`Esc`, the header pen, or `Mod+Shift+A` gets there); image links follow the same rule, so a click on a thumbnail pins it while armed and opens the lightbox in Interact. Live app sessions (`mode: "annotate-app"`) are excluded outright — the interceptor lives inside the bridge's `!LIVE` guard — because they navigate the proxied app for real. Known limitation, pre-existing and unchanged for LINKED documents (`/api/doc` mints a token per HTML file's own directory and refuses `..`, so `sub/page.html` opened as a linked document loads assets below it but not ones it reaches with `../`); **embedded** documents are not affected, because they load from the embedding page's token root — see the next paragraph. **Embedded local documents.** The same "a srcdoc has no URL of its own" fact broke `