fix(app): keep Cmd+T working after switching workspaces
The new-tab menu handler registered itself as `workspace-new-tab-menu:<serverId>:<paneId>`, leaving out the workspace. Pane ids are namespaced per workspace — every workspace's default pane is called "main" — so all workspaces on a host produced the same handler id. The deck keeps several workspaces mounted at once, the dispatcher stores one live handler per id, and useKeyboardActionHandler registers once per id and never re-registers on focus change. So the second workspace to mount evicted the first, and switching back left a mounted, focused workspace with no handler at all: Cmd+T and Cmd+Shift+A/T/C/E silently did nothing until the newer entry expired from the deck. Handler ids are now built from the surface's full identity rather than assembled at the call site, so the workspace can't be dropped again. The three workspace-wide handlers move onto the same constructor; they were already correct, but the shape belongs in one place.
This commit is contained in:
@@ -9,6 +9,8 @@ import {
|
||||
clickNewChat,
|
||||
clickNewTerminal,
|
||||
countTabsOfKind,
|
||||
waitForTabBar,
|
||||
pressNewTabShortcut,
|
||||
getTabTestIds,
|
||||
waitForTabWithTitle,
|
||||
measureTileTransition,
|
||||
@@ -27,10 +29,14 @@ import {
|
||||
seedTerminalProfiles,
|
||||
type TerminalProfile,
|
||||
} from "../support/helpers/new-workspace-launch";
|
||||
import { gotoAppShell } from "../support/helpers/app";
|
||||
import { waitForSidebarHydration } from "../support/helpers/workspace-ui";
|
||||
import { getServerId } from "../support/helpers/server-id";
|
||||
|
||||
// ─── Shared state ──────────────────────────────────────────────────────────
|
||||
|
||||
let workspace: SeededWorkspace;
|
||||
let secondWorkspaceId: string | null = null;
|
||||
|
||||
const EMPTY_PROMPT_PROFILE: TerminalProfile = {
|
||||
id: "e2e-empty-prompt",
|
||||
@@ -41,6 +47,14 @@ const EMPTY_PROMPT_PROFILE: TerminalProfile = {
|
||||
|
||||
test.beforeAll(async () => {
|
||||
workspace = await seedWorkspace({ repoPrefix: "launcher-e2e-" });
|
||||
const created = await workspace.client.createWorkspace({
|
||||
source: { kind: "directory", path: workspace.repoPath, projectId: workspace.projectId },
|
||||
title: "launcher-e2e-cmd-t-switch-target",
|
||||
});
|
||||
if (!created.workspace) {
|
||||
throw new Error(created.error ?? "Failed to create secondary workspace for cmd+t switch test");
|
||||
}
|
||||
secondWorkspaceId = created.workspace.id;
|
||||
});
|
||||
|
||||
test.afterAll(async () => {
|
||||
@@ -52,6 +66,40 @@ test.afterAll(async () => {
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
test.describe("Tab creation", () => {
|
||||
test("Cmd+T keeps opening the New tab menu after workspace switches", async ({ page }) => {
|
||||
if (!secondWorkspaceId) {
|
||||
throw new Error("Secondary workspace was not created");
|
||||
}
|
||||
|
||||
const serverId = getServerId();
|
||||
const newTabMenuItem = page.getByRole("menuitem", { name: /^Agent/ });
|
||||
|
||||
const switchWorkspaceRow = async (workspaceId: string) => {
|
||||
const row = page.getByTestId(`sidebar-workspace-row-${serverId}:${workspaceId}`).first();
|
||||
await expect(row).toBeVisible({ timeout: 30_000 });
|
||||
await row.click();
|
||||
// The shortcut must follow the route, so prove the route actually moved first.
|
||||
await expect(page).toHaveURL(new RegExp(`workspace/${workspaceId}(\\b|/|$)`), {
|
||||
timeout: 15_000,
|
||||
});
|
||||
await waitForTabBar(page);
|
||||
};
|
||||
|
||||
await gotoAppShell(page);
|
||||
await waitForSidebarHydration(page);
|
||||
|
||||
const sequence = [workspace.workspaceId, secondWorkspaceId];
|
||||
for (let i = 0; i < 8; i++) {
|
||||
await switchWorkspaceRow(sequence[i % sequence.length]);
|
||||
await pressNewTabShortcut(page);
|
||||
await expect(newTabMenuItem, `New tab menu did not open after switch ${i}`).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(newTabMenuItem).toBeHidden({ timeout: 5_000 });
|
||||
}
|
||||
});
|
||||
|
||||
test("retained inactive workspaces cannot own New tab shortcuts", async ({ page }) => {
|
||||
await gotoWorkspace(page, workspace.workspaceId);
|
||||
const workspaceUrl = page.url();
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id";
|
||||
|
||||
describe("buildWorkspaceKeyboardHandlerId", () => {
|
||||
it("separates workspaces that share a pane id", () => {
|
||||
// Every workspace's default pane is called "main", and the deck keeps several
|
||||
// workspaces mounted at once. Dropping the workspace id here left the evicted
|
||||
// workspace without a keyboard.
|
||||
const left = buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_a",
|
||||
paneId: "main",
|
||||
});
|
||||
const right = buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_b",
|
||||
paneId: "main",
|
||||
});
|
||||
|
||||
expect(left).not.toBe(right);
|
||||
});
|
||||
|
||||
it("separates panes within a workspace", () => {
|
||||
const main = buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_a",
|
||||
paneId: "main",
|
||||
});
|
||||
const explorer = buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_a",
|
||||
paneId: "explorer",
|
||||
});
|
||||
|
||||
expect(main).not.toBe(explorer);
|
||||
});
|
||||
|
||||
it("omits the pane segment for workspace-wide surfaces", () => {
|
||||
expect(
|
||||
buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-tab-actions",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_a",
|
||||
}),
|
||||
).toBe("workspace-tab-actions:srv:wks_a");
|
||||
});
|
||||
|
||||
it("treats a blank pane id as workspace-wide", () => {
|
||||
expect(
|
||||
buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: "srv",
|
||||
workspaceId: "wks_a",
|
||||
paneId: " ",
|
||||
}),
|
||||
).toBe("workspace-new-tab-menu:srv:wks_a");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* A keyboard handler id is an identity, not a label.
|
||||
*
|
||||
* The dispatcher keeps one live handler per id and useKeyboardActionHandler
|
||||
* registers once per id, never re-registering when focus changes. So when two
|
||||
* mounted surfaces produce the same id, the second one evicts the first and the
|
||||
* first is left without a keyboard for the rest of its mount — silently, with no
|
||||
* symptom until someone presses the key.
|
||||
*
|
||||
* The workspace deck keeps several workspaces mounted at once, so a workspace
|
||||
* surface is only identified by its server, its workspace, and its pane. Pane
|
||||
* ids are namespaced per workspace ("main" is every workspace's default pane),
|
||||
* which is why they cannot carry the identity on their own.
|
||||
*/
|
||||
export interface WorkspaceKeyboardSurface {
|
||||
/** Stable name for the surface, e.g. "workspace-new-tab-menu". */
|
||||
name: string;
|
||||
serverId: string;
|
||||
workspaceId: string;
|
||||
/** Omit for surfaces that own the whole workspace rather than one pane. */
|
||||
paneId?: string | null;
|
||||
}
|
||||
|
||||
export function buildWorkspaceKeyboardHandlerId({
|
||||
name,
|
||||
serverId,
|
||||
workspaceId,
|
||||
paneId,
|
||||
}: WorkspaceKeyboardSurface): string {
|
||||
const trimmedPaneId = paneId?.trim();
|
||||
const paneSegment = trimmedPaneId ? `:${trimmedPaneId}` : "";
|
||||
return `${name}:${serverId}:${workspaceId}${paneSegment}`;
|
||||
}
|
||||
@@ -104,6 +104,7 @@ import { useWorkspaceDirectory } from "@/stores/session-store-hooks";
|
||||
import { useCheckoutStatusQuery } from "@/git/use-status-query";
|
||||
import { TrailingActionScrim } from "@/components/ui/trailing-action-scrim";
|
||||
import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler";
|
||||
import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id";
|
||||
import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher";
|
||||
|
||||
const DROPDOWN_WIDTH = 220;
|
||||
@@ -1489,7 +1490,12 @@ function ResolvedWorkspaceDesktopTabsRow({
|
||||
);
|
||||
|
||||
useKeyboardActionHandler({
|
||||
handlerId: `workspace-new-tab-menu:${normalizedServerId}:${paneId ?? "main"}`,
|
||||
handlerId: buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-new-tab-menu",
|
||||
serverId: normalizedServerId,
|
||||
workspaceId: normalizedWorkspaceId,
|
||||
paneId,
|
||||
}),
|
||||
actions: [
|
||||
"workspace.tab.menu.open",
|
||||
"workspace.tab.target.agent",
|
||||
|
||||
@@ -67,6 +67,7 @@ import {
|
||||
type WorkspaceTabTarget,
|
||||
} from "@/workspace-tabs/model";
|
||||
import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler";
|
||||
import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id";
|
||||
import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher";
|
||||
import { useCreateFlowStore } from "@/stores/create-flow-store";
|
||||
import {
|
||||
@@ -3042,7 +3043,11 @@ function WorkspaceScreenContent({
|
||||
);
|
||||
|
||||
useKeyboardActionHandler({
|
||||
handlerId: `workspace-tab-actions:${normalizedServerId}:${normalizedWorkspaceId}`,
|
||||
handlerId: buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-tab-actions",
|
||||
serverId: normalizedServerId,
|
||||
workspaceId: normalizedWorkspaceId,
|
||||
}),
|
||||
actions: [
|
||||
"workspace.agent.new",
|
||||
"workspace.tab.close-current",
|
||||
@@ -3058,7 +3063,11 @@ function WorkspaceScreenContent({
|
||||
});
|
||||
|
||||
useKeyboardActionHandler({
|
||||
handlerId: `workspace-pane-actions:${normalizedServerId}:${normalizedWorkspaceId}`,
|
||||
handlerId: buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-pane-actions",
|
||||
serverId: normalizedServerId,
|
||||
workspaceId: normalizedWorkspaceId,
|
||||
}),
|
||||
actions: [
|
||||
"workspace.pane.split.right",
|
||||
"workspace.pane.split.down",
|
||||
@@ -3080,7 +3089,11 @@ function WorkspaceScreenContent({
|
||||
});
|
||||
|
||||
useKeyboardActionHandler({
|
||||
handlerId: `workspace-sidebar-actions:${normalizedServerId}:${normalizedWorkspaceId}`,
|
||||
handlerId: buildWorkspaceKeyboardHandlerId({
|
||||
name: "workspace-sidebar-actions",
|
||||
serverId: normalizedServerId,
|
||||
workspaceId: normalizedWorkspaceId,
|
||||
}),
|
||||
actions: ["sidebar.toggle.right"] as const,
|
||||
enabled: Boolean(isRouteFocused && normalizedServerId && normalizedWorkspaceId),
|
||||
priority: 100,
|
||||
|
||||
Reference in New Issue
Block a user