diff --git a/.storybook/test-runner.ts b/.storybook/test-runner.ts index a9aab189e5f..77a28909c49 100644 --- a/.storybook/test-runner.ts +++ b/.storybook/test-runner.ts @@ -2,14 +2,14 @@ import type { TestRunnerConfig } from "@storybook/test-runner"; // This hook is reloaded between Jest suites in the same worker. Importing @playwright/test // initializes a second test runner and fails on reload; use browser waits and Node assertions. import assert from "node:assert/strict"; -import { MOBILE_TOUCH_TARGET_PX } from "../src/constants/layout"; +import { MOBILE_TOUCH_TARGET_PX, TABLET_TOUCH_TARGET_PX } from "../src/constants/layout"; // Real :hover for the primary-button contrast stories (#6022). A play runs inside the page and // cannot set :hover, so only stories under this id prefix get two page-level helpers backed by // Playwright. All other stories run exactly as before. const HOVER_STORY_PREFIX = "app-primarybuttoncontrast--"; const hoverReady = new WeakSet(); -// The Narrow story's strip scrolls up to 621 px, and the shortest swipe seen moved it 56 px: +// The Narrow story's strip scrolls up to 669 px, and the shortest swipe seen moved it 56 px: // 16 swipes cover it with room to spare. const MAX_TOUCH_SWIPES = 16; @@ -71,6 +71,62 @@ async function assertPhoneFooterTouchTargets(page: StoryPage) { } } +const TOUCH_RESIZE_STORY = "app-seededworkspacea11y--narrow-sidebar-dark"; + +/** + * A finger must drag the right sidebar's resize handle: a touch drag fires no mouse events, and + * iPads report a coarse pointer even with a trackpad. Plays dispatch synthetic events, so only + * browser input proves the handle takes a touch drag (and that the page does not pan instead). + */ +async function assertTouchResizesRightSidebar(page: StoryPage) { + const client = await page.context().newCDPSession(page); + const viewport = page.viewportSize(); + // The drag persists the new width; restore storage for the stories that run after this one. + const storage = await page.evaluate(() => Object.entries(localStorage)); + const sidebar = page.getByRole("complementary", { name: "Workspace insights" }); + try { + await client.send("Emulation.setTouchEmulationEnabled", { enabled: true }); + await page.setViewportSize({ width: 1200, height: 900 }); + await page.waitForFunction(() => matchMedia("(pointer: coarse)").matches, undefined, { + timeout: 5000, + }); + const before = await sidebar.boundingBox(); + const handle = await page.locator("[data-sidebar-resize-handle]").boundingBox(); + if (!before || !handle) throw new Error("Right sidebar or its resize handle is missing"); + const x = handle.x + handle.width / 2; + const y = handle.y + handle.height / 2; + await client.send("Input.dispatchTouchEvent", { type: "touchStart", touchPoints: [{ x, y }] }); + for (let step = 1; step <= 4; step++) { + // A frame per move: the drag listeners attach in an effect after the touch starts. + await page.evaluate( + () => new Promise((resolve) => requestAnimationFrame(() => resolve())) + ); + await client.send("Input.dispatchTouchEvent", { + type: "touchMove", + touchPoints: [{ x: x - 20 * step, y }], + }); + } + await client.send("Input.dispatchTouchEvent", { type: "touchEnd", touchPoints: [] }); + // Dragging the left-edge handle 80 px left widens the sidebar by as much. + await page.waitForFunction( + (start) => { + const element = document.querySelector('[aria-label="Workspace insights"]'); + return element != null && element.getBoundingClientRect().width >= start + 60; + }, + before.width, + { timeout: 5000 } + ); + } finally { + await client.send("Emulation.setTouchEmulationEnabled", { enabled: false }); + await client.detach(); + if (viewport) await page.setViewportSize(viewport); + await page.evaluate((entries) => { + localStorage.clear(); + for (const [key, value] of entries) localStorage.setItem(key, value); + }, storage); + } +} + const config: TestRunnerConfig = { async preVisit(page, context) { // exposeFunction throws on a second registration, so register once per Playwright page. @@ -86,24 +142,43 @@ const config: TestRunnerConfig = { } return; } + if (context.id === TOUCH_RESIZE_STORY) { + if (page.context().browser()?.browserType().name() === "chromium") { + await assertTouchResizesRightSidebar(page); + } + return; + } if (context.id === "app-rightsidebar-stable-tab-controls--wide") { if (page.context().browser()?.browserType().name() !== "chromium") return; - // A tablet (coarse pointer above the phone breakpoint) gets no global 44px button floor, - // so the collapse button must size itself for touch. Plays cannot emulate the pointer. + // A tablet (coarse pointer above the phone breakpoint) gets no global 44px button floor. + // Its strip keeps the desktop height (44px rows made it far taller than the header), so + // the controls must size themselves to the 24px minimum. Plays cannot emulate the pointer. const client = await page.context().newCDPSession(page); const viewport = page.viewportSize(); + const strip = page.getByRole("tablist").locator(".."); try { - await client.send("Emulation.setTouchEmulationEnabled", { enabled: true }); await page.setViewportSize({ width: 1200, height: 900 }); + const desktop = await strip.boundingBox(); + await client.send("Emulation.setTouchEmulationEnabled", { enabled: true }); await page.waitForFunction(() => matchMedia("(pointer: coarse)").matches, undefined, { timeout: 5000, }); - const box = await page.getByRole("button", { name: "Collapse sidebar" }).boundingBox(); - if (!box) throw new Error("Collapse sidebar button is missing"); - assert( - box.width >= 44 && box.height >= 44, - `Collapse target is ${box.width}x${box.height}` - ); + const tablet = await strip.boundingBox(); + if (!desktop || !tablet) throw new Error("Tab strip is missing"); + assert.equal(tablet.height, desktop.height, "Touch must not grow the tablet tab strip"); + for (const name of [ + "Close New tab", + "Close Instructions", + "All tabs", + "Collapse sidebar", + ]) { + const box = await page.getByRole("button", { name, exact: true }).boundingBox(); + if (!box) throw new Error(`${name} button is missing`); + assert( + box.width >= TABLET_TOUCH_TARGET_PX && box.height >= TABLET_TOUCH_TARGET_PX, + `${name} target is ${box.width}x${box.height}` + ); + } } finally { await client.send("Emulation.setTouchEmulationEnabled", { enabled: false }); await client.detach(); @@ -173,12 +248,13 @@ const config: TestRunnerConfig = { } await client.send("Input.dispatchTouchEvent", { type: "touchEnd", touchPoints: [] }); await row.and(page.locator("[data-scroll-settled]")).waitFor({ timeout: 5000 }); + // The tap below lands 20 px into the last tab's visible part: on a phone, a long title + // can be wider than the row, so the tab's own left edge never scrolls into view. const reachedLastTab = await row.evaluate((el) => { const last = Array.from(el.querySelectorAll('[role="tab"]')).at(-1); if (!last) return false; const bounds = el.getBoundingClientRect(); - const tapX = last.getBoundingClientRect().left + 20; - return tapX >= bounds.left && tapX < bounds.right; + return Math.max(last.getBoundingClientRect().left, bounds.left) + 20 < bounds.right; }); if (reachedLastTab) break; } @@ -196,11 +272,11 @@ const config: TestRunnerConfig = { const last = await tabs.last().boundingBox(); const bounds = await row.boundingBox(); if (!last || !bounds) throw new Error("Scrolled tab is missing"); - assert(last.x + 20 >= bounds.x, "Last tab must be inside the left edge"); - assert(last.x + 20 < bounds.x + bounds.width, "Last tab must be inside the right edge"); + const tapX = Math.max(last.x, bounds.x) + 20; + assert(tapX < bounds.x + bounds.width, "Last tab must be inside the right edge"); await client.send("Input.dispatchTouchEvent", { type: "touchStart", - touchPoints: [{ x: last.x + 20, y: last.y + last.height / 2 }], + touchPoints: [{ x: tapX, y: last.y + last.height / 2 }], }); await client.send("Input.dispatchTouchEvent", { type: "touchEnd", touchPoints: [] }); await tabs.last().and(page.locator('[aria-selected="true"]')).waitFor({ timeout: 5000 }); diff --git a/docs/config/keybinds.mdx b/docs/config/keybinds.mdx index d3e5358b949..a5c5b5bd03b 100644 --- a/docs/config/keybinds.mdx +++ b/docs/config/keybinds.mdx @@ -72,7 +72,7 @@ While a workspace's actions menu (`⋮` in the sidebar row, `…` in the header) | New right sidebar tab (lists the tools) | `Ctrl+Alt+N` | | Close the right sidebar's active tab | `Ctrl+W` | -The right sidebar shows only the tabs you open. A new tab lists the tools it can show (Stats, Review, Instructions, Goal, Terminal, and more); pick one to open it in that tab's place. +The right sidebar shows only the tabs you open. A new tab lists the tools it can show (Stats, Review, Instructions, Goal, Terminal, and more); pick one to open it in that tab's place. The chevron at the right end of the tab strip lists your open tabs, with **New tab** at the bottom. ### Command Palette diff --git a/src/browser/components/LeftSidebar/LeftSidebar.stories.tsx b/src/browser/components/LeftSidebar/LeftSidebar.stories.tsx index 2ed80b3905b..e637086733a 100644 --- a/src/browser/components/LeftSidebar/LeftSidebar.stories.tsx +++ b/src/browser/components/LeftSidebar/LeftSidebar.stories.tsx @@ -65,7 +65,7 @@ interface LeftSidebarStoryOverrides { collapsed?: boolean; widthPx?: number; isResizing?: boolean; - onStartResize?: (e: React.MouseEvent) => void; + onStartResize?: (e: React.PointerEvent) => void; onToggleCollapsed?: () => void; } diff --git a/src/browser/components/LeftSidebar/LeftSidebar.tsx b/src/browser/components/LeftSidebar/LeftSidebar.tsx index d25de30e6c8..43886ccc393 100644 --- a/src/browser/components/LeftSidebar/LeftSidebar.tsx +++ b/src/browser/components/LeftSidebar/LeftSidebar.tsx @@ -11,7 +11,7 @@ interface LeftSidebarProps { onToggleCollapsed: () => void; widthPx?: number; isResizing?: boolean; - onStartResize?: (e: React.MouseEvent) => void; + onStartResize?: (e: React.PointerEvent) => void; sortedWorkspacesByProject: Map; workspaceRecency: Record; } @@ -100,10 +100,10 @@ export function LeftSidebar(props: LeftSidebarProps) {
onStartResize(e)} + onPointerDown={(e) => onStartResize(e)} /> )}
diff --git a/src/browser/features/RightSidebar/NewTabLauncher.tsx b/src/browser/features/RightSidebar/NewTabLauncher.tsx index 7127df4138e..a972815ae08 100644 --- a/src/browser/features/RightSidebar/NewTabLauncher.tsx +++ b/src/browser/features/RightSidebar/NewTabLauncher.tsx @@ -26,9 +26,9 @@ interface NewTabLauncherProps { onOpenSideChat?: () => void; creatingSideChat: boolean; /** - * Focus the first row once shown. Set only when the user asked for a New tab ("+" or the - * shortcut), never for the New tab a fresh workspace starts with, so the launcher does not - * steal focus from the chat input. + * Focus the first row once shown. Set only when the user asked for a New tab (its row in the + * All tabs menu or the shortcut), never for the New tab a fresh workspace starts with, so the + * launcher does not steal focus from the chat input. */ autoFocus: boolean; onAutoFocusConsumed: () => void; diff --git a/src/browser/features/RightSidebar/RightSidebar.stories.tsx b/src/browser/features/RightSidebar/RightSidebar.stories.tsx index 9fca404356e..23754eed881 100644 --- a/src/browser/features/RightSidebar/RightSidebar.stories.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.stories.tsx @@ -32,7 +32,7 @@ import { createAssistantMessage, createUserMessage } from "@/browser/stories/moc import type { MockSessionUsage } from "@/browser/stories/mocks/orpc"; import { AppWithMocks, PIXEL_DISABLED } from "@/browser/stories/meta.js"; import { createWorkspace } from "@/browser/stories/mocks/workspaces"; -import { blurActiveElement } from "@/browser/stories/storyPlayHelpers"; +import { blurActiveElement, openSidebarNewTab } from "@/browser/stories/storyPlayHelpers"; import { setupSimpleChatStory, setupStreamingChatStory } from "@/browser/stories/helpers/chatSetup"; import { textsBelowAaInThemes } from "@/browser/stories/helpers/contrast"; import { setHunkFirstSeen } from "@/browser/stories/helpers/reviews"; @@ -206,7 +206,7 @@ function seedSidebarLayout(workspaceId: string, tabs: TabType[], activeTab: TabT /** * The selected tab lies fully inside its strip's scroll viewport (not cut off at an edge or - * hidden behind "+"). Retries because labels grow after mount (cost badge, counts) and the + * hidden behind All tabs). Retries because labels grow after mount (cost badge, counts) and the * strip re-scrolls when they do. Stories using it fix the sidebar width themselves, so this * holds at any test-runner viewport size. */ @@ -1585,15 +1585,27 @@ export const NewTabLauncher: Story = { document.activeElement === tab || panel?.contains(document.activeElement) ).toBe(true); }); - await userEvent.click(canvas.getByRole("button", { name: "New tab" })); + await openSidebarNewTab(canvasElement); } + // The New tab row hands focus to the launcher, which keeps it after the menu unmounts: Radix + // refocuses the menu button on a timer then if focus is still inside the menu. + const launcherHasFocus = () => + canvasElement + .querySelector('[role="tabpanel"][id$="-panel-new"]') + ?.contains(document.activeElement) === true; + await waitFor(() => expect(launcherHasFocus()).toBe(true)); + await waitFor(() => + expect(within(document.body).queryByRole("dialog", { name: "All tabs" })).toBeNull() + ); + await new Promise((resolve) => setTimeout(resolve, 50)); + await expect(launcherHasFocus()).toBe(true); }, }; /** * The New tab launcher at the sidebar's minimum width (300px), next to more open tabs than fit: * launcher rows wrap their descriptions instead of overflowing, and the strip stays one row - * that scrolls sideways (hidden scrollbar, faded edges) with "+" pinned at the end. + * that scrolls sideways (hidden scrollbar, faded edges) with All tabs pinned at the end. */ export const NewTabLauncherNarrow: Story = { // Match Pixel's named desktop exactly; the global Storybook "desktop" is smaller. @@ -1721,7 +1733,7 @@ export const ManyTabsSingleRow: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); await canvas.findByRole("tab", { name: /^timeline/i, selected: true }); - await canvas.findByRole("button", { name: "New tab" }); + await canvas.findByRole("button", { name: "All tabs" }); // Wait for the cost badge: it widens Stats after mount, pushing the selected tab right. await canvas.findByRole("tab", { name: /stats.*\$0\.34/i }); await expectSelectedTabVisible(canvasElement); @@ -1770,8 +1782,12 @@ export const TabsReachableUnderWideTitlebarInset: Story = { const stats = await canvas.findByRole("tab", { name: /^stats/i, selected: true }); const review = await canvas.findByRole("tab", { name: /^review/i }); await expectSelectedTabVisible(canvasElement); + // All tabs must stay clickable too: it reaches every tab and the New tab. await waitFor(() => { if (!isHitTarget(stats)) throw new Error("selected tab is not under the pointer"); + if (!isHitTarget(canvas.getByRole("button", { name: "All tabs" }))) { + throw new Error("All tabs is not under the pointer"); + } }); // Like Playwright: bring the other tab into view, then it must take the click. review.scrollIntoView({ block: "nearest", inline: "nearest" }); diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index dd18c543fa5..61d7912310f 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -261,9 +261,14 @@ const SidebarContainer: React.FC = ({ // (0.75rem), so it sits evenly in the corner. isDesktop ? "top-12" : "top-11", ] - : // No width transition: expanding swaps the floating card for the docked panel, and - // animating the width between those two layouts reads as the card stretching. - "bg-surface-primary flex-shrink-0 overflow-hidden border-l" + : [ + // No width transition: expanding swaps the floating card for the docked panel, and + // animating the width between those two layouts reads as the card stretching. + "bg-surface-primary flex-shrink-0 overflow-hidden border-l", + // On coarse pointers the resize handle is a wider strip that draws the divider line + // on its right edge, where the tab strip's bottom border meets it. + "[@media(pointer:coarse)]:has-[[data-sidebar-resize-handle]]:border-l-0", + ] )} style={collapsed ? undefined : { width, maxWidth: "100%" }} role={role} @@ -288,7 +293,7 @@ interface RightSidebarProps { /** Custom width in pixels (persisted per-tab, provided by AIView) */ width?: number; /** Drag start handler for resize */ - onStartResize?: (e: React.MouseEvent) => void; + onStartResize?: (e: React.PointerEvent) => void; /** Whether currently resizing */ isResizing?: boolean; /** Callback when user adds a review note from Code Review tab */ @@ -382,7 +387,7 @@ interface RightSidebarTabsetNodeProps { setLayout: (updater: (prev: RightSidebarLayoutState) => RightSidebarLayoutState) => void; /** Handler to pop out a terminal tab to a separate window */ onPopOutTerminal: (tab: TabType) => void; - /** Handler for the strip's "+" button: open (or show) this tabset's New tab */ + /** The strip's New tab row (All tabs menu): open (or show) this tabset's New tab */ onAddNewTab: (tabsetId: string) => void; /** Set only for the tabset whose strip shows the sidebar's collapse button */ onCollapseSidebar?: () => void; @@ -398,7 +403,7 @@ interface RightSidebarTabsetNodeProps { isOnlyTabset: boolean; /** Tools the New tab launcher offers, in order (already filtered for availability) */ launcherTools: BaseTabType[]; - /** Tabset whose New tab launcher should take focus once shown (after "+" or the shortcut) */ + /** Tabset whose New tab launcher should take focus once shown (after its row or the shortcut) */ launcherAutoFocusTabsetId: string | null; onLauncherAutoFocusConsumed: () => void; /** Handler to close a terminal tab */ @@ -589,6 +594,8 @@ const RightSidebarTabsetNode: React.FC = (props) => // with index + close/pop-out actions) — see `tabRegistry.tsx` for why // terminals stay outside the static registry. let label: React.ReactNode; + // The All tabs menu shows `label` unless it holds buttons (a menu row cannot nest them). + let menuLabel: React.ReactNode | undefined; if (isNewTab(tab)) { label = ; } else if (isBaseTabId(tab)) { @@ -596,14 +603,16 @@ const RightSidebarTabsetNode: React.FC = (props) => label =