Preserve diff surface when toggling right panel (pingdotgg#3083)

Author: juliusmarminge

apps/web/src/components/ChatView.browser.tsx

@@ -1546,6 +1546,18 @@ function dispatchChatNewShortcut(): void {
   );
 }
 
+function dispatchConfiguredDiffToggleShortcut(): void {
+  window.dispatchEvent(
+    new KeyboardEvent("keydown", {
+      key: "g",
+      shiftKey: true,
+      altKey: true,
+      bubbles: true,
+      cancelable: true,
+    }),
+  );
+}
+
 function releaseModShortcut(key?: string): void {
   window.dispatchEvent(
     new KeyboardEvent("keyup", {
@@ -3383,6 +3395,76 @@ describe("ChatView timeline estimator parity (full app)", () => {
     }
   });
 
+  it("uses the configured diff toggle binding without discarding its surface", async () => {
+    const mounted = await mountChatView({
+      viewport: DEFAULT_VIEWPORT,
+      snapshot: createSnapshotForTargetUser({
+        targetMessageId: "msg-user-target-diff-hotkey" as MessageId,
+        targetText: "diff hotkey target",
+      }),
+      configureFixture: (nextFixture) => {
+        nextFixture.serverConfig = {
+          ...nextFixture.serverConfig,
+          keybindings: [
+            {
+              command: "diff.toggle",
+              shortcut: {
+                key: "g",
+                metaKey: false,
+                ctrlKey: false,
+                shiftKey: true,
+                altKey: true,
+                modKey: false,
+              },
+              whenAst: {
+                type: "not",
+                node: { type: "identifier", name: "terminalFocus" },
+              },
+            },
+          ],
+        };
+      },
+    });
+
+    try {
+      await waitForServerConfigToApply();
+      dispatchConfiguredDiffToggleShortcut();
+      await vi.waitFor(() => {
+        expect(
+          selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, THREAD_REF),
+        ).toEqual({
+          isOpen: true,
+          activeSurfaceId: "diff",
+          surfaces: [{ id: "diff", kind: "diff" }],
+        });
+      });
+
+      dispatchConfiguredDiffToggleShortcut();
+      await vi.waitFor(() => {
+        expect(
+          selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, THREAD_REF),
+        ).toEqual({
+          isOpen: false,
+          activeSurfaceId: "diff",
+          surfaces: [{ id: "diff", kind: "diff" }],
+        });
+      });
+
+      dispatchConfiguredDiffToggleShortcut();
+      await vi.waitFor(() => {
+        expect(
+          selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, THREAD_REF),
+        ).toEqual({
+          isOpen: true,
+          activeSurfaceId: "diff",
+          surfaces: [{ id: "diff", kind: "diff" }],
+        });
+      });
+    } finally {
+      await mounted.cleanup();
+    }
+  });
+
   it("focuses the composer and inserts printable text typed from the page background", async () => {
     const mounted = await mountChatView({
       viewport: DEFAULT_VIEWPORT,

apps/web/src/components/ChatView.tsx

@@ -2263,9 +2263,9 @@ export default function ChatView(props: ChatViewProps) {
   //     openness; when both fight, `selectActiveRightPanelKindWithUrl` lets
   //     diff win (URL is truth).
   //   - The two toggles below treat the panels as mutually exclusive: opening
-  //     one always tears down the other in BOTH the URL and the store. Without
-  //     this, e.g. clicking the browser button while diff is URL-pinned would
-  //     just toggle the (overridden) store value and look like a no-op.
+  //     one updates BOTH the URL and the store. Without this, e.g. clicking the
+  //     browser button while diff is URL-pinned would just toggle the
+  //     (overridden) store value and look like a no-op.
   const onTogglePreview = useCallback(() => {
     if (!activeThreadRef) return;
     if (previewPanelOpen) {
@@ -2303,11 +2303,9 @@ export default function ChatView(props: ChatViewProps) {
     const diffPanelOpen = activeRightPanelKind === "diff";
     if (!diffPanelOpen) {
       onDiffPanelOpen?.();
-      if (activeThreadRef) {
-        useRightPanelStore.getState().open(activeThreadRef, "diff");
-      }
-    } else if (activeThreadRef) {
-      useRightPanelStore.getState().closeSurface(activeThreadRef, "diff");
+    }
+    if (activeThreadRef) {
+      useRightPanelStore.getState().toggle(activeThreadRef, "diff");
     }
     void navigate({
       to: "/$environmentId/$threadId",

apps/web/src/rightPanelStore.test.ts

@@ -111,11 +111,16 @@ describe("rightPanelStore", () => {
     expect(useRightPanelStore.getState().byThreadKey).toEqual({});
   });
 
-  it("toggle opens then closes the same kind", () => {
-    useRightPanelStore.getState().toggle(refA, "preview");
-    expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("preview");
-    useRightPanelStore.getState().toggle(refA, "preview");
+  it("toggle hides the panel without discarding the active surface", () => {
+    useRightPanelStore.getState().toggle(refA, "diff");
+    expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff");
+    useRightPanelStore.getState().toggle(refA, "diff");
     expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBeNull();
+    expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toEqual({
+      isOpen: false,
+      activeSurfaceId: "diff",
+      surfaces: [{ id: "diff", kind: "diff" }],
+    });
   });
 
   it("toggle to a different kind switches active", () => {