fix(webapp): mode toggle escape, aria, external collapse

Author: kathiekiwi

apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.dom.test.ts

@@ -0,0 +1,91 @@
+// @vitest-environment jsdom
+import { createElement } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { act } from "react-dom/test-utils";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { OperatingSystemContextProvider } from "~/components/primitives/OperatingSystemProvider";
+import { ShortcutsProvider } from "~/components/primitives/ShortcutsProvider";
+import { ModeToggle } from "./DashboardAgentHeader";
+
+function withProviders(children: React.ReactNode) {
+  return createElement(
+    OperatingSystemContextProvider,
+    { platform: "mac" },
+    createElement(ShortcutsProvider, null, children)
+  );
+}
+
+let container: HTMLDivElement | undefined;
+let root: Root | undefined;
+
+afterEach(() => {
+  if (root) {
+    act(() => root!.unmount());
+  }
+  container?.remove();
+  container = undefined;
+  root = undefined;
+});
+
+function renderToggle(mode: "floating" | "rightPanel" | "fullscreen", onModeChange: () => void) {
+  container = document.createElement("div");
+  document.body.appendChild(container);
+  root = createRoot(container);
+  act(() => {
+    root!.render(withProviders(createElement(ModeToggle, { mode, onModeChange })));
+  });
+  return container;
+}
+
+// Row-reverse layout keeps the trigger as the first button in DOM order.
+function getTrigger(el: HTMLElement) {
+  return el.querySelectorAll("button")[0] as HTMLButtonElement;
+}
+
+function expandToggle(el: HTMLElement) {
+  const trigger = getTrigger(el);
+  act(() => {
+    trigger.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+  });
+}
+
+describe("ModeToggle", () => {
+  it("collapses on Escape without changing mode, and marks the event handled", () => {
+    const onModeChange = vi.fn();
+    const el = renderToggle("floating", onModeChange);
+    expandToggle(el);
+    expect(getTrigger(el).getAttribute("aria-expanded")).toBe("true");
+
+    // cancelable: true, like a real native keydown; otherwise preventDefault() is a no-op.
+    const escapeEvent = new KeyboardEvent("keydown", {
+      key: "Escape",
+      bubbles: true,
+      cancelable: true,
+    });
+    act(() => {
+      document.dispatchEvent(escapeEvent);
+    });
+
+    expect(escapeEvent.defaultPrevented).toBe(true);
+    expect(getTrigger(el).getAttribute("aria-expanded")).toBe("false");
+    expect(onModeChange).not.toHaveBeenCalled();
+  });
+
+  it("collapses when mode changes externally", () => {
+    container = document.createElement("div");
+    document.body.appendChild(container);
+    root = createRoot(container);
+    const onModeChange = vi.fn();
+    act(() => {
+      root!.render(withProviders(createElement(ModeToggle, { mode: "floating", onModeChange })));
+    });
+    expandToggle(container);
+    expect(getTrigger(container).getAttribute("aria-expanded")).toBe("true");
+
+    act(() => {
+      root!.render(withProviders(createElement(ModeToggle, { mode: "fullscreen", onModeChange })));
+    });
+
+    expect(getTrigger(container).getAttribute("aria-expanded")).toBe("false");
+  });
+});

apps/webapp/app/components/dashboard-agent/DashboardAgentHeader.tsx

@@ -31,7 +31,7 @@ export const NEW_CHAT_SHORTCUT: Shortcut = {
   enabledOnInputElements: true,
 };
 
-function ModeToggle({
+export function ModeToggle({
   mode,
   onModeChange,
 }: {
@@ -40,9 +40,23 @@ function ModeToggle({
 }) {
   const [isExpanded, setExpanded] = useState(false);
   const containerRef = useRef<HTMLDivElement>(null);
+  const triggerRef = useRef<HTMLButtonElement>(null);
   const currentOption = MODE_OPTIONS.find((option) => option.mode === mode) ?? MODE_OPTIONS[0];
   const otherOptions = MODE_OPTIONS.filter((option) => option.mode !== mode);
 
+  useEffect(() => {
+    // oxlint-disable-next-line react/set-state-in-effect -- collapses when `mode` changes externally (e.g. route-driven).
+    setExpanded(false);
+  }, [mode]);
+
+  // Button doesn't forward arbitrary aria props, so set them directly on the node.
+  useEffect(() => {
+    const el = triggerRef.current;
+    if (!el) return;
+    el.setAttribute("aria-haspopup", "true");
+    el.setAttribute("aria-expanded", String(isExpanded));
+  }, [isExpanded]);
+
   useEffect(() => {
     if (!isExpanded) return;
 
@@ -52,22 +66,34 @@ function ModeToggle({
       }
     }
 
+    // Capture phase + preventDefault: DashboardAgentPanel's own Escape handler
+    // (bubble phase) checks defaultPrevented before closing the whole panel.
     function handleKeyDown(event: KeyboardEvent) {
       if (event.key === "Escape") {
+        event.preventDefault();
         setExpanded(false);
       }
     }
 
     document.addEventListener("pointerdown", handlePointerDown);
-    document.addEventListener("keydown", handleKeyDown);
+    document.addEventListener("keydown", handleKeyDown, true);
     return () => {
       document.removeEventListener("pointerdown", handlePointerDown);
-      document.removeEventListener("keydown", handleKeyDown);
+      document.removeEventListener("keydown", handleKeyDown, true);
     };
   }, [isExpanded]);
 
   return (
-    <div ref={containerRef} className="flex items-center">
+    <div ref={containerRef} className="flex flex-row-reverse items-center">
+      <Button
+        ref={triggerRef}
+        variant="minimal/small"
+        className="aspect-square h-6 p-1"
+        aria-label="Change chat display mode"
+        tooltip={currentOption.label}
+        onClick={() => setExpanded((open) => !open)}
+        LeadingIcon={<currentOption.Icon className="size-4 text-text-dimmed" />}
+      />
       <AnimatePresence initial={false}>
         {isExpanded &&
           otherOptions.map(({ mode: option, label, Icon }) => (
@@ -93,14 +119,6 @@ function ModeToggle({
             </motion.div>
           ))}
       </AnimatePresence>
-      <Button
-        variant="minimal/small"
-        className="aspect-square h-6 p-1"
-        aria-label="Change chat display mode"
-        tooltip={currentOption.label}
-        onClick={() => setExpanded((open) => !open)}
-        LeadingIcon={<currentOption.Icon className="size-4 text-text-dimmed" />}
-      />
     </div>
   );
 }