From 6286d198a33f5fad4472b6044f18a2854b19bb92 Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Tue, 30 Jun 2026 22:25:19 -0700 Subject: [PATCH 1/2] Tighten thread actions menu dividers --- .../thread/ThreadActionsMenu.test.tsx | 37 +++++++++++++++++-- .../components/thread/ThreadActionsMenu.tsx | 13 +++++-- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/apps/app/src/components/thread/ThreadActionsMenu.test.tsx b/apps/app/src/components/thread/ThreadActionsMenu.test.tsx index 426bb89a34d..2ebb1d345bc 100644 --- a/apps/app/src/components/thread/ThreadActionsMenu.test.tsx +++ b/apps/app/src/components/thread/ThreadActionsMenu.test.tsx @@ -10,7 +10,7 @@ const mockActions = vi.hoisted(() => ({ archiveThreadAndChildren: vi.fn(), requestRename: vi.fn(), requestDelete: vi.fn(), - sendToPopout: null, + sendToPopout: null as ((thread: Thread) => void) | null, togglePin: vi.fn(), toggleRead: vi.fn(), unarchiveThread: vi.fn(), @@ -45,15 +45,23 @@ function makeThread(overrides: Partial = {}): Thread { }; } -async function renderOpenMenu(thread: Thread) { +async function renderOpenMenu( + thread: Thread, + { isCompactViewport = true }: { isCompactViewport?: boolean } = {}, +) { const onOpenChange = vi.fn(); render( - + , ); - fireEvent.click(screen.getByRole("button", { name: "Thread actions" })); + const trigger = screen.getByRole("button", { name: "Thread actions" }); + if (isCompactViewport) { + fireEvent.click(trigger); + } else { + fireEvent.pointerDown(trigger, { button: 0 }); + } await screen.findByRole("menuitem", { name: /Mark / }); expect(onOpenChange).toHaveBeenLastCalledWith(true); return onOpenChange; @@ -68,6 +76,7 @@ describe("ThreadActionsMenu", () => { afterEach(() => { cleanup(); vi.clearAllMocks(); + mockActions.sendToPopout = null; }); it.each([ @@ -119,4 +128,24 @@ describe("ThreadActionsMenu", () => { expectMenuItemIcon("Archive", "Archive"); expectMenuItemIcon("Delete", "Trash2"); }); + + it("omits dividers when rendering as a compact drawer", async () => { + await renderOpenMenu(makeThread(), { isCompactViewport: true }); + + expect(screen.queryAllByRole("separator")).toHaveLength(0); + }); + + it("renders one divider when the popout action is unavailable", async () => { + await renderOpenMenu(makeThread(), { isCompactViewport: false }); + + expect(screen.getAllByRole("separator")).toHaveLength(1); + }); + + it("renders both dividers when the popout action is available", async () => { + mockActions.sendToPopout = vi.fn(); + + await renderOpenMenu(makeThread(), { isCompactViewport: false }); + + expect(screen.getAllByRole("separator")).toHaveLength(2); + }); }); diff --git a/apps/app/src/components/thread/ThreadActionsMenu.tsx b/apps/app/src/components/thread/ThreadActionsMenu.tsx index c4ec25cc494..350f8b34bcf 100644 --- a/apps/app/src/components/thread/ThreadActionsMenu.tsx +++ b/apps/app/src/components/thread/ThreadActionsMenu.tsx @@ -17,6 +17,7 @@ import { import { Icon, type IconName } from "@/components/ui/icon.js"; import { Button } from "@/components/ui/button.js"; import { COARSE_POINTER_ICON_SIZE_CLASS } from "@/components/ui/coarse-pointer-sizing.js"; +import { useIsCompactViewport } from "@/components/ui/hooks/use-compact-viewport.js"; import { cn } from "@/lib/utils"; import { isThreadRead } from "@/lib/thread-read-state"; import { useThreadActions } from "./ThreadActionsProvider"; @@ -124,9 +125,13 @@ function ThreadActionsMenuItems({ unarchiveThread, sendToPopout, } = useThreadActions(); + const isCompactViewport = useIsCompactViewport(); + const isDrawer = surface === "dropdown" && isCompactViewport; + const showSeparators = !isDrawer; const isRead = isThreadRead(thread); const isArchived = thread.archivedAt != null; const isPinned = thread.pinnedAt !== null; + const canSendToPopout = sendToPopout !== null; return ( <> @@ -149,8 +154,8 @@ function ThreadActionsMenuItems({ > {isPinned ? "Unpin" : "Pin"} - - {sendToPopout !== null ? ( + {showSeparators ? : null} + {canSendToPopout ? ( Rename - + {showSeparators && canSendToPopout ? ( + + ) : null} Date: Tue, 30 Jun 2026 22:33:40 -0700 Subject: [PATCH 2/2] Group thread lifecycle actions behind divider --- .../thread/ThreadActionsMenu.test.tsx | 22 ++++++++++++++++++- .../components/thread/ThreadActionsMenu.tsx | 8 +++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/apps/app/src/components/thread/ThreadActionsMenu.test.tsx b/apps/app/src/components/thread/ThreadActionsMenu.test.tsx index 2ebb1d345bc..a9c80d1d12b 100644 --- a/apps/app/src/components/thread/ThreadActionsMenu.test.tsx +++ b/apps/app/src/components/thread/ThreadActionsMenu.test.tsx @@ -72,6 +72,18 @@ function expectMenuItemIcon(label: string, iconName: string) { expect(menuItem.querySelector(`[data-icon="${iconName}"]`)).not.toBeNull(); } +function getMenuRoleSequence(): string[] { + return Array.from( + screen + .getByRole("menu") + .querySelectorAll('[role="menuitem"], [role="separator"]'), + ).map((element) => + element.getAttribute("role") === "separator" + ? "separator" + : (element.textContent ?? "").trim(), + ); +} + describe("ThreadActionsMenu", () => { afterEach(() => { cleanup(); @@ -135,10 +147,18 @@ describe("ThreadActionsMenu", () => { expect(screen.queryAllByRole("separator")).toHaveLength(0); }); - it("renders one divider when the popout action is unavailable", async () => { + it("renders one divider before lifecycle actions when the popout action is unavailable", async () => { await renderOpenMenu(makeThread(), { isCompactViewport: false }); expect(screen.getAllByRole("separator")).toHaveLength(1); + expect(getMenuRoleSequence()).toEqual([ + "Mark read", + "Pin", + "Rename", + "separator", + "Archive", + "Delete", + ]); }); it("renders both dividers when the popout action is available", async () => { diff --git a/apps/app/src/components/thread/ThreadActionsMenu.tsx b/apps/app/src/components/thread/ThreadActionsMenu.tsx index 350f8b34bcf..234e4528b41 100644 --- a/apps/app/src/components/thread/ThreadActionsMenu.tsx +++ b/apps/app/src/components/thread/ThreadActionsMenu.tsx @@ -154,7 +154,9 @@ function ThreadActionsMenuItems({ > {isPinned ? "Unpin" : "Pin"} - {showSeparators ? : null} + {showSeparators && canSendToPopout ? ( + + ) : null} {canSendToPopout ? ( Rename - {showSeparators && canSendToPopout ? ( - - ) : null} + {showSeparators ? : null}