fix(web): adapt walkthrough label to panel width (#2106)
This commit is contained in:
@@ -514,7 +514,7 @@ function ChangesPanelWalkthroughButton({
|
||||
: "Walk me through these changes";
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<TooltipTrigger asChild className="order-first @[350px]/changes-panel:order-none">
|
||||
<span className="inline-flex" tabIndex={requestWalkthroughDisabled ? 0 : undefined}>
|
||||
<Button
|
||||
size="sm"
|
||||
@@ -526,7 +526,7 @@ function ChangesPanelWalkthroughButton({
|
||||
onClick={onRequestWalkthrough}
|
||||
>
|
||||
<IconRoute className="h-3 w-3" />
|
||||
<span className="hidden min-[430px]:inline sm:inline">Walkthrough</span>
|
||||
<span className="hidden @[350px]/changes-panel:inline">Walkthrough</span>
|
||||
</Button>
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
|
||||
@@ -30,7 +30,7 @@ const ChangesPanel = memo(function ChangesPanel(props: ChangesPanelProps) {
|
||||
});
|
||||
if (isArchived) return <ArchivedPanelPlaceholder />;
|
||||
return (
|
||||
<PanelRoot data-testid="changes-panel">
|
||||
<PanelRoot className="@container/changes-panel" data-testid="changes-panel">
|
||||
<ChangesPanelHeader
|
||||
hasChanges={data.git.hasChanges}
|
||||
hasCommits={data.git.hasCommits}
|
||||
|
||||
@@ -94,7 +94,7 @@ export const MobileChangesPanel = memo(function MobileChangesPanel({
|
||||
|
||||
return (
|
||||
<>
|
||||
<PanelRoot data-testid="mobile-changes-panel">
|
||||
<PanelRoot className="@container/changes-panel" data-testid="mobile-changes-panel">
|
||||
<ChangesPanelHeader
|
||||
hasChanges={data.git.hasChanges}
|
||||
hasCommits={data.git.hasCommits}
|
||||
|
||||
@@ -82,6 +82,7 @@ test.describe("Mobile code walkthrough", () => {
|
||||
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();
|
||||
|
||||
@@ -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<Locator> {
|
||||
return card;
|
||||
}
|
||||
|
||||
async function expectContainedInPanel(panel: Locator, action: Locator): Promise<void> {
|
||||
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,
|
||||
|
||||
@@ -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.
|
||||
@@ -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.
|
||||
@@ -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 |
|
||||
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user