diff --git a/webviews/src/agent-session/acpmux/threadMinimap/PreviewCard.tsx b/webviews/src/agent-session/acpmux/threadMinimap/PreviewCard.tsx index 00b541af2619..70791719d710 100644 --- a/webviews/src/agent-session/acpmux/threadMinimap/PreviewCard.tsx +++ b/webviews/src/agent-session/acpmux/threadMinimap/PreviewCard.tsx @@ -1,18 +1,56 @@ // The minimap's preview card beside the hovered or focused mark: the prompt on one line with its // bookmark, then the start of the reply. Both rails (ThreadMinimap.tsx, ScrollMap.tsx) draw it. -import React from "react"; +// +// The card draws in the transcript scroller's parent, not in the rail. The scroller masks its soft +// edges (conversation.css), and a masked ancestor is the card's backdrop root: inside it the blur +// had nothing under it to read, so the rows showed through the tint sharp (#23782). From the +// parent the card blurs the rows, as the chat's other glass does. An anchor left in the rail marks +// where the card's x and y count from, so it lands where it always did. The wheel over the card and +// a scrolling key on its bookmark still scroll the transcript, and reach the scroller's own +// listeners first (wheelPassThrough.ts), as they did when the card was inside it. +import React, { useCallback, useLayoutEffect, useRef, useState } from "react"; +import { createPortal } from "react-dom"; import { useT } from "../i18n"; +import { forwardWheel } from "../wheelPassThrough"; import { BookmarkIcon } from "./BookmarkIcon"; import type { ReplyBlock } from "./model"; +/// How far a key scrolls the transcript while focus is in it: a line, most of a page, or to an end. +function keyScroll(key: string, scroller: HTMLElement): number | undefined { + const page = scroller.clientHeight * 0.875; + if (key === "ArrowUp") return -40; + if (key === "ArrowDown") return 40; + if (key === "PageUp") return -page; + if (key === "PageDown") return page; + if (key === "Home") return -scroller.scrollTop; + if (key === "End") return scroller.scrollHeight; + return undefined; +} + +/// A scrolling key on the card's bookmark (its one stop) scrolls the transcript, as it did when the +/// card drew inside it. The scroller gets a copy first, so its own key listeners run as for a key in +/// it (the scroll counts as the reader's, a jump in flight ends). +function forwardScrollKey(scroller: HTMLElement, event: React.KeyboardEvent): void { + if (event.defaultPrevented || event.altKey || event.ctrlKey || event.metaKey) return; + const by = keyScroll(event.key, scroller); + if (by === undefined) return; + event.preventDefault(); + scroller.dispatchEvent(new KeyboardEvent("keydown", { key: event.key, bubbles: false })); + scroller.scrollTop += by; +} + export type PreviewBookmark = { marked: boolean; toggle(): void }; export const PreviewCard = React.forwardRef< HTMLDivElement, { id: string; + /// From the rail's origin (`.acpmux-minimap`), wherever the card draws. x: number; y: number; + /// The transcript scroller: the card draws in its parent, outside its mask. Without one the card + /// draws in place. + scroller?: HTMLElement | null; /// The prompt's line; a reply's card has none. prompt?: string; reply: readonly ReplyBlock[]; @@ -21,15 +59,44 @@ export const PreviewCard = React.forwardRef< onFocus?(): void; onBlur?(event: React.FocusEvent): void; } ->(function PreviewCard({ id, x, y, prompt, reply, bookmark, onPointerLeave, onFocus, onBlur }, ref) { +>(function PreviewCard({ id, x, y, scroller, prompt, reply, bookmark, onPointerLeave, onFocus, onBlur }, ref) { const t = useT(); - return ( + const layer = scroller?.parentElement; + const anchor = useRef(null); + const self = useRef(null); + /// The rail's origin in the card's containing block, measured after each commit (before paint). + const [origin, setOrigin] = useState({ x: 0, y: 0 }); + // oxlint-disable-next-line react-hooks/exhaustive-deps -- runs after every commit on purpose (the rail can move under the card with any render); its setState is guarded by the comparison. + useLayoutEffect(() => { + const from = anchor.current; + const node = self.current; + if (!layer || !from || !node) return; + const base = node.offsetParent instanceof HTMLElement ? node.offsetParent : layer; + const mark = from.getBoundingClientRect(); + const frame = base.getBoundingClientRect(); + const next = { + x: mark.left - frame.left - base.clientLeft + base.scrollLeft, + y: mark.top - frame.top - base.clientTop + base.scrollTop, + }; + if (next.x !== origin.x || next.y !== origin.y) setOrigin(next); + }); + const own = useCallback( + (node: HTMLDivElement | null) => { + self.current = node; + if (typeof ref === "function") ref(node); + else if (ref) ref.current = node; + }, + [ref], + ); + const at = layer ? origin : { x: 0, y: 0 }; + const card = (
forwardWheel(scroller!, event.nativeEvent) : undefined} onFocus={onFocus} onBlur={onBlur} > @@ -44,6 +111,7 @@ export const PreviewCard = React.forwardRef< aria-label={t(bookmark.marked ? "minimap.bookmarkRemove" : "minimap.bookmarkAdd")} title={t(bookmark.marked ? "minimap.bookmarkRemove" : "minimap.bookmarkAdd")} onClick={bookmark.toggle} + onKeyDown={layer ? (event) => forwardScrollKey(scroller!, event) : undefined} > @@ -63,4 +131,11 @@ export const PreviewCard = React.forwardRef< )}
); + if (!layer) return card; + return ( + <> + + {createPortal(card, layer)} + + ); }); diff --git a/webviews/src/agent-session/acpmux/threadMinimap/ScrollMap.tsx b/webviews/src/agent-session/acpmux/threadMinimap/ScrollMap.tsx index 2a23181f8c01..20028136a099 100644 --- a/webviews/src/agent-session/acpmux/threadMinimap/ScrollMap.tsx +++ b/webviews/src/agent-session/acpmux/threadMinimap/ScrollMap.tsx @@ -344,6 +344,8 @@ export function ScrollMap({ id={cardId} x={RAIL_LEFT + ROW_WIDTH} y={cardOffset(MAP_MARGIN + cardAt.y, cardHeight, viewportHeight)} + // Out of the scroller: its edge mask would keep the card's blur from the rows (PreviewCard.tsx). + scroller={scroller.current} prompt={cardTurn?.prompt} reply={cardTurn ? cardTurn.reply : agentReply} bookmark={ diff --git a/webviews/src/agent-session/acpmux/threadMinimap/ThreadMinimap.tsx b/webviews/src/agent-session/acpmux/threadMinimap/ThreadMinimap.tsx index b8a25704f1f3..5e476190c1c6 100644 --- a/webviews/src/agent-session/acpmux/threadMinimap/ThreadMinimap.tsx +++ b/webviews/src/agent-session/acpmux/threadMinimap/ThreadMinimap.tsx @@ -615,6 +615,8 @@ export function ThreadMinimap({ id={cardId} x={RAIL_LEFT + ROW_WIDTH} y={cardTop} + // Out of the scroller: its edge mask would keep the card's blur from the rows (PreviewCard.tsx). + scroller={scroller.current} prompt={cardTurn.prompt} reply={cardTurn.reply} bookmark={marks.can(cardTurn) ? { marked: cardMarked, toggle: () => marks.toggle(cardTurn) } : undefined} diff --git a/webviews/src/agent-session/acpmux/threadMinimap/threadMinimap.css b/webviews/src/agent-session/acpmux/threadMinimap/threadMinimap.css index 155ba2888c14..7bbc35ec6f83 100644 --- a/webviews/src/agent-session/acpmux/threadMinimap/threadMinimap.css +++ b/webviews/src/agent-session/acpmux/threadMinimap/threadMinimap.css @@ -7,6 +7,10 @@ top: 0; z-index: 3; height: 0; +} +/* The rail's tokens; the card takes them too, as it draws outside the rail (PreviewCard.tsx). */ +.acpmux-minimap, +.acpmux-minimap__card { /* Mixed into the opaque base (styles.css --acpmux-base), as the menus are: the page color can be clear. */ --acpmux-minimap-base: var(--acpmux-base, var(--agent-page-bg)); --acpmux-minimap-rest: var(--agent-muted); @@ -17,7 +21,8 @@ --acpmux-minimap-spring: linear(0, 0.398 10%, 0.682 20%, 0.843 30%, 0.925 40%, 0.972 50%, 1.004 60%, 1.008 70%, 1.003 80%, 1); } /* appearance.borders none clears the popover's rim in place, as it clears the popups'. */ -:root[data-borders="none"] .acpmux-minimap { +:root[data-borders="none"] .acpmux-minimap, +:root[data-borders="none"] .acpmux-minimap__card { --acpmux-minimap-rim: transparent; } .acpmux-minimap__rail { @@ -152,10 +157,24 @@ width: 9px; height: 10px; } +/* Where the card's x and y count from: the rail's origin, as the card draws in the transcript + scroller's parent (PreviewCard.tsx). */ +.acpmux-minimap__anchor { + position: absolute; + top: 0; + left: 0; + width: 0; + height: 0; + pointer-events: none; +} +/* Drawn in the transcript scroller's parent, never inside the scroller: the scroller's edge mask + (conversation.css) is a backdrop root, and under it the blur below shows the rows sharp. On the + tooltip layer, as the chat's other hover cards, so the composer under it never covers it. */ .acpmux-minimap__card { position: absolute; top: 0; left: 0; + z-index: var(--layer-tooltip); width: 320px; max-width: calc(100vw - 16px); box-sizing: border-box; diff --git a/webviews/src/agent-session/acpmux/wheelPassThrough.ts b/webviews/src/agent-session/acpmux/wheelPassThrough.ts index 3a8c77f5bea5..21b3ed7a49f8 100644 --- a/webviews/src/agent-session/acpmux/wheelPassThrough.ts +++ b/webviews/src/agent-session/acpmux/wheelPassThrough.ts @@ -26,20 +26,27 @@ function passesThrough(target: Element): boolean { return target.scrollHeight <= target.clientHeight; } +/// Scrolls `scroller` by a vertical wheel that landed outside it. The scroller first gets a copy of +/// the event, so its own wheel listeners run as for a wheel over it (the scroll counts as the +/// reader's, a glide, a minimap jump or a Jump to latest in flight ends). A pinch (ctrl) or a mostly +/// sideways wheel is left alone. +export function forwardWheel(scroller: HTMLElement, event: WheelEvent): void { + if (event.defaultPrevented || event.ctrlKey) return; + if (Math.abs(event.deltaY) <= Math.abs(event.deltaX)) return; + scroller.dispatchEvent( + new WheelEvent("wheel", { deltaX: 0, deltaY: event.deltaY, deltaMode: event.deltaMode, bubbles: false }), + ); + scroller.scrollTop += wheelPixels(event, scroller.clientHeight); +} + /// Sends the wheel over Jump to latest, or over a floating stack part's empty room, in `main` to -/// `scroller`. The scroller first gets a copy of the event, so its own wheel listeners run as for a -/// wheel over it (the scroll counts as the reader's, a glide or a Jump to latest in flight ends). The -/// listener is passive: a wheel over the transcript itself never waits on it. Returns the remover. +/// `scroller` (forwardWheel). The listener is passive: a wheel over the transcript itself never +/// waits on it. Returns the remover. export function installWheelPassThrough(scroller: HTMLElement, main: HTMLElement): () => void { const onWheel = (event: WheelEvent) => { - if (event.defaultPrevented || event.ctrlKey) return; - if (Math.abs(event.deltaY) <= Math.abs(event.deltaX)) return; const target = event.target instanceof Element ? event.target : null; if (!target || !passesThrough(target) || scroller.contains(target)) return; - scroller.dispatchEvent( - new WheelEvent("wheel", { deltaX: 0, deltaY: event.deltaY, deltaMode: event.deltaMode, bubbles: false }), - ); - scroller.scrollTop += wheelPixels(event, scroller.clientHeight); + forwardWheel(scroller, event); }; main.addEventListener("wheel", onWheel, { passive: true }); return () => main.removeEventListener("wheel", onWheel); diff --git a/webviews/test/thread-minimap-rail.test.tsx b/webviews/test/thread-minimap-rail.test.tsx index 3e6b735989b6..1b1797c92753 100644 --- a/webviews/test/thread-minimap-rail.test.tsx +++ b/webviews/test/thread-minimap-rail.test.tsx @@ -40,6 +40,8 @@ const replacements: Record = { unobserve() {} disconnect() {} }, + WheelEvent: dom.window.WheelEvent, + KeyboardEvent: dom.window.KeyboardEvent, requestAnimationFrame: dom.window.requestAnimationFrame.bind(dom.window), cancelAnimationFrame: dom.window.cancelAnimationFrame.bind(dom.window), IS_REACT_ACT_ENVIRONMENT: true, @@ -416,3 +418,87 @@ test("Next and Previous Bookmarked Prompt scroll to the bookmarked prompts after await view.unmount(); } }); + +test("the card draws outside the transcript scroller, whose edge mask would keep its blur from the rows", async () => { + const view = await mount(8); + try { + await act(async () => tick(3).focus()); + expect(card()?.textContent).toContain("Question 3"); + // The scroller masks its soft edges (conversation.css), and a masked ancestor is the card's + // backdrop root: inside it the glass showed the rows sharp through its tint (#23782). + expect(card()!.closest(".acpmux-scroll")).toBeNull(); + expect(card()!.parentElement).toBe(view.scroller.parentElement); + // The rail keeps an anchor where the card's x and y count from. + expect(view.scroller.querySelector(".acpmux-minimap > .acpmux-minimap__anchor")).not.toBeNull(); + } finally { + await view.unmount(); + } +}); + +test("the wheel and the scrolling keys over the card reach the transcript's own listeners, as inside it", async () => { + const { followTrace } = await import("../src/agent-session/acpmux/followTrace"); + const { animateScroll } = await import("../src/agent-session/acpmux/threadMinimap/scrollMotion"); + const view = await mount(8); + const input = jest.spyOn(followTrace, "input"); + try { + await act(async () => tick(3).focus()); + expect(card()?.textContent).toContain("Question 3"); + // jsdom has no scrollBy: a card that only called it (and skipped the listeners) would scroll here. + view.scroller.scrollBy = ((options: ScrollToOptions) => { + view.scroller.scrollTop += options.top ?? 0; + }) as typeof view.scroller.scrollBy; + const heard: string[] = []; + view.scroller.addEventListener("wheel", (event) => heard.push(`wheel ${(event as WheelEvent).deltaY}`)); + view.scroller.addEventListener("keydown", (event) => heard.push(`key ${(event as KeyboardEvent).key}`)); + // A minimap jump in flight: the reader's own wheel over the card ends it where it is. + view.scroller.scrollTop = 1000; + animateScroll(view.scroller, () => 5000, 60_000, [0, 0, 1, 1]); + input.mockClear(); + await act(async () => { + card()!.dispatchEvent(new dom.window.WheelEvent("wheel", { deltaY: -3, bubbles: true, cancelable: true })); + }); + // The transcript's own wheel listeners ran: the scroll is the reader's (App.tsx), so a slow + // scroll up from the latest row is not taken for a stray and snapped back. + expect(heard).toEqual(["wheel -3"]); + expect(input).toHaveBeenCalledTimes(1); + // The jump ended where it was: the frames that follow move the transcript no further. + const after = view.scroller.scrollTop; + for (let frame = 0; frame < 4; frame += 1) await new Promise((resolve) => requestAnimationFrame(resolve)); + expect(view.scroller.scrollTop).toBe(after); + // A page (deltaMode 2) is the scroller's height. + view.scroller.scrollTop = 1000; + await act(async () => { + card()!.dispatchEvent( + new dom.window.WheelEvent("wheel", { deltaY: 1, deltaMode: 2, bubbles: true, cancelable: true }), + ); + }); + expect(view.scroller.scrollTop).toBe(1000 + VIEWPORT); + // A pinch (ctrl) is not a scroll. + await act(async () => { + card()!.dispatchEvent(new dom.window.WheelEvent("wheel", { deltaY: 40, ctrlKey: true, bubbles: true })); + }); + expect(view.scroller.scrollTop).toBe(1000 + VIEWPORT); + // Page Down with focus on the card's bookmark pages the transcript, as it did inside it. + heard.length = 0; + input.mockClear(); + const bookmark = card()!.querySelector(".acpmux-minimap__bookmark")!; + await act(async () => { + bookmark.dispatchEvent( + new dom.window.KeyboardEvent("keydown", { key: "PageDown", bubbles: true, cancelable: true }), + ); + }); + expect(heard).toEqual(["key PageDown"]); + expect(input).toHaveBeenCalledTimes(1); + expect(view.scroller.scrollTop).toBe(1000 + VIEWPORT + VIEWPORT * 0.875); + // Return on the bookmark is its own: it scrolls nothing. + await act(async () => { + bookmark.dispatchEvent( + new dom.window.KeyboardEvent("keydown", { key: "Enter", bubbles: true, cancelable: true }), + ); + }); + expect(heard).toEqual(["key PageDown"]); + } finally { + input.mockRestore(); + await view.unmount(); + } +}); diff --git a/webviews/test/thread-scroll-map-rail.test.tsx b/webviews/test/thread-scroll-map-rail.test.tsx index d6eb7f367f23..7c7ab6b0d797 100644 --- a/webviews/test/thread-scroll-map-rail.test.tsx +++ b/webviews/test/thread-scroll-map-rail.test.tsx @@ -250,3 +250,17 @@ test("the previewed prompt stays lit while focus is on its card", async () => { await view.unmount(); } }); + +test("the card draws outside the transcript scroller, whose edge mask would keep its blur from the rows", async () => { + const view = await mount(12); + try { + await act(async () => tick(6).focus()); + expect(card()?.textContent).toContain("Question 6"); + // A masked ancestor is the card's backdrop root (conversation.css's soft edges, #23782). + expect(card()!.closest(".acpmux-scroll")).toBeNull(); + expect(card()!.parentElement).toBe(view.scroller.parentElement); + expect(view.scroller.querySelector(".acpmux-minimap > .acpmux-minimap__anchor")).not.toBeNull(); + } finally { + await view.unmount(); + } +});