diff --git a/apps/web/components/task/changes-panel-header.tsx b/apps/web/components/task/changes-panel-header.tsx index dde133f7c0..f7fd04c2b4 100644 --- a/apps/web/components/task/changes-panel-header.tsx +++ b/apps/web/components/task/changes-panel-header.tsx @@ -514,7 +514,7 @@ function ChangesPanelWalkthroughButton({ : "Walk me through these changes"; return ( - + diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index 94f2829533..14109df91d 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -30,7 +30,7 @@ const ChangesPanel = memo(function ChangesPanel(props: ChangesPanelProps) { }); if (isArchived) return ; return ( - + - + { testPage, apiClient, seedData, + prCapture, }) => { await seedWalkthroughTask(testPage, apiClient, seedData, "walkthrough-setup", "changes ready"); @@ -90,6 +91,24 @@ test.describe("Mobile code walkthrough", () => { await expect(changes).toBeVisible({ timeout: 15_000 }); const request = changes.getByTestId("changes-request-walkthrough"); await expect(request).toBeEnabled({ timeout: 30_000 }); + await expect(request.getByText("Walkthrough", { exact: true })).toBeVisible(); + const [changesBox, requestBox] = await Promise.all([ + changes.boundingBox(), + request.boundingBox(), + ]); + if (!changesBox || !requestBox) throw new Error("Mobile Changes toolbar geometry unavailable"); + expect(requestBox.x).toBeGreaterThanOrEqual(changesBox.x); + expect(requestBox.x + requestBox.width).toBeLessThanOrEqual( + changesBox.x + changesBox.width + 1, + ); + expect( + await testPage.evaluate( + () => document.documentElement.scrollWidth <= document.documentElement.clientWidth, + ), + ).toBe(true); + await prCapture.screenshot("mobile-changes-walkthrough", { + caption: "Mobile Changes keeps the Walkthrough action within the viewport", + }); await request.click(); await testPage.getByRole("button", { name: "Chat" }).click(); diff --git a/apps/web/e2e/tests/review/walkthrough.spec.ts b/apps/web/e2e/tests/review/walkthrough.spec.ts index c69b41fc1f..8383175b75 100644 --- a/apps/web/e2e/tests/review/walkthrough.spec.ts +++ b/apps/web/e2e/tests/review/walkthrough.spec.ts @@ -3,6 +3,7 @@ import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import type { SeedData } from "../../fixtures/test-base"; import type { Page, Locator } from "@playwright/test"; +import { resizeColumnViaSplitview } from "../../helpers/dockview-resize"; import { expectWalkthroughBehindDialog } from "./walkthrough-layering"; async function seedWalkthroughTask( @@ -77,9 +78,66 @@ async function openWalkthrough(testPage: Page): Promise { return card; } +async function expectContainedInPanel(panel: Locator, action: Locator): Promise { + const [panelBox, actionBox] = await Promise.all([panel.boundingBox(), action.boundingBox()]); + if (!panelBox || !actionBox) throw new Error("Changes toolbar geometry unavailable"); + expect(actionBox.x).toBeGreaterThanOrEqual(panelBox.x); + expect(actionBox.x + actionBox.width).toBeLessThanOrEqual(panelBox.x + panelBox.width + 1); + const actionCenterHitsButton = await action.evaluate((element) => { + const rect = element.getBoundingClientRect(); + const hit = document.elementFromPoint(rect.left + rect.width / 2, rect.top + rect.height / 2); + return hit === element || !!hit?.closest('[data-testid="changes-request-walkthrough"]'); + }); + expect(actionCenterHitsButton).toBe(true); +} + test.describe("Code walkthrough", () => { test.describe.configure({ retries: 2, timeout: 120_000 }); + test("adapts the Changes walkthrough label to panel width", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + await testPage.setViewportSize({ width: 1366, height: 900 }); + const session = await seedWalkthroughChangesTask(testPage, apiClient, seedData); + await session.clickTab("Changes"); + + const request = session.changesRequestWalkthroughButton(); + const label = request.getByText("Walkthrough", { exact: true }); + await expect(request).toBeEnabled({ timeout: 30_000 }); + + const narrowWidth = await resizeColumnViaSplitview(testPage, "right", 349); + expect(narrowWidth).toBe(349); + await expect + .poll(async () => Math.round((await session.changes.boundingBox())?.width ?? 0)) + .toBe(349); + await expect(label).toBeHidden(); + await expect(request).toBeVisible(); + await expectContainedInPanel(session.changes, request); + + const labeledWidth = await resizeColumnViaSplitview(testPage, "right", 350); + expect(labeledWidth).toBe(350); + await expect + .poll(async () => Math.round((await session.changes.boundingBox())?.width ?? 0)) + .toBe(350); + await expect(label).toBeVisible(); + await expectContainedInPanel(session.changes, request); + await prCapture.screenshot("desktop-changes-walkthrough", { + caption: "Walkthrough stays fully visible in the Changes toolbar", + }); + + const minimumWidth = await resizeColumnViaSplitview(testPage, "right", 180); + expect(minimumWidth).toBe(180); + await expect + .poll(async () => Math.round((await session.changes.boundingBox())?.width ?? 0)) + .toBe(180); + await expect(label).toBeHidden(); + await expect(request).toBeVisible(); + await expectContainedInPanel(session.changes, request); + }); + test("Changes-panel request asks the agent to walk through current changes", async ({ testPage, apiClient, diff --git a/docs/plans/changes-walkthrough-toolbar-width/plan.md b/docs/plans/changes-walkthrough-toolbar-width/plan.md new file mode 100644 index 0000000000..d47e50a689 --- /dev/null +++ b/docs/plans/changes-walkthrough-toolbar-width/plan.md @@ -0,0 +1,94 @@ +--- +spec: docs/specs/changes-walkthrough-toolbar-width/spec.md +created: 2026-07-31 +status: implemented +--- + +# Implementation Plan: Responsive Changes Walkthrough Action + +## Overview + +Replace the Walkthrough label's viewport breakpoint with a named inline-size +container rooted at the Changes panel. Prove the 349px/350px boundary and +toolbar containment in a real browser before applying the minimal class-name +change, then rerun the existing mobile request flow with responsive geometry +assertions. + +## Confirmed root cause + +`ChangesPanelWalkthroughButton` uses `min-[430px]`, which queries the browser +viewport. The desktop Changes panel is a nested Dockview pane whose width can be +349px while the laptop viewport remains much wider, so the label stays rendered +and `PanelHeaderBarSplit` clips the left slot to preserve its right-side branch +and Pull actions. + +## Frontend + +### Changes panel container + +- Mark the desktop `PanelRoot` in + `apps/web/components/task/changes-panel.tsx` and the phone `PanelRoot` in + `apps/web/components/task/mobile/mobile-changes-panel.tsx` as the same named + inline-size container. +- In `apps/web/components/task/changes-panel-header.tsx`, keep the label hidden + by default and reveal it only when that named panel container is at least + 350px wide. +- Preserve the icon, tooltip, accessible name, click handler, disabled state, + and the existing `PanelHeaderBarSplit` overflow policy. + +### Mobile design contract + +- **Desktop outcome:** the action is icon-only at 349px and narrower, and + icon-plus-label at 350px and wider. +- **Mobile entry point:** the existing **Changes** item in + `SessionMobileBottomNav` opens `MobileChangesPanel`; no navigation changes. +- **Nearest shipped exemplar:** `mobile-changes-panel.tsx` continues to provide + the focused, full-width phone Changes surface and shared action behavior. +- **Hierarchy and primary action:** Diff, Review, Walkthrough, branch, and Pull + remain in the existing inline header; requesting a walkthrough remains the + action's only outcome. +- **Presentation rationale:** this is a compact, frequent toolbar action, so an + inline icon fallback is preferable to adding an overflow drawer or route. +- **Geometry:** scroll ownership, dynamic viewport sizing, safe areas, and touch + targets are unchanged; the test guards panel containment and document-level + horizontal overflow. +- **Shared logic:** request state and handlers remain shared; only responsive + presentation changes. +- **Mobile proof:** extend the existing Pixel 5 walkthrough-request scenario to + assert the visible label fits and the document does not scroll horizontally. + +## Tests + +No unit test is appropriate because jsdom does not evaluate CSS container +queries or rendered geometry. The behavior is covered at the browser level. + +## E2E Tests + +- **Scenario:** at 349px the action is icon-only and contained; at 350px its + label appears and remains contained, even on a wide laptop viewport. + - **File:** `apps/web/e2e/tests/review/walkthrough.spec.ts` + - **Method:** resize the right Dockview column through the existing + `resizeColumnViaSplitview` helper, assert the panel's rendered width, label + visibility, accessible button visibility, and bounding-box containment. +- **Scenario:** the Pixel 5 Changes surface remains usable when its 393px panel + shows the label. + - **File:** `apps/web/e2e/tests/review/mobile-walkthrough.spec.ts` + - **Method:** extend the existing request test with label visibility, + bounding-box containment, and zero document-level horizontal overflow + before requesting the walkthrough. + +## Implementation Tasks + +- [x] [Task 01: Make the Walkthrough action panel-responsive](task-01-panel-responsive-walkthrough.md) + +Execution is sequential in the primary conversation. No subagent delegation is +planned or authorized. + +## Risks + +- Container queries measure the nearest named container's content box. Naming + the `PanelRoot`, rather than the padded header, keeps the 349px/350px contract + aligned with the user-observed panel width. +- Dockview can round resize targets. The E2E regression must poll the rendered + panel width and fail with the observed value rather than masking it with a + broad tolerance. diff --git a/docs/plans/changes-walkthrough-toolbar-width/task-01-panel-responsive-walkthrough.md b/docs/plans/changes-walkthrough-toolbar-width/task-01-panel-responsive-walkthrough.md new file mode 100644 index 0000000000..69d25c1c4b --- /dev/null +++ b/docs/plans/changes-walkthrough-toolbar-width/task-01-panel-responsive-walkthrough.md @@ -0,0 +1,84 @@ +--- +id: "01-panel-responsive-walkthrough" +title: "Make the Walkthrough action panel-responsive" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/changes-walkthrough-toolbar-width/spec.md" +--- + +# Task 01: Make the Walkthrough Action Panel-Responsive + +## Acceptance + +- A 349px Changes panel renders an accessible, fully contained icon-only + Walkthrough action, independent of the viewport width. +- A 350px-or-wider Changes panel renders the complete Walkthrough label without + clipping the action or the branch and Pull controls. +- The Pixel 5 Changes request flow remains usable, contained, and free of + document-level horizontal overflow. + +## TDD sequence + +1. Add the desktop 349px/350px geometry regression and the mobile containment + assertions, then run the focused command and confirm they fail because the + current label follows viewport width. +2. Add the named panel containers and replace the viewport breakpoint with the + 350px container-query boundary. +3. Run the focused desktop and mobile commands until both scenarios pass. +4. Refactor only if the minimal class changes expose duplication or unclear + naming; rerun the focused command after any refactor. + +## Files likely touched + +- `apps/web/components/task/changes-panel.tsx` +- `apps/web/components/task/mobile/mobile-changes-panel.tsx` +- `apps/web/components/task/changes-panel-header.tsx` +- `apps/web/e2e/tests/review/walkthrough.spec.ts` +- `apps/web/e2e/tests/review/mobile-walkthrough.spec.ts` +- `docs/plans/changes-walkthrough-toolbar-width/plan.md` +- `docs/plans/changes-walkthrough-toolbar-width/task-01-panel-responsive-walkthrough.md` + +## Verification + +Run from `apps/web`: + +```bash +pnpm e2e:run --host --project chromium -- tests/review/walkthrough.spec.ts --grep "adapts the Changes walkthrough label to panel width" --workers=1 +pnpm e2e:run --host --no-build --project mobile-chrome -- tests/review/mobile-walkthrough.spec.ts --grep "requests a walkthrough from Changes" --workers=1 +``` + +The managed runner rebuilds the production Vite assets before Playwright runs. + +## Dependencies + +None. + +## Parallelism + +`sequential`. The production and E2E changes define one coupled responsive +contract and should be completed through one Red-Green-Refactor cycle. + +## Inputs + +- Spec scenarios in + `docs/specs/changes-walkthrough-toolbar-width/spec.md`. +- Frontend and mobile contract in `plan.md`. +- Existing responsive toolbar implementation in + `apps/web/components/task/changes-panel-header.tsx`. +- Existing Dockview resize helper in `apps/web/e2e/helpers/dockview-resize.ts`. +- Existing request flows in the desktop and mobile walkthrough specs. + +## Risks + +- Apply the named container to the full panel root so the threshold is not + shifted by the header's horizontal padding. +- Do not relax `PanelHeaderBarSplit` overflow or right-slot shrink behavior; + those preserve the branch and Pull actions at narrow widths. + +## Output contract + +Report the files changed, the observed RED failures, the final focused command +result, any Dockview rounding encountered, remaining risks, and the updated task +and plan statuses in this conversation. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 4c41d48f8a..a1d1a016ba 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -140,6 +140,7 @@ Subscription quota tracking and per-agent cheap-model profile routing. | [task-layout-profiles](ui/task-layout-profiles.md) | draft | | [task-surface-refresh](ui/task-surface-refresh.md) | draft | | [walkthrough-navigation-layout](walkthrough-navigation-layout/spec.md) | shipped | +| [changes-walkthrough-toolbar-width](changes-walkthrough-toolbar-width/spec.md) | shipped | | [agent-message-comments](ui/agent-message-comments.md) | shipped | | [external-vcs-file-links](ui/external-vcs-file-links.md) | shipped | | [task-listing-display-preferences](ui/task-listing-display-preferences.md) | shipped | diff --git a/docs/specs/changes-walkthrough-toolbar-width/spec.md b/docs/specs/changes-walkthrough-toolbar-width/spec.md new file mode 100644 index 0000000000..621b311c26 --- /dev/null +++ b/docs/specs/changes-walkthrough-toolbar-width/spec.md @@ -0,0 +1,49 @@ +--- +status: shipped +created: 2026-07-31 +owner: product +--- + +# Responsive Changes Walkthrough Action + +## Why + +The Changes toolbar can be narrower than the browser viewport inside the task +workbench. At narrow panel widths, the **Walkthrough** action label is clipped +instead of adapting to the space actually available in that panel. + +## What + +- The Changes toolbar adapts the **Walkthrough** action to the inline width of + the Changes panel, independent of the browser viewport width. +- At a Changes-panel width of 349px or less, the action shows its route icon + without the visible **Walkthrough** label. +- At a Changes-panel width of 350px or more, the action shows both its route + icon and the **Walkthrough** label. +- Both presentations retain the same accessible name, tooltip, enabled and + disabled behavior, and click outcome. +- The action and the toolbar's branch and Pull controls remain contained within + the Changes panel without introducing document-level horizontal overflow. +- The desktop and phone Changes surfaces share the same width-driven behavior + and preserve the ability to request a walkthrough. + +## Scenarios + +- **GIVEN** a desktop task whose Changes panel is 349px wide, **WHEN** the + Changes toolbar renders, **THEN** the Walkthrough label is hidden while its + icon button remains visible, accessible, and fully contained in the panel. +- **GIVEN** a desktop task whose Changes panel is 350px wide, **WHEN** the + Changes toolbar renders, **THEN** the Walkthrough label is visible and the + complete action remains contained in the panel. +- **GIVEN** a wide laptop viewport with a Changes panel narrowed to 349px, + **WHEN** the toolbar renders, **THEN** the panel width rather than the + viewport width selects the icon-only presentation. +- **GIVEN** the phone Changes surface is wide enough to show the label, **WHEN** + the user requests a walkthrough, **THEN** the action remains within the + viewport and produces the same walkthrough request as desktop. + +## Out of scope + +- Redesigning the Changes toolbar or moving actions into an overflow menu. +- Changing the responsive behavior of Diff, Review, branch, or Pull controls. +- Changing walkthrough generation, display, navigation, or persistence.