feat: refine thread action menus

apps/desktop/src/electron/ElectronMenu.test.ts

@@ -98,7 +98,10 @@ describe("ElectronMenu", () => {
       const electronMenu = yield* ElectronMenu.ElectronMenu;
       const selectedItemId = yield* electronMenu.showContextMenu({
         window: makeWindow(2),
-        items: [{ id: "copy", label: "Copy" }],
+        items: [
+          { id: "copy", label: "Copy" },
+          { id: "delete", label: "Delete", destructive: true, separatorBefore: true },
+        ],
         position: Option.some({ x: 10.8, y: 20.2 }),
       });
 
@@ -110,6 +113,38 @@ describe("ElectronMenu", () => {
         enabled: true,
         click: buildFromTemplateMock.mock.calls[0]?.[0][0].click,
       });
+      assert.deepEqual(
+        buildFromTemplateMock.mock.calls[0]?.[0].map(
+          (item: Electron.MenuItemConstructorOptions) => item.type ?? item.label,
+        ),
+        ["Copy", "separator", "Delete"],
+      );
+    }).pipe(Effect.provide(TestLayer)),
+  );
+
+  it.effect("keeps a preceding non-destructive action in the destructive section", () =>
+    Effect.gen(function* () {
+      buildFromTemplateMock.mockImplementation(() => ({
+        popup: (options: Electron.PopupOptions) => options.callback?.(),
+      }));
+
+      const electronMenu = yield* ElectronMenu.ElectronMenu;
+      yield* electronMenu.showContextMenu({
+        window: makeWindow(),
+        items: [
+          { id: "copy", label: "Copy" },
+          { id: "archive", label: "Archive", separatorBefore: true },
+          { id: "delete", label: "Delete", destructive: true },
+        ],
+        position: Option.none(),
+      });
+
+      assert.deepEqual(
+        buildFromTemplateMock.mock.calls[0]?.[0].map(
+          (item: Electron.MenuItemConstructorOptions) => item.type ?? item.label,
+        ),
+        ["Copy", "separator", "Archive", "Delete"],
+      );
     }).pipe(Effect.provide(TestLayer)),
   );
 

apps/desktop/src/electron/ElectronMenu.ts

@@ -78,6 +78,7 @@ function normalizeContextMenuItems(source: readonly ContextMenuItem[]): ContextM
       label: sourceItem.label,
       destructive: sourceItem.destructive === true,
       disabled: sourceItem.disabled === true,
+      ...(sourceItem.separatorBefore === true ? { separatorBefore: true } : {}),
     };
 
     if (sourceItem.children) {
@@ -141,10 +142,24 @@ export const make = Effect.gen(function* () {
   ): Electron.MenuItemConstructorOptions[] => {
     const template: Electron.MenuItemConstructorOptions[] = [];
     let hasInsertedDestructiveSeparator = false;
+    let sectionStartedByExplicitSeparator = false;
+    const appendSeparator = () => {
+      if (template.length === 0 || template.at(-1)?.type === "separator") return;
+      template.push({ type: "separator" });
+    };
 
     for (const item of entries) {
-      if (item.destructive && !hasInsertedDestructiveSeparator && template.length > 0) {
-        template.push({ type: "separator" });
+      if (item.separatorBefore) {
+        appendSeparator();
+        sectionStartedByExplicitSeparator = true;
+      }
+      if (
+        item.destructive &&
+        !hasInsertedDestructiveSeparator &&
+        !sectionStartedByExplicitSeparator &&
+        template.length > 0
+      ) {
+        appendSeparator();
         hasInsertedDestructiveSeparator = true;
       }
 

apps/web/src/components/threadActionMenu.logic.test.ts

@@ -20,22 +20,28 @@ function ids(state: ThreadActionMenuState): string[] {
   return buildThreadActionMenuItems(state).map((item) => item.id);
 }
 
+function allIds(state: ThreadActionMenuState): string[] {
+  const flatten = (items: ReturnType<typeof buildThreadActionMenuItems>): string[] =>
+    items.flatMap((item) => [item.id, ...(item.children ? flatten(item.children) : [])]);
+  return flatten(buildThreadActionMenuItems(state));
+}
+
 describe("buildThreadActionMenuItems", () => {
   it("hides lifecycle items when the environment lacks the capabilities", () => {
     expect(
       ids({
         ...baseState,
         supports: { settlement: false, snooze: false, pinning: false, titleRegeneration: false },
       }),
-    ).toEqual(["rename", "mark-unread", "copy-path", "copy-thread-id", "archive", "delete"]);
+    ).toEqual(["rename", "mark-unread", "copy", "archive", "delete"]);
   });
 
   it("includes branch items only for threads with a branch", () => {
-    const withBranch = ids({ ...baseState, branch: "feat/menu" });
+    const withBranch = allIds({ ...baseState, branch: "feat/menu" });
     expect(withBranch).toContain("new-thread-on-branch");
     expect(withBranch).toContain("copy-branch");
-    expect(ids(baseState)).not.toContain("new-thread-on-branch");
-    expect(ids(baseState)).not.toContain("copy-branch");
+    expect(allIds(baseState)).not.toContain("new-thread-on-branch");
+    expect(allIds(baseState)).not.toContain("copy-branch");
   });
 
   it("flips lifecycle labels with thread state", () => {
@@ -64,11 +70,12 @@ describe("buildThreadActionMenuItems", () => {
     const items = buildThreadActionMenuItems({ ...baseState, branch: "main" });
     expect(items.at(-1)).toMatchObject({ id: "delete", destructive: true });
   });
-
   it("offers archive as a non-destructive action right before delete", () => {
     const items = buildThreadActionMenuItems(baseState);
     const archiveItem = items.at(-2);
     expect(archiveItem?.id).toBe("archive");
+    expect(archiveItem?.icon).toBe("archive");
+    expect(archiveItem?.separatorBefore).toBe(true);
     expect(archiveItem?.destructive).toBeFalsy();
     expect(items.at(-1)?.id).toBe("delete");
   });

apps/web/src/components/threadActionMenu.logic.ts

@@ -18,6 +18,7 @@ export type ThreadActionMenuId =
   | "rename"
   | "regenerate-title"
   | "mark-unread"
+  | "copy"
   | "copy-path"
   | "copy-branch"
   | "copy-thread-id"
@@ -56,14 +57,15 @@ export function buildThreadActionMenuItems(
           {
             id: "new-thread-on-branch" as const,
             label: `New thread on ${state.branch}`,
+            icon: "message-square-plus",
           },
         ]
       : []),
     ...(state.supports.pinning
       ? [
           state.isPinned
-            ? { id: "unpin" as const, label: "Unpin thread" }
-            : { id: "pin" as const, label: "Pin thread" },
+            ? { id: "unpin" as const, label: "Unpin thread", icon: "pin-off" }
+            : { id: "pin" as const, label: "Pin thread", icon: "pin" },
         ]
       : []),
     // Both lifecycle actions stay available on pinned threads: settling
@@ -72,17 +74,18 @@ export function buildThreadActionMenuItems(
     ...(state.supports.settlement
       ? [
           state.isSettled
-            ? { id: "unsettle" as const, label: "Un-settle thread" }
-            : { id: "settle" as const, label: "Settle thread" },
+            ? { id: "unsettle" as const, label: "Un-settle thread", icon: "circle-check" }
+            : { id: "settle" as const, label: "Settle thread", icon: "circle-check" },
         ]
       : []),
     ...(state.supports.snooze
       ? [
           state.isSnoozed
-            ? { id: "unsnooze" as const, label: "Wake thread" }
+            ? { id: "unsnooze" as const, label: "Wake thread", icon: "clock" }
             : {
                 id: "snooze" as const,
                 label: "Snooze",
+                icon: "clock",
                 disabled: !state.canSnoozeNow,
                 children: state.snoozePresets.map((preset) => ({
                   id: `snooze:${preset.id}` as const,
@@ -91,26 +94,48 @@ export function buildThreadActionMenuItems(
               },
         ]
       : []),
-    { id: "rename", label: "Rename thread" },
+    { id: "rename", label: "Rename thread", icon: "pencil", separatorBefore: true },
     ...(state.supports.titleRegeneration
       ? [
           {
             id: "regenerate-title" as const,
             label: state.isRegeneratingTitle ? "Regenerating…" : "Regenerate title",
+            icon: "refresh-cw",
             disabled: state.isRegeneratingTitle,
           },
         ]
       : []),
-    { id: "mark-unread", label: "Mark unread" },
-    { id: "copy-path", label: "Copy path", icon: "copy" },
-    ...(state.branch ? [{ id: "copy-branch" as const, label: "Copy branch", icon: "copy" }] : []),
-    { id: "copy-thread-id", label: "Copy thread ID", icon: "copy" },
+    { id: "mark-unread", label: "Mark unread", icon: "mail-open" },
+    {
+      id: "copy",
+      label: "Copy",
+      icon: "copy",
+      separatorBefore: true,
+      children: [
+        { id: "copy-path", label: "Path", icon: "folder" },
+        ...(state.branch
+          ? [{ id: "copy-branch" as const, label: "Branch", icon: "git-branch" }]
+          : []),
+        { id: "copy-thread-id", label: "Thread ID", icon: "hash" },
+      ],
+    },
     // Archive removes the thread from the sidebar while keeping its
     // conversation under Settings > Archived threads — distinct from Settle
     // (stays visible in the Settled shelf) and Delete (clears history for
     // good), so it sits beside Delete without borrowing its destructive
     // styling.
-    { id: "archive", label: "Archive thread", disabled: state.isRunning },
-    { id: "delete", label: "Delete", destructive: true, icon: "trash" },
+    {
+      id: "archive",
+      label: "Archive thread",
+      icon: "archive",
+      disabled: state.isRunning,
+      separatorBefore: true,
+    },
+    {
+      id: "delete",
+      label: "Delete",
+      destructive: true,
+      icon: "trash",
+    },
   ];
 }

apps/web/src/contextMenuFallback.test.ts

@@ -24,14 +24,24 @@ class FakeElement {
   parent: FakeElement | null = null;
   style: Record<string, string> & { cssText?: string } = {};
   dataset: Record<string, string> = {};
+  attributes = new Map<string, string>();
   className = "";
   disabled = false;
+  focused = false;
   type = "";
   private textValue = "";
   private readonly listeners = new Map<string, FakeListener[]>();
 
   constructor(readonly tagName: string) {}
 
+  get isConnected() {
+    let current: FakeElement | null = this;
+    while (current?.parent) {
+      current = current.parent;
+    }
+    return current?.tagName === "body";
+  }
+
   appendChild(child: FakeElement) {
     child.parent = this;
     this.children.push(child);
@@ -55,13 +65,37 @@ class FakeElement {
     this.listeners.set(type, existing);
   }
 
+  setAttribute(name: string, value: string) {
+    this.attributes.set(name, value);
+  }
+
   dispatchEvent(event: FakeDomEvent) {
     for (const listener of this.listeners.get(event.type) ?? []) {
       listener(event);
     }
     return true;
   }
 
+  focus() {
+    const fakeDocument = document as unknown as FakeDocument;
+    if (fakeDocument.activeElement === this) {
+      return;
+    }
+    fakeDocument.activeElement?.blur();
+    fakeDocument.activeElement = this;
+    this.focused = true;
+    this.dispatchEvent(new FakeDomEvent("focus"));
+  }
+
+  blur() {
+    const fakeDocument = document as unknown as FakeDocument;
+    if (fakeDocument.activeElement === this) {
+      fakeDocument.activeElement = null;
+    }
+    this.focused = false;
+    this.dispatchEvent(new FakeDomEvent("blur"));
+  }
+
   set textContent(value: string) {
     this.textValue = value;
   }
@@ -116,6 +150,7 @@ class FakeBody extends FakeElement {
 
 class FakeDocument {
   body = new FakeBody();
+  activeElement: FakeElement | null = null;
   private readonly listeners = new Map<string, FakeListener[]>();
 
   createElement(tagName: string) {
@@ -152,6 +187,7 @@ function findButton(label: string): FakeElement | undefined {
 
 beforeEach(() => {
   vi.stubGlobal("document", new FakeDocument());
+  vi.stubGlobal("HTMLElement", FakeElement);
   vi.stubGlobal("window", {
     innerWidth: 1280,
     innerHeight: 800,
@@ -183,6 +219,21 @@ afterEach(() => {
 });
 
 describe("showContextMenuFallback", () => {
+  it("renders one separator between menu sections", async () => {
+    const selectionPromise = showContextMenuFallback([
+      { id: "rename", label: "Rename" },
+      { id: "archive", label: "Archive", separatorBefore: true },
+    ]);
+    const separators = (document as unknown as FakeDocument)
+      .querySelectorAll("div")
+      .filter((element) => element.dataset.contextMenuSeparator === "true");
+
+    expect(separators).toHaveLength(1);
+    expect(separators[0]?.attributes.get("role")).toBe("separator");
+    dismissContextMenu();
+    await expect(selectionPromise).resolves.toBeNull();
+  });
+
   it("resolves a clicked flat menu item", async () => {
     const selectionPromise = showContextMenuFallback([
       { id: "rename", label: "Rename" },
@@ -235,6 +286,46 @@ describe("showContextMenuFallback", () => {
 
     await expect(selectionPromise).resolves.toBe("rename:project-b");
   });
+
+  it("opens and focuses nested submenus when the parent is activated", async () => {
+    const invoker = (document as unknown as FakeDocument).createElement("button");
+    (document as unknown as FakeDocument).body.appendChild(invoker);
+    invoker.focus();
+    const selectionPromise = showContextMenuFallback([
+      {
+        id: "copy:submenu",
+        label: "Copy",
+        children: [
+          { id: "copy:path", label: "Path" },
+          { id: "copy:branch", label: "Branch" },
+        ],
+      },
+    ]);
+
+    const parentButton = findButton("Copy");
+    expect(parentButton).toBeTruthy();
+    expect(parentButton?.attributes.get("aria-haspopup")).toBe("menu");
+    expect(parentButton?.attributes.get("aria-expanded")).toBe("false");
+    parentButton?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    expect(parentButton?.attributes.get("aria-expanded")).toBe("true");
+
+    const childButton = findButton("Path");
+    const siblingButton = findButton("Branch");
+    expect(childButton).toBeTruthy();
+    expect(siblingButton).toBeTruthy();
+    expect(childButton?.focused).toBe(true);
+    expect(childButton?.style.background).toBe("var(--accent)");
+    expect(childButton?.style.color).toBe("var(--accent-foreground)");
+    siblingButton?.dispatchEvent(new MouseEvent("mouseenter", { bubbles: true }));
+    expect(childButton?.focused).toBe(false);
+    expect(childButton?.style.background).toBe("transparent");
+    expect(siblingButton?.focused).toBe(true);
+    expect(siblingButton?.style.background).toBe("var(--accent)");
+    siblingButton?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+
+    await expect(selectionPromise).resolves.toBe("copy:branch");
+    expect(invoker.focused).toBe(true);
+  });
 });
 
 describe("dismissContextMenu", () => {