diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index 60c3553ee3..45cd070c34 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -199,6 +199,13 @@ const preview: Preview = { styles: { width: "1280px", height: "800px" }, type: "mobile", }, + wide: { + // Wide enough to trigger the @container query that reveals the + // sticky plan TOC next to a centered max-w-4xl transcript. + name: "Desktop wide", + styles: { width: "1600px", height: "900px" }, + type: "desktop", + }, }, }, chromatic: { diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 670ee8bbde..62b7ed1aab 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -228,6 +228,7 @@ Freely make breaking changes, and reorganize / cleanup IPC as needed. - **Use conditional rendering for testability:** Components like `AgentModePicker` use `{isOpen &&
...}` instead of Radix Portal. This renders inline and works in happy-dom. - When adding new dropdown/popover components that need tests/ui coverage, prefer the conditional rendering pattern over Radix Portal. - E2E tests (tests/e2e) work with Radix but are slow (~2min startup); reserve for scenarios that truly need real Electron. +- **Storybook responsive/Chromatic validation:** Do not prove responsive snapshots by only resizing `iframe.html`; that bypasses Storybook/Chromatic viewport mode configuration. If a story depends on a breakpoint (wide gutters, mobile, touch), pin an explicit `parameters.chromatic.modes[*].viewport`, mirror it with story `globals.viewport` for local viewing, and validate through the Storybook manager or an equivalent Chromatic-mode check. Add a play/static contract when a missing mode would silently snapshot the wrong UI. - Only use `validateApiKeys()` in tests that actually make AI API calls. ## Tool: todo_write diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index b907b7e74d..2679a4ae17 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -1065,11 +1065,19 @@ const ChatPaneContent: React.FC = (props) => { // In manual reading mode, anchoring should preserve the user's viewport // when async highlights/diagrams above the fold settle. style={autoScroll ? AUTO_SCROLL_TRANSCRIPT_STYLE : undefined} - className="h-full overflow-x-hidden overflow-y-auto p-[15px] leading-[1.5] break-words whitespace-pre-wrap" + // The named `transcript` container is what the sticky plan TOC queries + // for visibility — using a container query rather than a viewport media + // query means sidebars opening/closing correctly hide the TOC even when + // the viewport width is unchanged. See `.plan-toc-aside` in globals.css. + className="@container/transcript h-full overflow-x-hidden overflow-y-auto p-[15px] leading-[1.5] break-words whitespace-pre-wrap" >
diff --git a/src/browser/features/Tools/ProposePlan/PlanTableOfContents.test.tsx b/src/browser/features/Tools/ProposePlan/PlanTableOfContents.test.tsx new file mode 100644 index 0000000000..b98963b636 --- /dev/null +++ b/src/browser/features/Tools/ProposePlan/PlanTableOfContents.test.tsx @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { useRef } from "react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; + +import { installDom } from "../../../../../tests/ui/dom"; +import { PlanTableOfContents } from "./PlanTableOfContents"; +import type { PlanHeading } from "./extractPlanHeadings"; + +/** + * Renders the PlanTableOfContents against a hand-rolled DOM that contains real + *

/

/

elements. We test the click → scrollIntoView wiring here + * (instead of inside ProposePlanToolCall.test.tsx) because sibling test files + * mock MarkdownCore at file scope; those mocks make Streamdown render plain text + * without heading tags, which would break index-based DOM lookups. + */ +function HarnessRenderer({ entries, title }: { entries: PlanHeading[]; title?: string }) { + const ref = useRef(null); + return ( +
+ +
+

Intro

+

...

+

First section

+

...

+

Second section

+

...

+

Nested

+
+
+ ); +} + +describe("PlanTableOfContents", () => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("returns null when there are fewer than two visible (h2-h4) entries", () => { + // A lone h1 produces zero list entries (h1 is reserved for the heading) + // and would leave a TOC with only a title — useless for navigation. + const view = render(); + expect(view.queryByTestId("plan-toc")).toBeNull(); + }); + + test("ignores deeply nested headings (h5/h6) for display", () => { + const view = render( + + ); + // Both entries are filtered out as too-deep, leaving 0 visible — TOC hidden. + expect(view.queryByTestId("plan-toc")).toBeNull(); + }); + + test("filters h1 entries out of the navigation list (they live in the heading)", () => { + const entries: PlanHeading[] = [ + { renderIndex: 0, level: 1, text: "Intro" }, + { renderIndex: 1, level: 2, text: "First section" }, + { renderIndex: 2, level: 2, text: "Second section" }, + ]; + + const view = render(); + + // h2 entries remain as navigable buttons; h1 does not. + expect(view.getByRole("button", { name: "First section" })).toBeDefined(); + expect(view.getByRole("button", { name: "Second section" })).toBeDefined(); + expect(view.queryByRole("button", { name: "Intro" })).toBeNull(); + }); + + test("renders the supplied title as the TOC heading", () => { + const entries: PlanHeading[] = [ + { renderIndex: 0, level: 2, text: "A" }, + { renderIndex: 1, level: 2, text: "B" }, + ]; + + const view = render(); + const toc = view.getByTestId("plan-toc"); + expect(toc.textContent).toContain("My Plan Title"); + // Without any h1 in entries, the heading should NOT be a clickable button. + expect(view.queryByRole("button", { name: "My Plan Title" })).toBeNull(); + }); + + test("clicking the TOC heading scrolls to the plan's h1 when one exists", () => { + const entries: PlanHeading[] = [ + { renderIndex: 0, level: 1, text: "Plan Title From Markdown" }, + { renderIndex: 1, level: 2, text: "First section" }, + { renderIndex: 2, level: 2, text: "Second section" }, + ]; + + const view = render(); + + const headings = Array.from(view.container.querySelectorAll("h1, h2, h3")); + const scrollCalls: Array<{ target: HTMLElement; options: ScrollIntoViewOptions }> = []; + for (const heading of headings) { + heading.scrollIntoView = ((options: ScrollIntoViewOptions) => { + scrollCalls.push({ target: heading, options }); + }) as HTMLElement["scrollIntoView"]; + } + + fireEvent.click(view.getByRole("button", { name: "Plan Title From Markdown" })); + expect(scrollCalls).toHaveLength(1); + expect(scrollCalls[0].target).toBe(headings[0]); // the h1 + }); + + test("scrolls the correct rendered heading into view when an entry is clicked", () => { + const entries: PlanHeading[] = [ + { renderIndex: 0, level: 1, text: "Intro" }, + { renderIndex: 1, level: 2, text: "First section" }, + { renderIndex: 2, level: 2, text: "Second section" }, + { renderIndex: 3, level: 3, text: "Nested" }, + ]; + + const view = render(); + + const headings = Array.from(view.container.querySelectorAll("h1, h2, h3")); + expect(headings).toHaveLength(4); + + const scrollCalls: Array<{ target: HTMLElement; options: ScrollIntoViewOptions }> = []; + for (const heading of headings) { + heading.scrollIntoView = ((options: ScrollIntoViewOptions) => { + scrollCalls.push({ target: heading, options }); + }) as HTMLElement["scrollIntoView"]; + } + + fireEvent.click(view.getByRole("button", { name: "Second section" })); + + expect(scrollCalls).toHaveLength(1); + expect(scrollCalls[0].target).toBe(headings[2]); + // Top alignment ensures the heading lands just below the scrollport edge. + expect(scrollCalls[0].options.block).toBe("start"); + }); + + test("normalizes indentation so the shallowest visible level sits at column 0", () => { + // With h1 reserved for the heading, h2 (the shallowest visible) should + // render at data-level=1, and h3 at data-level=2. + const view = render( + + ); + + const items = view.container.querySelectorAll(".plan-toc-item"); + expect(items).toHaveLength(2); + expect(items[0].dataset.level).toBe("1"); + expect(items[1].dataset.level).toBe("2"); + }); +}); diff --git a/src/browser/features/Tools/ProposePlan/PlanTableOfContents.tsx b/src/browser/features/Tools/ProposePlan/PlanTableOfContents.tsx new file mode 100644 index 0000000000..79e8c9f703 --- /dev/null +++ b/src/browser/features/Tools/ProposePlan/PlanTableOfContents.tsx @@ -0,0 +1,142 @@ +import React from "react"; +import { cn } from "@/common/lib/utils"; +import { TooltipIfPresent } from "@/browser/components/Tooltip/Tooltip"; +import type { PlanHeading } from "./extractPlanHeadings"; + +/** + * Sticky table of contents rendered alongside a plan in the chat transcript. + * + * Layout: + * - At intermediate widths the left gutter shows only sideways hint text so + * users can discover that widening the transcript reveals the TOC. + * - When the transcript container is wide enough, CSS reveals the same sticky + * left-gutter nav (see `.plan-toc-aside` in globals.css). The sticky container + * follows the user while the plan is on screen, then naturally scrolls away + * once the plan exits the viewport. + * + * Heading navigation is index-based: each entry stores its position among ALL + * h1..h6 elements the plan renders, so clicking a TOC item does + * `container.querySelectorAll("h1,h2,h3,h4,h5,h6")[renderIndex].scrollIntoView()`. + * This avoids touching the shared markdown rehype pipeline just for plan TOC. + */ +export interface PlanTableOfContentsProps { + /** Heading entries extracted from the plan markdown, in document order. */ + entries: PlanHeading[]; + /** + * Ref to a DOM container whose subtree owns the rendered plan headings. + * Must be a stable ancestor of the markdown output for `scrollIntoView` to + * locate the right element. + */ + contentRef: React.RefObject; + /** + * Heading rendered at the top of the TOC. Defaults to "Contents". + * + * The plan title is the natural label for a plan TOC, so we let the host + * surface it here — that conserves vertical space (no separate "CONTENTS" + * label *and* an h1 list entry) and tightens the visual hierarchy: the + * title sits at column 0, h2 entries align with it, and h3+ are indented + * under their parent h2. + */ + title?: string; + className?: string; +} + +const HEADING_SELECTOR = "h1, h2, h3, h4, h5, h6"; +const DEFAULT_HEADING = "Contents"; + +export const PlanTableOfContents: React.FC = (props) => { + // h1 is reserved for the TOC's heading (the plan title), so it never appears + // as a list entry — but it still consumes a renderIndex because the rendered + // DOM still contains an

. h5/h6 are also hidden as visual noise. + const visibleEntries = props.entries.filter((entry) => entry.level >= 2 && entry.level <= 4); + if (visibleEntries.length < 2) { + // A TOC with 0 or 1 entries adds visual chrome without navigation value. + // (Note: this check intentionally excludes the title, since the title is + // a single label, not a navigable destination on its own.) + return null; + } + + // Normalize indentation: anchor the shallowest visible level at column 0 so + // a plan that starts at "###" doesn't look uniformly indented. + const minLevel = Math.min(...visibleEntries.map((entry) => entry.level)); + + const handleNavigate = (renderIndex: number) => { + const container = props.contentRef.current; + if (!container) return; + const headings = container.querySelectorAll(HEADING_SELECTOR); + const target = headings.item(renderIndex); + // `scrollIntoView` is not implemented by happy-dom in unit tests; the guard + // keeps tests from crashing while still exercising the lookup path. + if (target && typeof target.scrollIntoView === "function") { + target.scrollIntoView({ block: "start" }); + } + }; + + // Use the supplied title when non-blank; fall back to "Contents" otherwise. + // Treat " " as blank — a whitespace-only title would render as an empty + // heading line and look broken. + const trimmedTitle = props.title?.trim(); + const headingText = trimmedTitle && trimmedTitle.length > 0 ? trimmedTitle : DEFAULT_HEADING; + // If the plan's source markdown begins with an h1, clicking the TOC heading + // jumps to that h1 (the natural "top of plan" target). Otherwise the heading + // is a static label — there's nothing meaningful to scroll to. + const titleHeadingEntry = props.entries.find((entry) => entry.level === 1); + + return ( + + ); +}; diff --git a/src/browser/features/Tools/ProposePlan/ProposePlanToolCall.stories.tsx b/src/browser/features/Tools/ProposePlan/ProposePlanToolCall.stories.tsx index be85905d3d..2c4143b57f 100644 --- a/src/browser/features/Tools/ProposePlan/ProposePlanToolCall.stories.tsx +++ b/src/browser/features/Tools/ProposePlan/ProposePlanToolCall.stories.tsx @@ -1,3 +1,5 @@ +import { waitFor, within } from "@storybook/test"; + import type { AppStory } from "@/browser/stories/meta.js"; import { appMeta, AppWithMocks } from "@/browser/stories/meta.js"; import { setupSimpleChatStory } from "@/browser/stories/helpers/chatSetup"; @@ -8,6 +10,20 @@ import { STABLE_TIMESTAMP } from "@/browser/stories/mocks/workspaces"; const meta = { ...appMeta, title: "App/Chat/Tools/ProposePlan" }; export default meta; +const PLAN_TOC_CHROMATIC_VIEWPORT = { width: 1600, height: 900 } as const; +const PLAN_TOC_CHROMATIC_MODES = { + "dark-wide": { theme: "dark", viewport: PLAN_TOC_CHROMATIC_VIEWPORT }, +} as const; + +function isChromaticRuntime(): boolean { + if (typeof window === "undefined") { + return false; + } + + const chromaticRuntimeFlag = (window as Window & { chromatic?: boolean }).chromatic; + return /Chromatic/i.test(window.navigator.userAgent) || chromaticRuntimeFlag === true; +} + /** * Story showing a propose_plan tool call with Plan UI. * Tests the plan card rendering with icon action buttons at the bottom. @@ -180,6 +196,129 @@ export const ProposePlanMobile: AppStory = { }, }; +/** + * Wide-viewport story that exercises the sticky plan TOC. + * + * The TOC lives in an absolutely-positioned `