From c2ba394241678e9e0ad2830f08a484e4a0a59007 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Tue, 11 Aug 2026 11:21:36 +0700 Subject: [PATCH 1/2] fix(app): open the sidebar row menu on the first click, and stop clipping View all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found while writing the sidebar tests. The row menu ignored its first click. `createHomeModel`'s records memo mints a fresh `{session, project}` wrapper per session on every recompute — the projects memo alone returns a new array on a pointerdown — and the list rendered those through a reference-keyed ``, so the row was torn down and rebuilt mid-gesture. With no surviving common ancestor the browser fires no `click` at all, so the gesture vanished and only a second click landed. Iterating session ids instead keeps the row DOM across a rebuild and feeds it the new record reactively, which also stops a background refresh from closing an open menu or dropping an in-progress rename. Below roughly a 380px window the flexible area falls under one row, and "View all" was drawn into it anyway and clipped by overflow-hidden. `fitsRecentsRow` withholds it at those sizes: whole or absent, never cut. Both regressions are pinned. The three new or extended assertions were each run against the unfixed code and fail there. The same first-click defect remains on the home "View all" list, whose rows live in upstream's home.tsx. A stable-identity cache in the fork-only home-model would have covered both, but it does not fix that list — its rows come from different arrays — so it is not included here. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 14 ++++ .../sidebar-section-collapse.spec.ts | 21 +++--- .../sidebar-session-menu-first-click.spec.ts | 75 +++++++++++++++++++ .../src/components/app-sidebar-rows.test.ts | 25 ++++++- .../app/src/components/app-sidebar-rows.ts | 9 +++ packages/app/src/components/app-sidebar.tsx | 38 +++++++--- 6 files changed, 160 insertions(+), 22 deletions(-) create mode 100644 packages/app/e2e/regression/sidebar-session-menu-first-click.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ba6e74541..7116dd86b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,20 @@ file is the human-readable summary of what changed and why. ### Desktop / Web UI +- **The ⋮ menu on a sidebar session opens on the first click** — it used to + ignore one click and open on the next. The rows were being rebuilt from + scratch whenever anything the recent-sessions list depends on changed, which + could happen mid-click and threw the click away; they are now updated in + place. The same rebuild could also close the menu under you while it was + open, or drop a rename you were part-way through typing. The ⋯ menu beside a + session title was never affected. The equivalent menu on the **View all** + list still needs one extra click. + +- **Recents no longer draws a half-cut View all** — in a window short enough + that the list has no room for a whole row (roughly under 380px tall), **View + all** was still drawn and clipped by the edge of the list. It is now left out + at those sizes instead. + - **Delete reads as destructive** — the **Delete** item in a session's ⋮ / ⋯ menu is now red, in both the sidebar rows and the menu beside a session title, and it has lost its trailing ellipsis. It still asks for confirmation diff --git a/packages/app/e2e/regression/sidebar-section-collapse.spec.ts b/packages/app/e2e/regression/sidebar-section-collapse.spec.ts index aaaf01600b..6055985c42 100644 --- a/packages/app/e2e/regression/sidebar-section-collapse.spec.ts +++ b/packages/app/e2e/regression/sidebar-section-collapse.spec.ts @@ -100,13 +100,11 @@ test("View all is never pushed out of the list as the window shortens", async ({ // silently cuts View all off. Compare against the list box rather than the // viewport, and sweep heights so the assertion does not depend on landing in // a lucky band. - // Stops at 400. Below that the flexible area itself drops under one row - // (12px at a 380px window, 0 at 360) and View all is drawn anyway, so it is - // clipped — capacity clamps the session rows to zero but nothing withholds - // View all when there is no room for it either. That is pre-existing, not - // something the collapse work introduced, and it wants its own fix; this - // sweep guards the range where the reservation is supposed to hold. - for (const height of [900, 700, 560, 480, 420, 400]) { + // Swept down past 400, where the flexible area drops under one row (12px at + // a 380px window, none at 360). View all is withheld entirely at those sizes + // rather than drawn into space that clips it, so the invariant holds all the + // way down: it is either whole or absent, never cut in half. + for (const height of [900, 700, 560, 480, 420, 400, 380, 360, 320]) { await page.setViewportSize({ width: 1280, height }) // The row count is driven by a ResizeObserver, so the list is briefly still // filled for the old box after a resize. Poll rather than measure the frame @@ -114,8 +112,13 @@ test("View all is never pushed out of the list as the window shortens", async ({ // reflow without waiting away the failure. await expect(async () => { const list = await recents(page).locator('[data-slot="sidebar-recents-list"]').boundingBox() - const all = await viewAll(page).boundingBox() - if (!all) return // too short to draw the section at all + // Withheld outright once the list cannot hold a whole row — absent is a + // pass, clipped is not. + if ((await viewAll(page).count()) === 0) { + expect({ height, listFits: (list?.height ?? 0) >= 28 }).toEqual({ height, listFits: false }) + return + } + const all = (await viewAll(page).boundingBox())! expect(list).not.toBeNull() expect( { height, bottomOverflow: Math.max(0, Math.round(all.y + all.height - (list!.y + list!.height))) }, diff --git a/packages/app/e2e/regression/sidebar-session-menu-first-click.spec.ts b/packages/app/e2e/regression/sidebar-session-menu-first-click.spec.ts new file mode 100644 index 0000000000..a4c6f3b078 --- /dev/null +++ b/packages/app/e2e/regression/sidebar-session-menu-first-click.spec.ts @@ -0,0 +1,75 @@ +import { expect, test, type Page } from "@playwright/test" +import { fixture, pageMessages } from "../smoke/session-timeline.fixture" +import { mockOpenCodeServer } from "../utils/mock-server" +import { expectAppVisible } from "../utils/waits" + +const sidebar = (page: Page) => page.locator('[data-component="app-sidebar"]') +const rows = (page: Page) => sidebar(page).locator('[data-component="sidebar-session-row"]') +const content = (page: Page) => page.locator('[data-component="menu-v2-content"]') + +async function boot(page: Page) { + await mockOpenCodeServer(page, { + sessions: fixture.sessions, + provider: fixture.provider, + directory: fixture.directory, + project: fixture.project, + pageMessages, + }) + await page.addInitScript((directory) => { + localStorage.setItem( + "opencode.global.dat:server", + JSON.stringify({ + projects: { local: [{ worktree: directory, expanded: true }] }, + lastProject: { local: directory }, + }), + ) + }, fixture.directory) + await page.goto("/") + await expectAppVisible(rows(page).first()) + // Let the initial burst of record rebuilds finish, so this measures the + // steady state rather than boot churn. + await expect(rows(page)).toHaveCount(fixture.sessions.length) +} + +async function revealActions(page: Page) { + await rows(page).first().hover() + // The wrapper fades out of pointer-events:none; the button is "visible" to + // Playwright at opacity 0, so the fade is the only honest signal. + await expect(sidebar(page).locator('[data-slot="session-row-actions"]').first()).toHaveCSS("opacity", "1") +} + +test("the row menu opens on the first click", async ({ page }) => { + await boot(page) + await revealActions(page) + + // Exactly one click. The records memo rebuilds its `{session, project}` + // wrappers on unrelated recomputes, and while the list was keyed on those + // references the row was torn down between pointerdown and mouseup — no + // common ancestor survived for the browser to fire `click` on, so the first + // click did nothing and only the second opened the menu. + await sidebar(page).locator('[data-action="session-menu"]').first().click() + await expect(content(page)).toHaveCount(1) +}) + +test("a record rebuild leaves the open menu and the row DOM alone", async ({ page }) => { + await boot(page) + await revealActions(page) + await sidebar(page).locator('[data-action="session-menu"]').first().click() + await expectAppVisible(content(page)) + + const row = await rows(page).first().elementHandle() + + // Force the rebuild the sidebar used to be destroyed by. Refetching the + // session index re-runs the records memo and hands `` a fresh set of + // record objects; keyed by id, the rows have to survive it. + await page.evaluate(() => window.dispatchEvent(new Event("focus"))) + await page.evaluate(() => window.dispatchEvent(new Event("online"))) + await page.waitForTimeout(600) + + await expect(content(page)).toHaveCount(1) + expect( + await rows(page) + .first() + .evaluate((el, previous) => el === previous, row), + ).toBe(true) +}) diff --git a/packages/app/src/components/app-sidebar-rows.test.ts b/packages/app/src/components/app-sidebar-rows.test.ts index 04c07da1ae..15c5b11802 100644 --- a/packages/app/src/components/app-sidebar-rows.test.ts +++ b/packages/app/src/components/app-sidebar-rows.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { HOME_NAV_ROW_GAP, HOME_NAV_ROW_PITCH, recentsRowCapacity } from "./app-sidebar-rows" +import { fitsRecentsRow, HOME_NAV_ROW_GAP, HOME_NAV_ROW_PITCH, recentsRowCapacity } from "./app-sidebar-rows" // n rows laid out at this pitch occupy 32n - 4: the last row carries no // trailing gap. Deriving the fixtures rather than hardcoding pixel counts keeps @@ -33,3 +33,26 @@ describe("recentsRowCapacity", () => { expect(heights.map(recentsRowCapacity)).toEqual([0, 1, 2, 3, 4, 5, 6, 7]) }) }) + +describe("fitsRecentsRow", () => { + test("takes one whole row of space", () => { + expect(fitsRecentsRow(exactly(1))).toBe(true) + expect(fitsRecentsRow(exactly(1) - 1)).toBe(false) + }) + + test("refuses the heights where View all used to be clipped", () => { + // Measured from the real column: a 380px window leaves 12px of flexible + // area and a 360px window leaves none, and the list clips rather than + // scrolls, so View all was drawn cut in half or not at all. + expect(fitsRecentsRow(12)).toBe(false) + expect(fitsRecentsRow(0)).toBe(false) + }) + + test("agrees with the capacity budget once a session fits", () => { + // Anywhere capacity allows a session row, there is by construction room for + // View all too — the budget reserves it. The two must not disagree. + for (let height = 0; height <= 400; height += 1) { + if (recentsRowCapacity(height) > 0) expect(fitsRecentsRow(height)).toBe(true) + } + }) +}) diff --git a/packages/app/src/components/app-sidebar-rows.ts b/packages/app/src/components/app-sidebar-rows.ts index 4a83f0513b..135744919c 100644 --- a/packages/app/src/components/app-sidebar-rows.ts +++ b/packages/app/src/components/app-sidebar-rows.ts @@ -11,3 +11,12 @@ export const HOME_NAV_ROW_GAP = 4 export function recentsRowCapacity(height: number) { return Math.max(0, Math.floor((height + HOME_NAV_ROW_GAP) / HOME_NAV_ROW_PITCH) - 1) } + +// Whether the list can draw "View all" at all. The capacity above clamps the +// session rows to zero once the flexible area falls under two rows, but the +// area keeps shrinking past that — under one row at a ~380px window, to nothing +// at ~360 — and the list clips with overflow-hidden, so drawing the row anyway +// just cut it in half. One row is the floor for showing it. +export function fitsRecentsRow(height: number) { + return height >= HOME_NAV_ROW_PITCH - HOME_NAV_ROW_GAP +} diff --git a/packages/app/src/components/app-sidebar.tsx b/packages/app/src/components/app-sidebar.tsx index 7f054ffba3..4357d85836 100644 --- a/packages/app/src/components/app-sidebar.tsx +++ b/packages/app/src/components/app-sidebar.tsx @@ -9,7 +9,7 @@ import { Icon as IconV2 } from "@opencode-ai/ui/v2/icon" import { IconButtonV2 } from "@opencode-ai/ui/v2/icon-button-v2" import { TooltipV2 } from "@opencode-ai/ui/v2/tooltip-v2" import { useDialog } from "@opencode-ai/ui/context/dialog" -import { recentsRowCapacity } from "@/components/app-sidebar-rows" +import { fitsRecentsRow, recentsRowCapacity } from "@/components/app-sidebar-rows" import { useServerManagementController } from "@/components/dialog-select-server" import { DialogDeleteSession, SessionActionsMenu } from "@/components/session-actions-menu" import { DialogServerV2 } from "@/components/settings-v2/dialog-server-v2" @@ -271,18 +271,32 @@ function SidebarRecents(props: { data-slot="sidebar-recents-list" class="flex min-h-0 min-w-0 flex-1 flex-col gap-1 overflow-hidden" > - - {(record) => ( - - )} + {/* Iterated by session id, not by record. `createHomeModel`'s records + memo rebuilds every `{session, project}` wrapper whenever anything + it reads changes — the projects memo alone hands back a fresh array + on a pointerdown — and `` keys on reference, so passing records + straight in tore down and rebuilt every row on those recomputes. + That destroyed the ⋮ trigger between pointerdown and mouseup, so the + browser had no common ancestor to fire `click` on and the menu + silently ate the gesture. Ids compare by value, so the rows now + survive a rebuild and take the new record reactively. */} + record.session.id)}> + {(id) => { + const record = createMemo(() => props.records.find((item) => item.session.id === id)) + return ( + + + + ) + }} - 0}> + 0 && fitsRecentsRow(listHeight())}> Date: Tue, 11 Aug 2026 11:30:49 +0700 Subject: [PATCH 2/2] fix(app): key the View all session list by id so its row menu opens first click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries the sidebar fix to the other surface. `groupSessions` mints new group objects on every call and the records memo mints new record wrappers, so both `` levels on the View all screen were keyed on references that churned — tearing down the rows on recomputes that changed nothing, including between a pointerdown and its mouseup, which is why the ⋮ menu there ignored its first click too. Both levels now iterate ids. DIVERGENCES gains a row: home.tsx is upstream, and the on-conflict note spells out that reverting to `` reintroduces the bug. Both new assertions were run against the unfixed home.tsx and fail there. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 16 ++-- DIVERGENCES.md | 1 + .../home-session-menu-first-click.spec.ts | 69 +++++++++++++++++ packages/app/src/pages/home.tsx | 77 ++++++++++++------- 4 files changed, 127 insertions(+), 36 deletions(-) create mode 100644 packages/app/e2e/regression/home-session-menu-first-click.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7116dd86b8..f68d1e769f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,14 +9,14 @@ file is the human-readable summary of what changed and why. ### Desktop / Web UI -- **The ⋮ menu on a sidebar session opens on the first click** — it used to - ignore one click and open on the next. The rows were being rebuilt from - scratch whenever anything the recent-sessions list depends on changed, which - could happen mid-click and threw the click away; they are now updated in - place. The same rebuild could also close the menu under you while it was - open, or drop a rename you were part-way through typing. The ⋯ menu beside a - session title was never affected. The equivalent menu on the **View all** - list still needs one extra click. +- **The ⋮ menu on a session row opens on the first click** — it used to ignore + one click and open on the next, both in the sidebar's Recents and on the + **View all** list. The rows were being rebuilt from scratch whenever anything + the session list depends on changed, which could happen mid-click and threw + the click away; they are now updated in place. The same rebuild could also + close the menu under you while it was open, or drop a rename you were + part-way through typing. The ⋯ menu beside a session title was never + affected. - **Recents no longer draws a half-cut View all** — in a window short enough that the list has no room for a whole row (roughly under 380px tall), **View diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 4019e0c3bd..7208d8ceb2 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -124,6 +124,7 @@ from the squash message first, or use a plain merge. | `packages/ui/src/theme/context.tsx`, `packages/app/public/oc-theme-preload.js`, `packages/app/src/theme-preload.test.ts`, `packages/app/src/pages/layout.tsx`, `packages/app/src/components/settings-general.tsx`, `packages/app/src/components/settings-v2/general.tsx` | **One theme in the web/desktop UI; light and dark are the only axis.** Upstream ships 36 selectable palettes with a Settings → Appearance **Theme** picker and command-palette entries. The fork exposes none of them: OC-2 is the theme, `colorScheme` (system/light/dark) is the only appearance choice. The lock lives at `normalize()` in `theme/context.tsx`, renamed to the exported `normalizeThemeID()` and returning the exported `SHIPPED_THEME_ID` unconditionally — that one function is already on every path (provider init, the `onMount` migration, `load`, the cross-tab `storage` listener, `setTheme`, `previewTheme`), so a stored id from before the picker was removed, or one written by another tab, resolves to OC-2 without a second guard. It is exported (and the six call sites renamed) so `theme/context.test.ts` can cover the invariant directly instead of only through its call sites — the module imports cleanly under `bun test` because `import.meta.glob` is called lazily inside `getFiles()`. It also gives the migration for free: `onMount` already rewrites `opencode-theme-id` and calls `clear()` when the raw value differs from the normalized one, which now drops the stale `opencode-theme-css-{light,dark}` cache. `ids()` returns `["oc-2"]` and `loadThemes()` loads only OC-2 instead of all 36 (it was `Promise.all(themeIDs().map(load))`). The bundled theme JSON, the `names` map and `registerTheme` are **left alone** — unreferenced data costs nothing and keeps upstream's theme edits applying cleanly. `oc-theme-preload.js` does the same rewrite *before* mount, so the first launch after upgrading does not paint the user's old palette for a frame; with no other theme reachable its cached-CSS `