mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: remember last active sidebar tab per agent session (#24631)
closes CODAGT-142 The Agents right-panel tabs (Git, Terminal, Desktop) reset to the default tab (Git) every time the user switches between agent sessions. This happens because `KeyedAgentChatPage` forces a full remount on `agentId` change, discarding the `useState` that holds the active tab. Persist the active tab per agent session in `localStorage`, following the existing `agents.draft-input.<chatID>` pattern. When the user returns to a session, the last tab they selected is restored. If the stored tab is temporarily unavailable (e.g. Desktop while the workspace is stopped), `SidebarTabView`'s existing fallback to the first available tab applies, and the stored value survives so it can be honoured once that tab reappears. Archiving a chat clears its stored tab entry so unarchiving starts fresh with the default tab.
This commit is contained in:
@@ -3,10 +3,14 @@ import { createRef } from "react";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import {
|
||||
clearPersistedSidebarTabId,
|
||||
draftInputStorageKeyPrefix,
|
||||
filterWorkspaceOptionsByOrganization,
|
||||
getPersistedDraftInputValue,
|
||||
getPersistedSidebarTabId,
|
||||
lastActiveSidebarTabStorageKeyPrefix,
|
||||
restoreOptimisticRequestSnapshot,
|
||||
savePersistedSidebarTabId,
|
||||
submitEditAndScroll,
|
||||
useConversationEditingState,
|
||||
waitForPendingChatSettingsSyncs,
|
||||
@@ -912,3 +916,103 @@ describe("submitEditAndScroll", () => {
|
||||
expect(editMessage).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("sidebar tab persistence", () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
describe("getPersistedSidebarTabId", () => {
|
||||
it("returns null when no value is stored for that chat", () => {
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the stored string when one is present", () => {
|
||||
localStorage.setItem(
|
||||
`${lastActiveSidebarTabStorageKeyPrefix}chat-1`,
|
||||
"terminal",
|
||||
);
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBe("terminal");
|
||||
});
|
||||
|
||||
it("returns null when chatID is undefined", () => {
|
||||
expect(getPersistedSidebarTabId(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when chatID is empty string", () => {
|
||||
expect(getPersistedSidebarTabId("")).toBeNull();
|
||||
});
|
||||
|
||||
it("reads from the key agents.last-active-tab.<chatID>", () => {
|
||||
const chatID = "chat-xyz";
|
||||
localStorage.setItem(`agents.last-active-tab.${chatID}`, "git");
|
||||
expect(getPersistedSidebarTabId(chatID)).toBe("git");
|
||||
});
|
||||
});
|
||||
|
||||
describe("savePersistedSidebarTabId", () => {
|
||||
it("writes tabID to agents.last-active-tab.<chatID>", () => {
|
||||
savePersistedSidebarTabId("chat-1", "desktop");
|
||||
expect(
|
||||
localStorage.getItem(`${lastActiveSidebarTabStorageKeyPrefix}chat-1`),
|
||||
).toBe("desktop");
|
||||
});
|
||||
|
||||
it("is a no-op when chatID is undefined", () => {
|
||||
savePersistedSidebarTabId(undefined, "desktop");
|
||||
expect(localStorage.length).toBe(0);
|
||||
});
|
||||
|
||||
it("is a no-op when chatID is empty string", () => {
|
||||
savePersistedSidebarTabId("", "desktop");
|
||||
expect(localStorage.length).toBe(0);
|
||||
});
|
||||
|
||||
it("can be round-tripped with getPersistedSidebarTabId", () => {
|
||||
savePersistedSidebarTabId("chat-rt", "terminal");
|
||||
expect(getPersistedSidebarTabId("chat-rt")).toBe("terminal");
|
||||
});
|
||||
|
||||
it("does not collide across different chatIDs", () => {
|
||||
savePersistedSidebarTabId("chat-a", "git");
|
||||
savePersistedSidebarTabId("chat-b", "desktop");
|
||||
expect(getPersistedSidebarTabId("chat-a")).toBe("git");
|
||||
expect(getPersistedSidebarTabId("chat-b")).toBe("desktop");
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearPersistedSidebarTabId", () => {
|
||||
it("removes agents.last-active-tab.<chatID> from storage", () => {
|
||||
savePersistedSidebarTabId("chat-1", "terminal");
|
||||
clearPersistedSidebarTabId("chat-1");
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("is a no-op when nothing is stored", () => {
|
||||
// Calling twice should not throw.
|
||||
clearPersistedSidebarTabId("chat-1");
|
||||
clearPersistedSidebarTabId("chat-1");
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("is a no-op when chatID is undefined", () => {
|
||||
savePersistedSidebarTabId("chat-1", "git");
|
||||
clearPersistedSidebarTabId(undefined);
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBe("git");
|
||||
});
|
||||
|
||||
it("is a no-op when chatID is empty string", () => {
|
||||
savePersistedSidebarTabId("chat-1", "git");
|
||||
clearPersistedSidebarTabId("");
|
||||
expect(getPersistedSidebarTabId("chat-1")).toBe("git");
|
||||
});
|
||||
|
||||
it("only affects the target chat's entry", () => {
|
||||
savePersistedSidebarTabId("chat-a", "git");
|
||||
savePersistedSidebarTabId("chat-b", "desktop");
|
||||
clearPersistedSidebarTabId("chat-a");
|
||||
expect(getPersistedSidebarTabId("chat-a")).toBeNull();
|
||||
expect(getPersistedSidebarTabId("chat-b")).toBe("desktop");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -96,6 +96,8 @@ export const RIGHT_PANEL_OPEN_KEY = "agents.right-panel-open";
|
||||
const lastModelConfigIDStorageKey = "agents.last-model-config-id";
|
||||
/** @internal Exported for testing. */
|
||||
export const draftInputStorageKeyPrefix = "agents.draft-input.";
|
||||
/** @internal localStorage key prefix for the per-chat active sidebar tab. Exported for testing. */
|
||||
export const lastActiveSidebarTabStorageKeyPrefix = "agents.last-active-tab.";
|
||||
|
||||
const clearChatPlanMode = "" satisfies ChatPlanModeOrClear;
|
||||
|
||||
@@ -109,7 +111,7 @@ type PlanModeSwitch = TypesGen.ChatPlanMode | "clear";
|
||||
export function getPersistedDraftInputValue(
|
||||
chatID: string | undefined,
|
||||
): string {
|
||||
if (typeof window === "undefined" || !chatID) {
|
||||
if (!chatID) {
|
||||
return "";
|
||||
}
|
||||
return parseStoredDraft(
|
||||
@@ -117,6 +119,49 @@ export function getPersistedDraftInputValue(
|
||||
).text;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the persisted active sidebar tab ID for a given chat. Returns
|
||||
* `null` when no value is stored or the chat ID is missing.
|
||||
*/
|
||||
export function getPersistedSidebarTabId(
|
||||
chatID: string | undefined,
|
||||
): string | null {
|
||||
if (!chatID) {
|
||||
return null;
|
||||
}
|
||||
return localStorage.getItem(
|
||||
`${lastActiveSidebarTabStorageKeyPrefix}${chatID}`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Persist the active sidebar tab ID for a given chat so it can be
|
||||
* restored across session switches. No-op when the chat ID is missing.
|
||||
*/
|
||||
export function savePersistedSidebarTabId(
|
||||
chatID: string | undefined,
|
||||
tabID: string,
|
||||
): void {
|
||||
if (!chatID) {
|
||||
return;
|
||||
}
|
||||
localStorage.setItem(
|
||||
`${lastActiveSidebarTabStorageKeyPrefix}${chatID}`,
|
||||
tabID,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove the persisted active sidebar tab ID for a given chat. Called
|
||||
* when a chat is archived so a future unarchive starts fresh.
|
||||
*/
|
||||
export function clearPersistedSidebarTabId(chatID: string | undefined): void {
|
||||
if (!chatID) {
|
||||
return;
|
||||
}
|
||||
localStorage.removeItem(`${lastActiveSidebarTabStorageKeyPrefix}${chatID}`);
|
||||
}
|
||||
|
||||
/** @internal Exported for testing. */
|
||||
export const restoreOptimisticRequestSnapshot = (
|
||||
store: Pick<
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
withProxyProvider,
|
||||
withWebSocket,
|
||||
} from "#/testHelpers/storybook";
|
||||
import { lastActiveSidebarTabStorageKeyPrefix } from "./AgentChatPage";
|
||||
import {
|
||||
AgentChatPageLoadingView,
|
||||
AgentChatPageNotFoundView,
|
||||
@@ -1011,3 +1012,154 @@ export const TerminalFocusOnTabSwitch: Story = {
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
const sidebarTabStorageKey = `${lastActiveSidebarTabStorageKeyPrefix}${AGENT_ID}`;
|
||||
|
||||
/**
|
||||
* When localStorage contains a persisted tab ID for this chat, the sidebar
|
||||
* should restore it on mount. Seed localStorage with "terminal" and verify
|
||||
* that the Terminal tab is selected instead of the default Git tab.
|
||||
*/
|
||||
export const RestoresPersistedSidebarTab: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.setItem(sidebarTabStorageKey, "terminal");
|
||||
return () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
};
|
||||
},
|
||||
render: () => (
|
||||
<StoryAgentChatPageView
|
||||
showSidebarPanel
|
||||
workspace={MockWorkspace}
|
||||
workspaceAgent={MockWorkspaceAgent}
|
||||
sshCommand="ssh coder.workspace"
|
||||
/>
|
||||
),
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
await waitFor(() => {
|
||||
const terminalTab = canvas.getByRole("tab", { name: "Terminal" });
|
||||
expect(terminalTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
const gitTab = canvas.getByRole("tab", { name: "Git" });
|
||||
expect(gitTab).toHaveAttribute("aria-selected", "false");
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Clicking a sidebar tab persists the selection to localStorage so that
|
||||
* it is restored across session switches.
|
||||
*/
|
||||
export const PersistsSidebarTabClick: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
return () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
};
|
||||
},
|
||||
render: () => (
|
||||
<StoryAgentChatPageView
|
||||
showSidebarPanel
|
||||
workspace={MockWorkspace}
|
||||
workspaceAgent={MockWorkspaceAgent}
|
||||
sshCommand="ssh coder.workspace"
|
||||
/>
|
||||
),
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
await waitFor(() => {
|
||||
const gitTab = canvas.getByRole("tab", { name: "Git" });
|
||||
expect(gitTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
const terminalTab = canvas.getByRole("tab", { name: "Terminal" });
|
||||
await userEvent.click(terminalTab);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(terminalTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(sidebarTabStorageKey)).toBe("terminal");
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* When localStorage holds a tab ID whose tab is not currently available
|
||||
* (e.g. `"terminal"` while the workspace is stopped), the sidebar
|
||||
* should fall back to the first available tab (Git) and the stored
|
||||
* value must be preserved so it can be honoured once the tab reappears.
|
||||
*
|
||||
* This locks down the contract described in the PR: `getEffectiveTabId`
|
||||
* only reads `sidebarTabId` and never writes back. A future write-back
|
||||
* in the fallback path would silently break restore-after-recovery, so
|
||||
* this story exists to catch that regression.
|
||||
*/
|
||||
export const PreservesUnavailableSidebarTab: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.setItem(sidebarTabStorageKey, "terminal");
|
||||
return () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
};
|
||||
},
|
||||
render: () => <StoryAgentChatPageView showSidebarPanel />,
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
await waitFor(() => {
|
||||
const gitTab = canvas.getByRole("tab", { name: "Git" });
|
||||
expect(gitTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
expect(canvas.queryByRole("tab", { name: "Terminal" })).toBeNull();
|
||||
|
||||
expect(localStorage.getItem(sidebarTabStorageKey)).toBe("terminal");
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* When a chat is archived, clicking a sidebar tab must not persist the
|
||||
* selection to localStorage. The archive flow clears the entry on
|
||||
* purpose so that a subsequent unarchive starts from the default tab;
|
||||
* persisting here would silently recreate the entry for any tab the
|
||||
* user clicks while viewing the read-only archived view.
|
||||
*
|
||||
* This locks down the fix for the codex P2 review comment.
|
||||
*/
|
||||
export const DoesNotPersistForArchivedChat: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
return () => {
|
||||
localStorage.removeItem(sidebarTabStorageKey);
|
||||
};
|
||||
},
|
||||
render: () => (
|
||||
<StoryAgentChatPageView
|
||||
showSidebarPanel
|
||||
isArchived
|
||||
isInputDisabled
|
||||
workspace={MockWorkspace}
|
||||
workspaceAgent={MockWorkspaceAgent}
|
||||
sshCommand="ssh coder.workspace"
|
||||
/>
|
||||
),
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
await waitFor(() => {
|
||||
const gitTab = canvas.getByRole("tab", { name: "Git" });
|
||||
expect(gitTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
const terminalTab = canvas.getByRole("tab", { name: "Terminal" });
|
||||
await userEvent.click(terminalTab);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(terminalTab).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(sidebarTabStorageKey)).toBeNull();
|
||||
},
|
||||
};
|
||||
|
||||
@@ -14,6 +14,10 @@ import type * as TypesGen from "#/api/typesGenerated";
|
||||
import type { ChatDiffStatus, ChatMessagePart } from "#/api/typesGenerated";
|
||||
import { cn } from "#/utils/cn";
|
||||
import { pageTitle } from "#/utils/page";
|
||||
import {
|
||||
getPersistedSidebarTabId,
|
||||
savePersistedSidebarTabId,
|
||||
} from "./AgentChatPage";
|
||||
import {
|
||||
AgentChatInput,
|
||||
type ChatMessageInputRef,
|
||||
@@ -268,9 +272,16 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({
|
||||
const effectiveScrollToBottomRef =
|
||||
scrollToBottomRef ?? internalScrollToBottomRef;
|
||||
|
||||
// State for programmatically switching the sidebar tab (e.g. when
|
||||
// the user clicks the inline desktop preview card).
|
||||
const [sidebarTabId, setSidebarTabId] = useState<string | null>(null);
|
||||
const [sidebarTabId, setSidebarTabIdState] = useState<string | null>(() =>
|
||||
getPersistedSidebarTabId(agentId),
|
||||
);
|
||||
|
||||
const setSidebarTabId = (tabId: string) => {
|
||||
setSidebarTabIdState(tabId);
|
||||
if (!isArchived) {
|
||||
savePersistedSidebarTabId(agentId, tabId);
|
||||
}
|
||||
};
|
||||
|
||||
const handleOpenDesktop = () => {
|
||||
onSetShowSidebarPanel(true);
|
||||
|
||||
@@ -39,6 +39,7 @@ import { DeleteDialog } from "#/components/Dialogs/DeleteDialog/DeleteDialog";
|
||||
import { useAuthenticated } from "#/hooks/useAuthenticated";
|
||||
import { useDashboard } from "#/modules/dashboard/useDashboard";
|
||||
import { createReconnectingWebSocket } from "#/utils/reconnectingWebSocket";
|
||||
import { clearPersistedSidebarTabId } from "./AgentChatPage";
|
||||
import { AgentsPageView } from "./AgentsPageView";
|
||||
import { emptyInputStorageKey } from "./components/AgentCreateForm";
|
||||
import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings";
|
||||
@@ -164,6 +165,7 @@ const AgentsPage: FC = () => {
|
||||
...archiveChatBase,
|
||||
onSuccess: (_data, chatId) => {
|
||||
clearChatErrorReason(chatId);
|
||||
clearPersistedSidebarTabId(chatId);
|
||||
},
|
||||
onError: (error, chatId, context) => {
|
||||
archiveChatBase.onError(error, chatId, context);
|
||||
@@ -186,6 +188,7 @@ const AgentsPage: FC = () => {
|
||||
),
|
||||
onSuccess: async ({ chatId }) => {
|
||||
clearChatErrorReason(chatId);
|
||||
clearPersistedSidebarTabId(chatId);
|
||||
await invalidateChatListQueries(queryClient);
|
||||
await queryClient.invalidateQueries({
|
||||
queryKey: chatKey(chatId),
|
||||
|
||||
Reference in New Issue
Block a user