Skip to content

Commit 7bd7b5f

Browse files
quickbeardclaude
quickbeard
and
claude
committed
test(app): cover the sidebar section and the session menu's Delete item
Unit: the Recents row budget moves to app-sidebar-rows.ts as `recentsRowCapacity` and gets a table of heights. It was the one real logic seam in this work — off-by-one arithmetic over the row pitch and gap that decides whether "View all" stays reachable — and it was untested. Extraction is behaviour-identical. E2E: the Delete item is now asserted on both surfaces the user sees it on, the sidebar row menu and the in-session header menu, sharing one set of assertions. The header trigger opens on the first click, so only the sidebar helper retries past the known first-click defect. E2E: the section header is checked for keyboard operability, and a height sweep pins that "View all" is never clipped out of the list. That sweep stops at a 400px window. Below it the flexible area drops under one row and View all is drawn with nowhere to go — pre-existing, unrelated to the collapse work, and left for its own fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d132b04 commit 7bd7b5f

5 files changed

Lines changed: 151 additions & 30 deletions

File tree

‎packages/app/e2e/regression/session-menu-delete-item.spec.ts‎

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect, test, type Page } from "@playwright/test"
2+
import { base64Encode } from "@opencode-ai/core/util/encode"
23
import { fixture, pageMessages } from "../smoke/session-timeline.fixture"
34
import { mockOpenCodeServer } from "../utils/mock-server"
45
import { expectAppVisible } from "../utils/waits"
@@ -8,7 +9,7 @@ const items = (page: Page) => content(page).locator('[data-component="menu-v2-it
89
const label = (page: Page, name: string) =>
910
items(page).filter({ hasText: name }).locator('[data-slot="menu-v2-item-content"]')
1011

11-
async function openSessionMenu(page: Page) {
12+
async function boot(page: Page) {
1213
await mockOpenCodeServer(page, {
1314
sessions: fixture.sessions,
1415
provider: fixture.provider,
@@ -25,22 +26,15 @@ async function openSessionMenu(page: Page) {
2526
}),
2627
)
2728
}, fixture.directory)
28-
await page.goto("/")
29-
30-
const sidebar = page.locator('[data-component="app-sidebar"]')
31-
const row = sidebar.locator('[data-component="sidebar-session-row"]').first()
32-
await expectAppVisible(row)
33-
// Wait for the hover-reveal wrapper to finish fading before clicking — the
34-
// button itself is "visible" to Playwright at opacity 0, so it is no signal.
35-
await row.hover()
36-
await expect(sidebar.locator('[data-slot="session-row-actions"]').first()).toHaveCSS("opacity", "1")
29+
}
3730

38-
// Retried, because the row trigger swallows its first click and opens on the
39-
// second. That reproduces on a clean main with the wrapper fully faded in and
40-
// the app settled, so it is a pre-existing defect in the trigger and not
41-
// something this spec should assert into place — retry past it here and fix it
42-
// on its own.
43-
const trigger = sidebar.locator('[data-action="session-menu"]').first()
31+
// Retried, because the sidebar row trigger swallows its first click and opens on
32+
// the second. That reproduces on a clean main with the hover wrapper fully faded
33+
// in and the app settled, so it is a pre-existing defect in the trigger and not
34+
// something these specs should assert into place — retry past it here and fix it
35+
// on its own. The in-session header trigger has no such wrapper and opens first
36+
// time, which is why only this helper retries.
37+
async function clickUntilOpen(page: Page, trigger: ReturnType<Page["locator"]>) {
4438
await expect(async () => {
4539
await trigger.click()
4640
await expect(content(page)).toHaveCount(1, { timeout: 1000 })
@@ -60,9 +54,7 @@ async function dangerColor(page: Page) {
6054
})
6155
}
6256

63-
test("Delete reads as destructive and carries no ellipsis", async ({ page }) => {
64-
await openSessionMenu(page)
65-
57+
async function expectDeleteReadsAsDestructive(page: Page) {
6658
// Ellipsis dropped deliberately — Share keeps its own, so this is not a blanket
6759
// strip of the convention.
6860
await expect(label(page, "Delete")).toHaveText("Delete")
@@ -78,4 +70,34 @@ test("Delete reads as destructive and carries no ellipsis", async ({ page }) =>
7870
// Highlighting only swaps the background, so the colour has to survive hover.
7971
await items(page).filter({ hasText: "Delete" }).hover()
8072
await expect(label(page, "Delete")).toHaveCSS("color", danger)
73+
}
74+
75+
test("Delete reads as destructive in the sidebar row menu", async ({ page }) => {
76+
await boot(page)
77+
await page.goto("/")
78+
79+
const sidebar = page.locator('[data-component="app-sidebar"]')
80+
const row = sidebar.locator('[data-component="sidebar-session-row"]').first()
81+
await expectAppVisible(row)
82+
// Wait for the hover-reveal wrapper to finish fading before clicking — the
83+
// button itself is "visible" to Playwright at opacity 0, so it is no signal.
84+
await row.hover()
85+
await expect(sidebar.locator('[data-slot="session-row-actions"]').first()).toHaveCSS("opacity", "1")
86+
87+
await clickUntilOpen(page, sidebar.locator('[data-action="session-menu"]').first())
88+
await expectDeleteReadsAsDestructive(page)
89+
})
90+
91+
test("Delete reads as destructive in the in-session header menu", async ({ page }) => {
92+
await boot(page)
93+
await page.goto(`/${base64Encode(fixture.directory)}/session/${fixture.targetID}`)
94+
95+
// The header menu carries no data-action hook, only its aria-label, and it is
96+
// scoped to main so the sidebar row triggers cannot match.
97+
const trigger = page.locator("main").getByRole("button", { name: "More options" }).first()
98+
await expectAppVisible(trigger)
99+
await trigger.click()
100+
await expectAppVisible(content(page))
101+
102+
await expectDeleteReadsAsDestructive(page)
81103
})

‎packages/app/e2e/regression/sidebar-section-collapse.spec.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const sidebar = (page: Page) => page.locator('[data-component="app-sidebar"]')
77
const recents = (page: Page) => sidebar(page).locator('[data-section="recents"]')
88
const toggle = (page: Page) => recents(page).locator('[data-action="sidebar-section-toggle"]')
99
const viewAll = (page: Page) => recents(page).locator('[data-action="sidebar-view-all"]')
10+
const sessionRows = (page: Page) => recents(page).locator('[data-component="sidebar-session-row"]')
1011
const settings = (page: Page) => sidebar(page).getByRole("button", { name: "Settings" })
1112

1213
async function boot(page: Page) {
@@ -69,3 +70,57 @@ test("collapsing Recents moves neither its own header nor the bottom nav", async
6970
expect(Math.abs(after.recents!.y - before.recents!.y)).toBeLessThan(2)
7071
expect(Math.abs(after.settings!.y - before.settings!.y)).toBeLessThan(2)
7172
})
73+
74+
test("the header toggles from the keyboard", async ({ page }) => {
75+
await boot(page)
76+
77+
// The header is a real button carrying the label, so it has to be reachable
78+
// and operable without a pointer — that is what lets `aria-expanded` alone
79+
// name the control instead of a translated aria-label.
80+
await toggle(page).focus()
81+
await expect(toggle(page)).toBeFocused()
82+
83+
await page.keyboard.press("Enter")
84+
await expect(toggle(page)).toHaveAttribute("aria-expanded", "false")
85+
await expect(viewAll(page)).toHaveCount(0)
86+
87+
await page.keyboard.press("Space")
88+
await expect(toggle(page)).toHaveAttribute("aria-expanded", "true")
89+
await expectAppVisible(viewAll(page))
90+
})
91+
92+
test("View all is never pushed out of the list as the window shortens", async ({ page }) => {
93+
await boot(page)
94+
await expectAppVisible(viewAll(page))
95+
expect(await sessionRows(page).count()).toBe(fixture.sessions.length)
96+
97+
// Recents draws only as many rows as fit and reserves the last slot for View
98+
// all (see recentsRowCapacity). The list clips with overflow-hidden, so
99+
// over-filling it does not push anything into the viewport's way — it just
100+
// silently cuts View all off. Compare against the list box rather than the
101+
// viewport, and sweep heights so the assertion does not depend on landing in
102+
// a lucky band.
103+
// Stops at 400. Below that the flexible area itself drops under one row
104+
// (12px at a 380px window, 0 at 360) and View all is drawn anyway, so it is
105+
// clipped — capacity clamps the session rows to zero but nothing withholds
106+
// View all when there is no room for it either. That is pre-existing, not
107+
// something the collapse work introduced, and it wants its own fix; this
108+
// sweep guards the range where the reservation is supposed to hold.
109+
for (const height of [900, 700, 560, 480, 420, 400]) {
110+
await page.setViewportSize({ width: 1280, height })
111+
// The row count is driven by a ResizeObserver, so the list is briefly still
112+
// filled for the old box after a resize. Poll rather than measure the frame
113+
// straight after: a genuine clip never resolves, so this waits out the
114+
// reflow without waiting away the failure.
115+
await expect(async () => {
116+
const list = await recents(page).locator('[data-slot="sidebar-recents-list"]').boundingBox()
117+
const all = await viewAll(page).boundingBox()
118+
if (!all) return // too short to draw the section at all
119+
expect(list).not.toBeNull()
120+
expect(
121+
{ height, bottomOverflow: Math.max(0, Math.round(all.y + all.height - (list!.y + list!.height))) },
122+
`View all is clipped at viewport height ${height}`,
123+
).toEqual({ height, bottomOverflow: 0 })
124+
}).toPass({ timeout: 5000 })
125+
}
126+
})

‎packages/app/src/components/app-sidebar-rows.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { HOME_NAV_ROW_GAP, HOME_NAV_ROW_PITCH, recentsRowCapacity } from "./app-sidebar-rows"
3+
4+
// n rows laid out at this pitch occupy 32n - 4: the last row carries no
5+
// trailing gap. Deriving the fixtures rather than hardcoding pixel counts keeps
6+
// the test honest if the row height or the gap ever moves.
7+
const exactly = (rows: number) => rows * HOME_NAV_ROW_PITCH - HOME_NAV_ROW_GAP
8+
9+
describe("recentsRowCapacity", () => {
10+
test("reserves the last fitting row for View all", () => {
11+
expect(recentsRowCapacity(exactly(1))).toBe(0)
12+
expect(recentsRowCapacity(exactly(2))).toBe(1)
13+
expect(recentsRowCapacity(exactly(5))).toBe(4)
14+
})
15+
16+
test("never reports a row it cannot draw whole", () => {
17+
// One pixel short of fitting a third row is still a two-row box, so the
18+
// budget stays at one session plus View all.
19+
expect(recentsRowCapacity(exactly(3) - 1)).toBe(1)
20+
expect(recentsRowCapacity(exactly(3))).toBe(2)
21+
})
22+
23+
test("clamps instead of going negative when nothing fits", () => {
24+
expect(recentsRowCapacity(0)).toBe(0)
25+
expect(recentsRowCapacity(exactly(1) - 1)).toBe(0)
26+
// The observer reports 0 while the section is collapsed and the list is
27+
// unmounted; a negative slice length would throw off Array.slice.
28+
expect(recentsRowCapacity(-40)).toBe(0)
29+
})
30+
31+
test("grows one session at a time with the box", () => {
32+
const heights = Array.from({ length: 8 }, (_, index) => exactly(index + 1))
33+
expect(heights.map(recentsRowCapacity)).toEqual([0, 1, 2, 3, 4, 5, 6, 7])
34+
})
35+
})

‎packages/app/src/components/app-sidebar-rows.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// HOME_PROJECT_NAV_ROW rows are h-7 (28px) in a flex column with gap-1 (4px),
2+
// so n rows occupy 32n - 4 — the last one carries no trailing gap.
3+
export const HOME_NAV_ROW_PITCH = 32
4+
export const HOME_NAV_ROW_GAP = 4
5+
6+
// How many session rows the sidebar's Recents list may draw into `height` px of
7+
// the flexible area. One row of the budget belongs to "View all", which renders
8+
// inside the same container so it hugs the last session instead of being pushed
9+
// to the bottom — reserving it here is what keeps "View all" reachable without
10+
// scrolling at any window height.
11+
export function recentsRowCapacity(height: number) {
12+
return Math.max(0, Math.floor((height + HOME_NAV_ROW_GAP) / HOME_NAV_ROW_PITCH) - 1)
13+
}

‎packages/app/src/components/app-sidebar.tsx‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Icon as IconV2 } from "@opencode-ai/ui/v2/icon"
99
import { IconButtonV2 } from "@opencode-ai/ui/v2/icon-button-v2"
1010
import { TooltipV2 } from "@opencode-ai/ui/v2/tooltip-v2"
1111
import { useDialog } from "@opencode-ai/ui/context/dialog"
12+
import { recentsRowCapacity } from "@/components/app-sidebar-rows"
1213
import { useServerManagementController } from "@/components/dialog-select-server"
1314
import { DialogDeleteSession, SessionActionsMenu } from "@/components/session-actions-menu"
1415
import { DialogServerV2 } from "@/components/settings-v2/dialog-server-v2"
@@ -45,10 +46,6 @@ import { sessionTitle } from "@/utils/session-title"
4546
// every route. Sections top to bottom: Projects, Recents (New row, a list sized
4647
// to the free height, and a pinned View all), then Settings/Help anchored to the
4748
// bottom.
48-
// HOME_PROJECT_NAV_ROW rows are h-7 (28px) in a flex column with gap-1 (4px),
49-
// so n rows occupy 32n - 4 — the last one carries no trailing gap.
50-
const HOME_NAV_ROW_PITCH = 32
51-
const HOME_NAV_ROW_GAP = 4
5249

5350
const CHANNEL = import.meta.env.VITE_CODEV_CHANNEL
5451

@@ -249,12 +246,7 @@ function SidebarRecents(props: {
249246
() => list() ?? [],
250247
(rect) => setListHeight(rect.height),
251248
)
252-
// One row of the budget belongs to "View all", which renders inside the same
253-
// container so it hugs the last session instead of being pushed to the bottom
254-
// of the flexible area.
255-
const visible = createMemo(() =>
256-
props.records.slice(0, Math.max(0, Math.floor((listHeight() + HOME_NAV_ROW_GAP) / HOME_NAV_ROW_PITCH) - 1)),
257-
)
249+
const visible = createMemo(() => props.records.slice(0, recentsRowCapacity(listHeight())))
258250

259251
return (
260252
<SidebarSection
@@ -274,7 +266,11 @@ function SidebarRecents(props: {
274266
<span class={HOME_PROJECT_NAV_LABEL}>{language.t("sidebar.recents.new")}</span>
275267
</button>
276268
</Show>
277-
<div ref={setList} class="flex min-h-0 min-w-0 flex-1 flex-col gap-1 overflow-hidden">
269+
<div
270+
ref={setList}
271+
data-slot="sidebar-recents-list"
272+
class="flex min-h-0 min-w-0 flex-1 flex-col gap-1 overflow-hidden"
273+
>
278274
<For each={visible()}>
279275
{(record) => (
280276
<SidebarSessionRow

0 commit comments

Comments
 (0)