diff --git a/packages/app/e2e/session/session-tab-chip-contract.spec.ts b/packages/app/e2e/session/session-tab-chip-contract.spec.ts new file mode 100644 index 000000000..6d932c9c9 --- /dev/null +++ b/packages/app/e2e/session/session-tab-chip-contract.spec.ts @@ -0,0 +1,449 @@ +import { expect, test, type Page } from "../fixtures" +import { openRightPanel } from "../actions" +import { modKey } from "../utils" + +// Contract for the right-panel tab strip after the PR #880 chip + × cleanup. +// +// 1) Selected tab background uses `--row-active-overlay` (the same selection +// overlay the sidebar session row uses), not the previous opaque +// `var(--sidebar)`. Single source of truth for "this row/tab is selected" +// across the app. +// +// 2) The × close button visual is small (~8px) — clearly subordinate to the +// leading icon. The click slot stays at 14px to preserve a comfortable +// hover target. +// +// 3) Leading icon and × overlap at the same horizontal center (within 1px). +// PR #878 fixed the swap but left a 2-3px drift because CSS literals +// (`width: 14px`, `left: 10px`) ignore that html base font-size is 13px, +// so `size-3.5` (used by leadingSpan + closeBtn) renders ~11px instead. +// +// 4) Icon swap is instant — no opacity transition. Sweeping the mouse across +// multiple tabs must not flash both icons during a fade. +// +// These assertions complement (don't replace) the snap grid in +// packages/app/e2e/snap/right-panel-tabs-hover.snap.ts. + +const FILES_TAB = { name: "Files" } +const REVIEW_TAB = { name: "Review" } +const STATUS_TAB = { name: "Status" } + +async function openExtraTabs(page: Page) { + await page.locator("main").first().click() + await page.keyboard.press(`${modKey}+\\`) // fileTree.toggle + await page.keyboard.press(`${modKey}+Shift+R`) // review.toggle + // `>= 3` not `=== 3` — Status / Files / Review are the three this spec + // exercises, but a future default-open tab (Terminal / 上下文 / …) would + // otherwise fail this gate before reaching the actual assertions. + await expect.poll(() => page.getByRole("tab").count(), { timeout: 5_000 }).toBeGreaterThanOrEqual(3) +} + +test.describe("right-panel tab chip + × contract", () => { + test("selected tab uses --row-active-overlay (matches sidebar session row)", async ({ page, gotoSession }) => { + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // Click Files so Files is selected. Then sample its computed bg. + await page.getByRole("tab", FILES_TAB).click() + await page.mouse.move(0, 0) // no hover; rest state + + const tokenAndComputed = await page.evaluate(() => { + // Resolve the active-overlay token by painting it onto a throwaway + // element so the browser canonicalises the rgba string. Comparing the + // raw `--row-active-overlay` value would only match if the token were + // already in the exact `rgba(…)` form the renderer emits. + const probe = document.createElement("div") + probe.style.backgroundColor = "var(--row-active-overlay)" + document.body.appendChild(probe) + const expected = getComputedStyle(probe).backgroundColor + probe.remove() + // Chip background lives on the wrapper now (matches the sidebar session + // row's chip-on-container vocabulary), not the inner trigger button. + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const computed = wrap ? getComputedStyle(wrap).backgroundColor : "" + return { expected, computed } + }) + + // Exact match against the resolved `--row-active-overlay` token. + // Replacing the token with `--row-hover-overlay` (different alpha) or + // any other low-alpha rgba would now break this test — the previous + // alpha-range check tolerated that silently. + expect(tokenAndComputed.computed).toBe(tokenAndComputed.expected) + }) + + test("× glyph stays ~8px (subordinate to the 14px leading icon)", async ({ page, gotoSession }) => { + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // Hover the Files tab so the × is rendered (opacity 1). + await page.getByRole("tab", FILES_TAB).hover() + + const dims = await page.evaluate(() => { + const wrap = document.querySelector('[data-slot="tabs-trigger-wrapper"][data-value="files"]') + const slot = wrap?.querySelector('[data-slot="tabs-trigger-close-button"]') as HTMLElement | null + const svg = slot?.querySelector('[data-slot="icon-svg"]') as HTMLElement | null + const box = (el: HTMLElement | null) => { + if (!el) return null + const r = el.getBoundingClientRect() + return { w: Math.round(r.width), h: Math.round(r.height) } + } + return { svg: box(svg) } + }) + + // Visible × glyph: 6 to 10px so it reads as a subordinate affordance + // instead of overflowing the cell like the 16×16 raw leading icon does. + expect(dims.svg?.w).toBeGreaterThanOrEqual(6) + expect(dims.svg?.w).toBeLessThanOrEqual(10) + expect(dims.svg?.h).toBeGreaterThanOrEqual(6) + expect(dims.svg?.h).toBeLessThanOrEqual(10) + }) + + test("× center aligns with leading icon center (within 1px)", async ({ page, gotoSession }) => { + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // After the grid overlay refactor: the close-button slot is a full-cell + // flex container that pushes the × button to the same + // `padding-inline-start` the trigger uses for its leading icon. Measure + // the × icon-button (the actual visible glyph holder), not the slot box, + // against the leading icon span. + await page.getByRole("tab", FILES_TAB).hover() + + const positions = await page.evaluate(() => { + const wrap = document.querySelector('[data-slot="tabs-trigger-wrapper"][data-value="files"]') as HTMLElement | null + const leading = wrap?.querySelector('[data-slot="tab-icon-default"]') as HTMLElement | null + const closeBtn = wrap?.querySelector('[data-slot="tabs-trigger-close-button"] [data-component="icon-button"]') as HTMLElement | null + const box = (el: HTMLElement | null) => { + if (!el) return null + const r = el.getBoundingClientRect() + return { x: r.x, w: r.width, center: r.x + r.width / 2 } + } + return { leading: box(leading), close: box(closeBtn) } + }) + + expect(positions.leading).not.toBeNull() + expect(positions.close).not.toBeNull() + const diff = Math.abs(positions.leading!.center - positions.close!.center) + expect(diff).toBeLessThanOrEqual(1) + }) + + test("icon swap is instant (no opacity transition)", async ({ page, gotoSession }) => { + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + await page.mouse.move(0, 0) + + const transitions = await page.evaluate(() => { + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const icon = wrap?.querySelector('[data-slot="tab-icon-default"]') as HTMLElement | null + const closeSlot = wrap?.querySelector('[data-slot="tabs-trigger-close-button"]') as HTMLElement | null + const dur = (el: HTMLElement | null) => (el ? getComputedStyle(el).transitionDuration : null) + return { iconDur: dur(icon), closeSlotDur: dur(closeSlot) } + }) + + // Both durations must be zero — the swap is instant on hover, not faded. + // Multi-property transitions can produce a comma-joined duration string, + // so split on commas and verify every part is zero. Split-and-check is + // intentional: CodeQL flagged an earlier regex form (js/redos) and CodeQL + // also misreads quote-wrapped duration examples in comments as regex + // patterns, so the literal example is deliberately omitted here. + const allZero = (val: string | null) => + val !== null && val.split(",").every((part) => part.trim() === "0s") + expect(allZero(transitions.iconDur)).toBe(true) + expect(allZero(transitions.closeSlotDur)).toBe(true) + }) + + test("selected closable tab with mouse parked away shows leading icon, not ×", async ({ + page, + gotoSession, + }) => { + // Regression for the "leading icon + × both visible" state. PR #878 left a + // base-level rule: + // + // [data-component="tabs"] [data-slot="tabs-trigger-wrapper"]:has([data-selected]) + // [data-slot="tabs-trigger-close-button"] { opacity: 1 } + // + // That outranks the sidepanel slot's `opacity: 0` rest state (3 attrs vs 4), + // so every selected closable tab forced the × on regardless of hover. The + // user saw the × layered on top of the leading icon whenever they clicked + // a tab and moved the cursor away. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // Select Files (closable), then park the cursor far off the tab strip so + // neither :hover nor the swap rules fire. + await page.getByRole("tab", FILES_TAB).click() + await page.mouse.move(10, 700) + + const state = await page.evaluate(() => { + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const leading = wrap?.querySelector('[data-slot="tab-icon-default"]') as HTMLElement | null + const slot = wrap?.querySelector('[data-slot="tabs-trigger-close-button"]') as HTMLElement | null + return { + leadingOpacity: leading ? getComputedStyle(leading).opacity : null, + closeOpacity: slot ? getComputedStyle(slot).opacity : null, + selected: + wrap?.querySelector('[data-slot="tabs-trigger"][data-selected]') !== null, + } + }) + + expect(state.selected).toBe(true) + expect(state.leadingOpacity).toBe("1") + expect(state.closeOpacity).toBe("0") + }) + + test("hover paints a chip preview (--row-hover-overlay), selected wins", async ({ + page, + gotoSession, + }) => { + // Hover should preview the selection with a lighter overlay than the + // selected state — matches the sidebar session row's two-tier vocabulary + // (--row-hover-overlay 4%, --row-active-overlay 6%). Without this rule the + // titlebar tab strip felt dead on hover and clicks landed without any + // visual lead-in, which the user flagged as poor UX. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // Files is closable AND currently unselected (Status is the default). + // Hover it; assert chip bg is the hover overlay, not transparent and not + // the selected overlay. + await page.getByRole("tab", FILES_TAB).hover() + + const colors = await page.evaluate(() => { + // Resolve hover + active tokens through actual paint (same trick as + // the selected-overlay test) so we can compare rgba strings exactly. + const resolve = (varName: string) => { + const probe = document.createElement("div") + probe.style.backgroundColor = `var(${varName})` + document.body.appendChild(probe) + const out = getComputedStyle(probe).backgroundColor + probe.remove() + return out + } + const hoverExpected = resolve("--row-hover-overlay") + const activeExpected = resolve("--row-active-overlay") + // Chip backgrounds now live on the wrapper, not the trigger button. + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const bg = wrap ? getComputedStyle(wrap).backgroundColor : "" + return { bg, hoverExpected, activeExpected } + }) + + // Exact-token match: hover state must paint `--row-hover-overlay` + // verbatim, not "some other low-alpha rgba". The selected overlay is a + // distinct token at a different alpha; swapping them silently must fail + // here. + expect(colors.bg).toBe(colors.hoverExpected) + expect(colors.bg).not.toBe(colors.activeExpected) + }) + + test("selected tab on hover keeps the active overlay (selected wins)", async ({ + page, + gotoSession, + }) => { + // Regression guard for the cascade hazard caught in code review: the + // hover wrapper rule had a higher specificity (b=6, includes `:hover` + + // `:not(:disabled)`) than the selected rule (b=5, just `:has([data-selected])`), + // so hovering the *currently selected* tab repainted it with the + // lighter 4% hover overlay instead of the heavier 6% active overlay — + // the active tab visually weakened under the cursor. Title says + // "selected wins"; this test makes that promise testable. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + // Click Files so it becomes selected, THEN hover it (combined state). + await page.getByRole("tab", FILES_TAB).click() + await page.getByRole("tab", FILES_TAB).hover() + + const colors = await page.evaluate(() => { + const resolve = (varName: string) => { + const probe = document.createElement("div") + probe.style.backgroundColor = `var(${varName})` + document.body.appendChild(probe) + const out = getComputedStyle(probe).backgroundColor + probe.remove() + return out + } + const hoverExpected = resolve("--row-hover-overlay") + const activeExpected = resolve("--row-active-overlay") + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const bg = wrap ? getComputedStyle(wrap).backgroundColor : "" + return { bg, hoverExpected, activeExpected } + }) + + expect(colors.bg).toBe(colors.activeExpected) + expect(colors.bg).not.toBe(colors.hoverExpected) + }) + + test("short sidepanel tabs share a 72px min-width footprint", async ({ page, gotoSession }) => { + // Status / Files / Review labels are short (2-3 Chinese chars or 5-6 + // Latin chars). Without a floor they render visibly mismatched in the + // titlebar strip — the user's "宽度不统一难看" feedback. min-width 72px + // covers the natural width of a 2-char CJK label + icon + gap + padding + // (~53px) plus breathing room, so all 2-char tabs land on the same width + // in production (Chinese labels are full-width ~13px per char, uniform). + // Latin labels in the e2e environment (Status/Files/Review at 5-6 chars) + // also fall under the floor and land on the same width. Longer labels + // (Terminal at 8 chars, 上下文 at 3 CJK chars) extend past 72 naturally + // and the strip scrolls horizontally if the total exceeds the slot width. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + await page.mouse.move(0, 0) + + const widths = await page.evaluate(() => { + // Scope to the right-panel tablist so an unrelated Tabs instance + // elsewhere on the page (e.g. inside the Review content area) cannot + // pollute the measurement. + const root = document.querySelector( + '[data-component="tabs"][data-variant="sidepanel"][data-scope="right-panel"]', + ) + if (!root) return [] + return Array.from(root.querySelectorAll('[data-slot="tabs-trigger"]')).map((trig) => ({ + value: trig.getAttribute("data-value"), + width: Math.round(trig.getBoundingClientRect().width), + })) + }) + + const MIN_WIDTH_PX = 72 + // In production Chinese ("状态" / "文件" / "评审" all 2 full-width CJK + // chars) all three short tabs hit the min-width floor exactly. In the + // e2e Latin environment "Status" (40px) and "Files" (28.6px) still fall + // under the floor and snap to 72; "Review" (43.9px in system-ui) plus + // icon + gap + padding adds up to ~73px, just over the floor, so it + // renders at its natural width. Assert per-tab to lock both bands: + // - Status + Files: exactly 72 (the floor is the only thing keeping + // them uniform — drop min-width and they shrink visibly) + // - Review: at least 72 (it stretches naturally above the floor here, + // but in Chinese it lands at 72; tolerate either) + const byValue = Object.fromEntries(widths.map((w) => [w.value, w.width])) + expect(byValue.status).toBe(MIN_WIDTH_PX) + expect(byValue.files).toBe(MIN_WIDTH_PX) + expect(byValue.review).toBeGreaterThanOrEqual(MIN_WIDTH_PX) + }) + + test("closable wrapper has no trailing padding (parity with non-closable)", async ({ page, gotoSession }) => { + // Regression guard: base tabs CSS adds `padding-right: 12px` to any + // wrapper that owns a close-button slot. The sidepanel × is anchored on + // top of the leading icon (anchor positioning), so trailing padding has + // no purpose here — and leaving it in made closable wrappers (Files / + // Review / Terminal) visibly wider than Status at the same min-width. + // The sidepanel override resets it to 0; this test pins that. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + const paddings = await page.evaluate(() => { + // Scope to the right-panel tablist (see min-width test above). + const root = document.querySelector( + '[data-component="tabs"][data-variant="sidepanel"][data-scope="right-panel"]', + ) + if (!root) return [] + return Array.from( + root.querySelectorAll('[data-slot="tabs-trigger-wrapper"]'), + ).map((w) => ({ + value: w.getAttribute("data-value"), + hasCloseSlot: + w.querySelector('[data-slot="tabs-trigger-close-button"]') !== null, + paddingRight: getComputedStyle(w).paddingRight, + })) + }) + + const closable = paddings.filter((p) => p.hasCloseSlot) + expect(closable.length).toBeGreaterThan(0) + for (const p of closable) { + expect(p.paddingRight).toBe("0px") + } + }) + + test("Status (non-closable) shows no × on hover and keeps its icon", async ({ page, gotoSession }) => { + // Regression guard: PR #878 had a moment where Status's icon faded under + // a `:has(close-button-slot)` selector even though Status has no slot. + // The current contract is: Status icon never fades, no × ever renders. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + + await page.getByRole("tab", STATUS_TAB).hover() + const dims = await page.evaluate(() => { + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="status"]', + ) as HTMLElement | null + const icon = wrap?.querySelector('[data-slot="tab-icon-default"]') as HTMLElement | null + const closeSlot = wrap?.querySelector('[data-slot="tabs-trigger-close-button"]') + const iconOpacity = icon ? getComputedStyle(icon).opacity : null + return { iconOpacity, hasCloseSlot: closeSlot !== null } + }) + + expect(dims.iconOpacity).toBe("1") + expect(dims.hasCloseSlot).toBe(false) + + // Sanity: Review's wrapper still has the slot (closable peer). + const reviewHasSlot = await page.evaluate(() => + document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="review"] [data-slot="tabs-trigger-close-button"]', + ) !== null, + ) + expect(reviewHasSlot).toBe(true) + await page.getByRole("tab", REVIEW_TAB).hover() // also closes the hover test cleanly + }) + + test("chip geometry pins to the 4pt grid (gap, padding, radius)", async ({ page, gotoSession }) => { + // DESIGN.md L233 (4pt grid) + L305 (radius tiers sm/md/lg = 6/10/14). + // The previous production values `gap-1.5` (6px) and `px-2.5` (10px) + // drifted off the 4pt grid; PR #880 settled the full chip geometry: + // - list gap (between chips) : 4px (--space-xs) + // - trigger gap (icon ↔ label) : 8px (--space-sm) + // - trigger padding-inline (chip edge) : 4px (--space-xs) + // - wrapper border-radius (chip corner) : 10px (--radius-md) + // Pin every value so a future Tailwind/token tweak cannot quietly drift + // any one of them off the contract. + await gotoSession() + await openRightPanel(page) + await openExtraTabs(page) + await page.mouse.move(0, 0) + + const geometry = await page.evaluate(() => { + // Scope to the sidepanel-variant tabs (right-panel strip) — the doc + // may host other tablists whose spacing is unrelated. + const list = document.querySelector( + '[data-component="tabs"][data-variant="sidepanel"] [data-slot="tabs-list"]', + ) as HTMLElement | null + const wrap = document.querySelector( + '[data-slot="tabs-trigger-wrapper"][data-value="files"]', + ) as HTMLElement | null + const trigger = wrap?.querySelector('[data-slot="tabs-trigger"]') as HTMLElement | null + const listCS = list ? getComputedStyle(list) : null + const wrapCS = wrap ? getComputedStyle(wrap) : null + const trigCS = trigger ? getComputedStyle(trigger) : null + return { + listGap: listCS?.columnGap ?? null, + triggerGap: trigCS?.columnGap ?? null, + triggerPaddingLeft: trigCS?.paddingLeft ?? null, + triggerPaddingRight: trigCS?.paddingRight ?? null, + wrapperBorderRadius: wrapCS?.borderTopLeftRadius ?? null, + } + }) + + expect(geometry.listGap).toBe("4px") + expect(geometry.triggerGap).toBe("8px") + expect(geometry.triggerPaddingLeft).toBe("4px") + expect(geometry.triggerPaddingRight).toBe("4px") + expect(geometry.wrapperBorderRadius).toBe("10px") + }) +}) diff --git a/packages/app/e2e/session/titlebar-right-rail-contract.spec.ts b/packages/app/e2e/session/titlebar-right-rail-contract.spec.ts new file mode 100644 index 000000000..591f7490b --- /dev/null +++ b/packages/app/e2e/session/titlebar-right-rail-contract.spec.ts @@ -0,0 +1,130 @@ +import { expect, test } from "../fixtures" +import { openRightPanel } from "../actions" +import { modKey } from "../utils" + +// Contract for the titlebar's right rail — the flex row inside the titlebar's +// rightmost grid column that hosts: +// 1. `#pawwork-titlebar-right` → the right utility panel toggle (or +// StatusPopover fallback on non-session routes) +// 2. `#pawwork-titlebar-tabs` → the right-panel tab strip portal target +// +// PR #880 moved the tab strip from absolute overlay to an in-flow flex sibling +// of the toggle so the two own disjoint geometry (no pointer-events +// choreography needed). These tests guard the load-bearing properties of that +// new layout — properties that are easy to silently break by dropping a +// `self-stretch`, re-introducing absolute positioning on the slot, or +// forgetting to grow the toggle area when the tab set grows. +// +// Sibling specs: +// - session-tab-chip-contract.spec.ts — chip visuals & geometry (×, hover, +// selection overlay, 4pt grid pinning) + +test.describe("titlebar right rail contract", () => { + test("right utility toggle stays clickable with the full tab set open", async ({ + page, + gotoSession, + }) => { + // Regression guard: PR #878 had the titlebar tabs slot absolute-positioned + // over `#pawwork-titlebar-right`, relying on `pointer-events-none` / + // `-auto` choreography to keep the toggle clickable through the overlay. + // That broke at 4+ tabs because the `+` button got pushed into the + // toggle's x range and `pointer-events: auto` (required for the `+` + // dropdown to open) intercepted clicks meant for the toggle. + // Fix: the tabs slot is now an in-flow flex sibling of the toggle inside + // the titlebar's right rail (see `Titlebar` comments) — disjoint + // geometry, no overlay, no click choreography. This test opens the full + // default-reachable tab set so any future regression that reintroduces + // overlap (e.g. re-absolutising the slot, growing toggle into the rail) + // fails fast. + await gotoSession() + await openRightPanel(page) + // Open the maximum default-reachable tab set so the strip is widest. + await page.locator("main").first().click() + await page.keyboard.press(`${modKey}+\\`) // fileTree.toggle → Files + await page.keyboard.press(`${modKey}+Shift+R`) // review.toggle → Review + await page.keyboard.press("Control+`") // terminal.toggle → Terminal (always Ctrl) + await page.mouse.move(0, 0) + + const toggle = page.getByRole("button", { name: "Right utility panel" }) + await expect(toggle).toHaveAttribute("aria-expanded", "true") + // If anything in the tabs-portal overlay swallows the click, this + // times out with "subtree intercepts pointer events". + await toggle.click({ timeout: 3_000 }) + await expect(toggle).toHaveAttribute("aria-expanded", "false", { timeout: 2_000 }) + }) + + test("tabs slot shrinks to 0 width when viewport drops below the desktop breakpoint", async ({ + page, + gotoSession, + }) => { + // Regression guard from PR #880 followup review: `SessionSidePanel` gates + // its render on `createMediaQuery("(min-width: 768px)")`, but + // `--right-panel-width` and the titlebar's `tabsRailActive` only check + // `layout.rightPanel.opened()`. Without an explicit viewport gate, opening + // the panel at desktop width and then shrinking the viewport below 768px + // would leave the titlebar reserving panel-width of empty rail (no portal + // mounts under the breakpoint), pushing the right utility toggle off the + // viewport edge with nothing visible to justify the gap. + await gotoSession() + await openRightPanel(page) + // Sanity: rail occupies panel-width while we're still desktop. + const desktopTabsWidth = await page + .locator("#pawwork-titlebar-tabs") + .evaluate((el) => Math.round(el.getBoundingClientRect().width)) + expect(desktopTabsWidth).toBeGreaterThan(0) + + await page.setViewportSize({ width: 600, height: 900 }) + + // Poll until layout settles — viewport resize → media query → Solid memo → + // DOM update isn't synchronous, and reading geometry on the same tick that + // `setViewportSize` resolves can race the transition. + await expect + .poll( + () => + page.evaluate(() => { + const tabs = document.getElementById("pawwork-titlebar-tabs") as HTMLElement | null + const cs = tabs ? getComputedStyle(tabs) : null + return { + width: tabs ? Math.round(tabs.getBoundingClientRect().width) : null, + // `border-l` should be gone — no stray 1px line in a 0-width slot. + borderLeft: cs?.borderLeftWidth ?? null, + } + }), + { timeout: 2_000 }, + ) + .toEqual({ width: 0, borderLeft: "0px" }) + }) + + test("tabs slot border-l spans the full titlebar height (no top/bottom seam break)", async ({ + page, + gotoSession, + }) => { + // Regression guard from PR #880 followup review: with the tabs slot moved + // from absolute overlay to an in-flow flex sibling, its `self-stretch` only + // matches the parent flex container's content height. The titlebar root + // uses `items-center` (grid), which lets each grid cell collapse to its + // child's content box unless the cell opts out with `self-stretch` / `h-full`. + // If the right rail isn't full-height, the tabs slot's `border-l` also + // isn't full-height — it would visibly break above and below the toggle's + // 30px row, and stop meeting the right-panel body's `border-l` directly + // below the titlebar. This test pins the slot's painted height to the + // titlebar's own height so any future regression that drops the + // stretch chain fails fast. + await gotoSession() + await openRightPanel(page) + await page.mouse.move(0, 0) + + const heights = await page.evaluate(() => { + const titlebar = document.querySelector('[data-component="titlebar-shell"]') as HTMLElement | null + const tabs = document.getElementById("pawwork-titlebar-tabs") as HTMLElement | null + return { + titlebar: titlebar ? Math.round(titlebar.getBoundingClientRect().height) : null, + tabs: tabs ? Math.round(tabs.getBoundingClientRect().height) : null, + } + }) + + expect(heights.titlebar).not.toBeNull() + expect(heights.tabs).not.toBeNull() + expect(heights.tabs).toBe(heights.titlebar) + }) +}) diff --git a/packages/app/e2e/snap/right-panel-tabs-hover.snap.ts b/packages/app/e2e/snap/right-panel-tabs-hover.snap.ts new file mode 100644 index 000000000..4f5bcc1cd --- /dev/null +++ b/packages/app/e2e/snap/right-panel-tabs-hover.snap.ts @@ -0,0 +1,136 @@ +import { expect, type Page } from "@playwright/test" +import { openRightPanel, openSidebar } from "../actions" +import { test } from "../fixtures" +import { sessionItemSelector } from "../selectors" +import { applyDarkModeForTests } from "../utils" +import { composeGrid, snapOutputPath, type Shot } from "./_compose" + +// Hover-state probe for the right-panel tab strip. Captures three states of +// the closable Files tab so we can verify the close-button affordance: +// +// 1) rest — Files tab at rest, leading icon only, no close button +// 2) hover — mouse hovering Files tab; expectation: leading icon +// fades to opacity 0, close × fades in at the same 14×14 +// cell, no layout shift, no double-icon visible +// 3) measurements — page-level dump of bounding boxes for the close-button +// slot vs leading icon vs label so we know exactly where +// each piece lands in screen coordinates. +// +// Why not extend right-panel-titlebar.snap.ts: that snap intentionally moves +// the mouse to (0,0) to freeze a clean static frame. Hover state needs its +// own target so the two contracts don't fight. + +test.use({ viewport: { width: 1440, height: 900 }, deviceScaleFactor: 2, reducedMotion: "reduce" }) + +async function openFilesAndReview(page: Page) { + await page.locator("main").first().click() + await page.keyboard.press("ControlOrMeta+\\") + await page.keyboard.press("ControlOrMeta+Shift+R") + await expect.poll(() => page.getByRole("tab").count(), { timeout: 5_000 }).toBe(3) + await page.getByRole("tab", { name: "Status" }).click() +} + +async function dumpBoxes(page: Page, label: string) { + // Dump the bounding boxes of every interesting element in the tab strip so + // we can reason about exact positions vs the CSS-declared 14×14 + left:10 + // contract in tabs.css. + const data = await page.evaluate(() => { + const tabs = Array.from( + document.querySelectorAll('[data-slot="tabs-trigger-wrapper"]'), + ) + return tabs.map((wrap) => { + const trig = wrap.querySelector('[data-slot="tabs-trigger"]') + const icon = wrap.querySelector('[data-slot="tab-icon-default"]') + const iconSvg = icon?.querySelector('[data-component="icon"]') + const closeSlot = wrap.querySelector('[data-slot="tabs-trigger-close-button"]') + const closeBtn = closeSlot?.querySelector('[data-component="icon-button"]') + const closeBtnIcon = closeBtn?.querySelector('[data-component="icon"]') + const cs = (el?: HTMLElement | null) => (el ? window.getComputedStyle(el) : null) + const box = (el?: HTMLElement | null) => { + if (!el) return null + const r = el.getBoundingClientRect() + return { x: Math.round(r.x), y: Math.round(r.y), w: Math.round(r.width), h: Math.round(r.height) } + } + return { + value: wrap.getAttribute("data-value"), + selected: trig?.getAttribute("data-selected") === "" || trig?.hasAttribute("data-selected"), + wrapper: box(wrap), + trigger: box(trig), + leadingSpan: box(icon), + leadingIconDiv: box(iconSvg), + closeSlot: box(closeSlot), + closeBtn: box(closeBtn), + closeBtnIcon: box(closeBtnIcon), + closeSlotOpacity: cs(closeSlot)?.opacity, + leadingSpanOpacity: cs(icon)?.opacity, + triggerFontWeight: cs(trig)?.fontWeight, + } + }) + }) + process.stdout.write(`\n[boxes ${label}]\n${JSON.stringify(data, null, 2)}\n`) +} + +// Tab-strip clip: shared by every shot so all states crop identically and +// diff cleanly in the grid. +const TAB_STRIP_CLIP = { x: 600, y: 0, width: 700, height: 80 } as const + +async function captureShot(page: Page, name: string): Promise { + await page.waitForTimeout(200) + await dumpBoxes(page, name) + return { + name, + buf: await page.screenshot({ clip: TAB_STRIP_CLIP, animations: "disabled" }), + } +} + +async function captureStates( + page: Page, + label: "light" | "dark", + sessionID: string, +): Promise { + await openSidebar(page) + await page.locator(sessionItemSelector(sessionID)).click() + await openRightPanel(page) + await openFilesAndReview(page) + + // Rest state — mouse parked far away so no tab hovers. + await page.mouse.move(0, 0) + const rest = await captureShot(page, `${label}-rest`) + + // Hover state — hover the Files tab (closable). This should fade the + // leading icon and reveal the × close button. + await page.getByRole("tab", { name: "Files" }).hover() + const hoverFiles = await captureShot(page, `${label}-hover-files`) + + // Hover state on Review (also closable). openFilesAndReview clicks Status, + // so Review/Files are open but unselected — same expected swap as Files. + await page.getByRole("tab", { name: "Review" }).hover() + const hoverReview = await captureShot(page, `${label}-hover-review`) + + return [rest, hoverFiles, hoverReview] +} + +test("right-panel-tabs-hover", async ({ page, project }) => { + test.setTimeout(180_000) + + let sessionID: string | undefined + await project.open({ + beforeGoto: async ({ sdk }) => { + const session = await sdk.session.create({ title: "snap right panel tabs hover" }).then((res) => res.data) + sessionID = session?.id + // Track immediately on creation so cleanup runs even if a later + // step in project.open() throws before this test completes. + if (sessionID) project.trackSession(sessionID) + }, + }) + if (!sessionID) throw new Error("Session create did not return an id") + + const shots: Shot[] = [] + shots.push(...(await captureStates(page, "light", sessionID))) + await applyDarkModeForTests(page) + shots.push(...(await captureStates(page, "dark", sessionID))) + + const out = snapOutputPath("right-panel-tabs-hover") + await composeGrid(shots, out) + process.stdout.write(`\n[snap] right-panel-tabs-hover grid -> ${out}\n\n`) +}) diff --git a/packages/app/src/components/session/session-sortable-shell-tab.tsx b/packages/app/src/components/session/session-sortable-shell-tab.tsx index 247358b98..857f153e7 100644 --- a/packages/app/src/components/session/session-sortable-shell-tab.tsx +++ b/packages/app/src/components/session/session-sortable-shell-tab.tsx @@ -69,9 +69,12 @@ export function ShellTab(props: { props.onClose(props.value) // Wait one tick for Solid to commit the DOM removal, then restore focus. + // Re-check `isConnected` inside the rAF: rapid successive closes can + // remove the captured sibling before its frame fires, and calling + // `.focus()` on a detached node silently moves focus to . if (focusTarget) { requestAnimationFrame(() => { - focusTarget?.focus() + if (focusTarget?.isConnected) focusTarget.focus() }) } } @@ -90,15 +93,23 @@ export function ShellTab(props: { value={props.value} class="shrink-0 h-full" classes={{ - button: - "h-7 min-h-7 inline-flex items-center whitespace-nowrap rounded-md text-h3 text-fg-weak gap-1.5 px-2.5", + // Spacing (gap, padding-inline, border-radius) lives in tabs.css under + // the sidepanel variant: icon↔label gap `--space-sm` (8), chip-edge + // padding `--space-xs` (4), corner `--radius-md` (10). Tailwind + // utilities are kept out here because the app pins + // `html { font-size: 13px }`, which makes the default rem-based + // spacing scale drift off the 4pt grid (gap-2 = 0.5rem = 6.5px + // instead of 8). Routing through CSS variables in the variant block + // keeps the chip exactly on the grid. + button: "h-7 min-h-7 inline-flex items-center whitespace-nowrap text-h3 text-fg-weak", }} onMiddleClick={close} aria-label={props.label} - // Advertise the Delete key shortcut to assistive technology — only on - // closable tabs, since Status's onKeyDown is a no-op and exposing the - // shortcut there would be a false promise. - aria-keyshortcuts={props.closable ? "Delete" : undefined} + // Advertise both close shortcuts to assistive technology — handler + // below accepts Delete OR Backspace (the macOS alias). Space-separated + // per ARIA spec. Only declared on closable tabs; Status's onKeyDown is + // a no-op and exposing the shortcut there would be a false promise. + aria-keyshortcuts={props.closable ? "Delete Backspace" : undefined} onKeyDown={ props.closable ? (event: KeyboardEvent) => { diff --git a/packages/app/src/components/titlebar.tsx b/packages/app/src/components/titlebar.tsx index 118104616..9dc8d2d75 100644 --- a/packages/app/src/components/titlebar.tsx +++ b/packages/app/src/components/titlebar.tsx @@ -1,5 +1,6 @@ import { createEffect, createMemo, Show, untrack } from "solid-js" import { createStore } from "solid-js/store" +import { createMediaQuery } from "@solid-primitives/media" import { useLocation, useNavigate, useParams } from "@solidjs/router" import { Icon } from "@opencode-ai/ui/icon" import { Button } from "@opencode-ai/ui/button" @@ -22,6 +23,23 @@ export function Titlebar() { const mac = createMemo(() => isMacShell(platform)) const windows = createMemo(() => isWindowsShell(platform)) + // Must match `SessionSidePanel`'s own desktop gate — that component only + // mounts the panel (and portals tab content into the titlebar) at ≥768px. + // Without this same predicate, opening the panel at desktop width and then + // resizing below the breakpoint would leave the titlebar reserving + // panel-width of empty rail (no portal to fill it), pushing the right + // utility toggle off the viewport. Single source of truth for "is the + // right panel actually visible right now": route + state + viewport. + const isDesktop = createMediaQuery("(min-width: 768px)") + // Tabs rail is only meaningful on session routes — `--right-panel-width` + // is a global CSS var that survives navigation, so without this gate the + // tabs slot would still claim panel-width on home/settings (where + // SessionSidePanel doesn't render any tabs), pushing the right utility + // toggle's StatusPopover fallback to the left. + const tabsRailActive = createMemo( + () => isDesktop() && location.pathname.includes("/session") && layout.rightPanel.opened(), + ) + const tabsRailWidth = () => (tabsRailActive() ? "var(--right-panel-width, 0px)" : "0px") const zoom = () => platform.webviewZoom?.() ?? 1 const currentTitlebarHeight = () => mac() ? "var(--shell-titlebar-current-height, var(--shell-titlebar-height, 44px))" : undefined @@ -149,62 +167,74 @@ export function Titlebar() {
-
+ {/* Right titlebar rail. Two in-flow flex siblings, ordered left→right: + (1) `#pawwork-titlebar-right` — the right utility toggle (or + StatusPopover fallback on non-session routes), portalled in + by SessionHeader. + (2) `#pawwork-titlebar-tabs` — the right-panel tab strip, portalled + in by SessionSidePanel only when the panel is open. + + The tabs slot's width follows `var(--right-panel-width)` so it + occupies the same x-range as the right-panel body directly below + and the `border-l` reads as one continuous separator from titlebar + top to viewport bottom. Because the two slots are flex siblings + (not an absolute overlay over the toggle), the toggle is naturally + pushed left by `--right-panel-width` when the panel opens and + slides back to the viewport edge when it closes. The 240ms + transition on `--right-panel-width` carries the toggle smoothly + along with the panel edge, and no pointer-events choreography is + needed — the toggle and the tab strip own disjoint geometry. + + "Borrowed identity": the tabs slot stamps `data-component="tabs"` + + `data-variant="sidepanel"` + `data-scope` + `data-orientation` + so the descendant selectors in `packages/ui/src/components/tabs.css` + (e.g. `[data-component="tabs"] [data-slot="tabs-list"]`) match the + portalled `Tabs.List`. The base `[data-component="tabs"]` rule + also sets `flex-direction: column` on the host (expecting + Tabs.List + Tabs.Content stacked vertically); the slot's own + `flex-row` class flips that locally so it stays a horizontal + strip. + + `border-l` and the panel-width track only when `tabsRailActive` + (session route + right panel open). On home/settings the slot + shrinks to 0 width — without this gate, navigating away while + the panel was left open would still claim panel-width in the + titlebar (the CSS var survives navigation) and push the + StatusPopover fallback to the left. + + `pr-2` lives on `#pawwork-titlebar-right` (not the outer rail) + so it reads as "toggle inset from viewport edge" when the panel + is closed and "gap between toggle and tabs border-l" when open. + Putting it on the outer rail would shift the tabs slot 8px + inboard of the viewport, misaligning its `border-l` with the + right-panel body's `border-l` directly below it. + + `self-stretch` on the rail is load-bearing — the titlebar root + uses `items-center`, which lets each grid cell collapse to its + child's content height. Without this opt-out, the tabs slot's + `self-stretch` would only reach the rail's content height + (≈30px toggle row), and its `border-l` would break above and + below the toggle row instead of meeting the right-panel body's + `border-l` as one continuous separator. */} +
+
- - {/* Portal slot for the right-panel tab bar. Lives inside the titlebar so the - tabs read as part of the window chrome rather than a second toolbar - beneath it. The slot sits directly above the right-panel body — same - width (`var(--right-panel-width)`) and anchored to the viewport's right - edge (`right: 0`). `border-l` puts the 1px on the slot's left edge, - which is the same x as `right-panel-body`'s `border-l` immediately - below it, so the two read as one continuous separator from titlebar - top to viewport bottom. - - The `data-component="tabs"` + `data-variant="sidepanel"` + `data-scope` - + `data-orientation` attributes mirror what - renders on its root. Portalling Tabs.List takes it out of that ancestor, - so the CSS in packages/ui/src/components/tabs.css (which uses descendant - selectors like `[data-component="tabs"] [data-slot="tabs-list"]`) would - otherwise miss it — no flex, no height, no sidepanel hover/selected - colors. Stamping the same data attrs here lets all existing selectors - re-match without forking the stylesheet. - - `flex-row` is intentional and not redundant: the same `[data-component="tabs"]` - rule that we are inheriting also sets `flex-direction: column` on the host - (it expects to wrap Tabs.List + Tabs.Content vertically). Without an explicit - override, the slot ends up as a column flex container and `items-center` would - align its single child horizontally instead of vertically, leaving the tabs - glued to the top of the titlebar. - - Only populated when the right panel is open (SessionSidePanel guards its Portal). */} - {/* `pointer-events-none` on the slot mirrors `#pawwork-titlebar-left` — only - the portalled tab buttons (which carry their own `pointer-events-auto` - via the sidepanel CSS variant) should swallow clicks. Otherwise the - slot's z-10 box covers the Right utility panel toggle in - `#pawwork-titlebar-right` whenever the panel is open, making the toggle - unclickable (caught by perf-probe-baseline's session-streaming-long - run on PR #878). */} -
) } diff --git a/packages/app/src/index.css b/packages/app/src/index.css index cfb7fab39..4bc965a74 100644 --- a/packages/app/src/index.css +++ b/packages/app/src/index.css @@ -127,9 +127,45 @@ border-top-width: 0; } + /* Right-panel tab chip backgrounds share the sidebar session row's + * vocabulary, on the WRAPPER (matches the sidebar's chip-on-container + * shape, not the inner trigger button): + * hover (non-selected) → --row-hover-overlay (4% overlay) + * selected → --row-active-overlay (6% overlay) + * One selection language across the app; the lighter hover overlay + * previews where a click will land. Replaces the previous opaque + * var(--sidebar) chip, which read too heavy now that the tab strip lives + * in the titlebar. Paired with the fg-weak → fg-strong color shift in + * tabs.css; font-weight is not used as a selection marker (all sidepanel + * triggers already inherit 500 from text-h3, and bumping a subset would + * shift tab widths). + * + * `:not(:disabled)` is here to bump specificity (5 → 6) above the + * sidepanel base rule in tabs.css: + * [data-component="tabs"][data-variant="sidepanel"] + * [data-slot="tabs-trigger-wrapper"]:hover:not(:disabled):not([data-selected]) + * { background-color: transparent } + * which is 6 and would otherwise force wrapper bg back to transparent on + * hover. Equal specificity now — order wins, this rule declared later + * (after the @import) takes effect. + * + * Selected+hover precedence is handled by the *next* rule (below), which + * pulls the same trick to outrank this hover rule by source order so the + * heavier active overlay always wins on the focused chip. */ [data-component="tabs"][data-variant="sidepanel"][data-scope="right-panel"] - [data-slot="tabs-trigger"][data-selected] { - background-color: var(--sidebar); + [data-slot="tabs-trigger-wrapper"]:hover:not(:disabled) { + background-color: var(--row-hover-overlay); + } + + /* `:not(:disabled)` bumps this rule's b-count to 6 (matching the hover + * rule above), so source order wins — this rule is declared last, so + * selected+hover paints the heavier active overlay instead of the lighter + * hover overlay. Without it, hover at b=6 outranked selected at b=5 and + * the active tab visually weakened under the cursor (regression caught in + * code review). */ + [data-component="tabs"][data-variant="sidepanel"][data-scope="right-panel"] + [data-slot="tabs-trigger-wrapper"]:not(:disabled):has([data-selected]) { + background-color: var(--row-active-overlay); } @media (min-width: 1280px) { diff --git a/packages/app/src/pages/session/session-side-panel.tsx b/packages/app/src/pages/session/session-side-panel.tsx index e50711bae..ff8b43538 100644 --- a/packages/app/src/pages/session/session-side-panel.tsx +++ b/packages/app/src/pages/session/session-side-panel.tsx @@ -353,11 +353,13 @@ export function SessionSidePanel(props: { {(mount) => ( - {/* `pointer-events-auto` brings clicks back here — the titlebar slot - itself is `pointer-events-none` (see Titlebar) so the rest of the - slot box does not occlude the Right utility panel toggle that sits - in the right portal beneath it. */} - + {/* `gap` is intentionally omitted — the sidepanel variant + in tabs.css owns the inter-tab gap via `var(--space-xs)` + (4px / 4pt-grid). Tailwind's `gap-1` would sit in the + utilities layer and outrank the components-layer rule, + and the rem-13 base would also drift it off the grid + (gap-1 = 0.25rem = 3.25px, not 4). */} + {(tab) => ( @@ -384,6 +386,9 @@ export function SessionSidePanel(props: { )} + {/* Spacer pushes the `+` button to the rail's right edge so + the chip strip reads left-justified and `+` lives at the + end of the rail (matching docs/design/ui_kits/desktop/RightPanel.jsx). */}
{/* 40px right-gutter reserve — matches docs/design/src/rightpanel.jsx, gives the tab row breathing room against the panel edge. */} diff --git a/packages/ui/src/components/icon-button.css b/packages/ui/src/components/icon-button.css index b597368d7..8fe0ee7d1 100644 --- a/packages/ui/src/components/icon-button.css +++ b/packages/ui/src/components/icon-button.css @@ -76,15 +76,21 @@ } } -/* titlebar-icon: 32×30 non-square, DESIGN.md §346 — only 4 uses in titlebar.tsx */ -[data-component="icon-button"].titlebar-icon { +/* titlebar-icon: 32×30 non-square, DESIGN.md §346. + Lives in icon-button.css for historical reasons but is intentionally NOT + scoped to `[data-component="icon-button"]` — the four titlebar usages in + `titlebar.tsx` + `session-header.tsx` render `