fix(ui): keep discussions closed until toggled (#122072)

This commit is contained in:
Peter Steinberger
2026-08-11 07:51:18 -07:00
committed by GitHub
parent 6472d22a77
commit 2183d637a2
3 changed files with 26 additions and 58 deletions
@@ -53,7 +53,7 @@ describeControlUiE2e("session discussion toggle", () => {
await server?.close();
});
it("opens and closes the sidebar from the same header action", async () => {
it("keeps an existing discussion closed until the header action opens it", async () => {
const context = await browser.newContext({
...(captureUiProof
? { recordVideo: { dir: proofDir, size: { height: 720, width: 1280 } } }
@@ -73,7 +73,10 @@ describeControlUiE2e("session discussion toggle", () => {
},
],
methodResponses: {
"session.discussion.info": { state: "available" },
"session.discussion.info": {
openUrl: "https://discussion.example/session",
state: "open",
},
"session.discussion.open": {
openUrl: "https://discussion.example/session",
state: "open",
@@ -88,6 +91,9 @@ describeControlUiE2e("session discussion toggle", () => {
const showDiscussion = page.getByRole("button", { name: "Show discussion" });
await expect.poll(() => showDiscussion.isVisible()).toBe(true);
await expect.poll(() => showDiscussion.getAttribute("aria-pressed")).toBe("false");
if (captureUiProof) {
await page.screenshot({ path: path.join(proofDir, "discussion-initial-closed.png") });
}
await showDiscussion.click();
@@ -95,7 +101,7 @@ describeControlUiE2e("session discussion toggle", () => {
const closeDiscussion = page.getByRole("button", { name: "Close Discussion" });
await expect.poll(() => hideDiscussion.getAttribute("aria-pressed")).toBe("true");
await expect.poll(() => closeDiscussion.isVisible()).toBe(true);
expect(await gateway.getRequests("session.discussion.open")).toHaveLength(1);
expect(await gateway.getRequests("session.discussion.open")).toHaveLength(0);
if (captureUiProof) {
await page.screenshot({ path: path.join(proofDir, "discussion-open.png") });
}
@@ -104,7 +110,7 @@ describeControlUiE2e("session discussion toggle", () => {
await expect.poll(() => closeDiscussion.isVisible()).toBe(false);
await expect.poll(() => showDiscussion.getAttribute("aria-pressed")).toBe("false");
expect(await gateway.getRequests("session.discussion.open")).toHaveLength(1);
expect(await gateway.getRequests("session.discussion.open")).toHaveLength(0);
if (captureUiProof) {
await page.screenshot({ path: path.join(proofDir, "discussion-closed.png") });
}
+1 -30
View File
@@ -1,8 +1,5 @@
import { html, nothing } from "lit";
import type {
SessionDiscussionInfo,
SessionDiscussionState,
} from "../../../../packages/gateway-protocol/src/index.js";
import type { SessionDiscussionInfo } from "../../../../packages/gateway-protocol/src/index.js";
import type { GatewaySessionRow } from "../../api/types.ts";
import { isDesktopPanelAvailable } from "../../app/app-shell-chrome.ts";
import { resolveControlUiAuthCandidates } from "../../app/control-ui-auth.ts";
@@ -498,7 +495,6 @@ export abstract class ChatPaneHeader extends ChatPaneSessionMenu {
return;
}
this.sessionDiscussionStates.set(sessionKey, info.state);
this.maybeAutoShowSessionDiscussion(sessionKey, info.state);
this.requestUpdate();
} catch {
// Leave unprobed: the action stays hidden and a later switch retries.
@@ -516,31 +512,6 @@ export abstract class ChatPaneHeader extends ChatPaneSessionMenu {
}
}
// An "open" probe result means this session already has a bound discussion;
// surface it immediately instead of hiding live chat behind the toggle.
// Probe resolution is the only hook needed: willUpdate deletes the target
// key's cached state on every session switch (and reconnect clears all), so
// each activation resolves a fresh probe and reaches this. Within one
// activation the cache dedupes — closing the sidebar sticks, and an
// already-open discussion column is never duplicated.
protected maybeAutoShowSessionDiscussion(
sessionKey: string,
discussionState: SessionDiscussionState,
) {
const state = this.state;
if (
discussionState !== "open" ||
!state ||
state.sessionKey.trim() !== sessionKey ||
state.sidebarLayout.columns.some((column) =>
column.panels.some((panel) => panel.slot === "discussion"),
)
) {
return;
}
this.openSessionDiscussionSlot();
}
protected buildSessionDiscussionPanel(
state: NonNullable<typeof this.state>,
sessionKey: string,
@@ -47,21 +47,18 @@ function createDiscussionPane(params: {
return { pane, state, updateSidebarLayout, request };
}
describe("chat pane session discussion auto-show", () => {
it("auto-shows the discussion slot when the probe reports an open discussion", async () => {
const { pane, state, updateSidebarLayout } = createDiscussionPane({
describe("chat pane session discussion", () => {
it("does not auto-show an open discussion", async () => {
const { pane, updateSidebarLayout } = createDiscussionPane({
info: { state: "open", embedUrl: "https://clack.example/embed/c1" },
});
await pane.probeSessionDiscussion(SESSION_KEY);
expect(updateSidebarLayout).toHaveBeenCalledTimes(1);
expect(
state.sidebarLayout.columns.flatMap((column) => column.panels.map((panel) => panel.slot)),
).toEqual(["discussion"]);
expect(updateSidebarLayout).not.toHaveBeenCalled();
});
it("keeps the reported external URL with the promoted discussion panel", async () => {
it("keeps the reported external URL with the discussion panel", async () => {
const openUrl = "https://clack.example/channels/c1";
const { pane, state } = createDiscussionPane({
info: { state: "open", embedUrl: "https://clack.example/embed/c1", openUrl },
@@ -148,12 +145,17 @@ describe("chat pane session discussion auto-show", () => {
info: { state: "open", embedUrl: "https://clack.example/embed/c1" },
detailOpen: true,
});
const container = document.createElement("div");
document.body.append(container);
await pane.probeSessionDiscussion(SESSION_KEY);
render(pane.renderSessionDiscussionAction(), container);
container.querySelector<HTMLButtonElement>(".chat-session-discussion-toggle")?.click();
expect(
state.sidebarLayout.columns.flatMap((column) => column.panels.map((panel) => panel.slot)),
).toEqual(["detail", "discussion"]);
container.remove();
});
it("opens as a collapsed tab when two columns cannot fit side by side", async () => {
@@ -162,12 +164,17 @@ describe("chat pane session discussion auto-show", () => {
detailOpen: true,
});
pane.paneWidth = 700;
const container = document.createElement("div");
document.body.append(container);
await pane.probeSessionDiscussion(SESSION_KEY);
render(pane.renderSessionDiscussionAction(), container);
container.querySelector<HTMLButtonElement>(".chat-session-discussion-toggle")?.click();
expect(
state.sidebarLayout.columns.flatMap((column) => column.panels.map((panel) => panel.slot)),
).toEqual(["detail", "discussion"]);
container.remove();
});
it("ignores a stale none callback after switching sessions", async () => {
@@ -196,20 +203,4 @@ describe("chat pane session discussion auto-show", () => {
expect(state.sidebarLayout.columns[0]?.panels[0]?.slot).toBe("discussion");
});
it("does not auto-show when the pane switched sessions before the probe resolved", async () => {
let resolveInfo!: (value: SessionDiscussionInfo) => void;
const { pane, state, updateSidebarLayout } = createDiscussionPane({
info: new Promise<SessionDiscussionInfo>((resolve) => {
resolveInfo = resolve;
}),
});
const probe = pane.probeSessionDiscussion(SESSION_KEY);
state.sessionKey = "agent:main:other";
resolveInfo({ state: "open", embedUrl: "https://clack.example/embed/c1" });
await probe;
expect(updateSidebarLayout).not.toHaveBeenCalled();
});
});