diff --git a/CHANGELOG.md b/CHANGELOG.md index 4058e701d..67e4b7628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Fixed + +- **Opening a record on a phone.** In the account menu, "Open a record" + used to open a second menu beside the first, and on a narrow screen it + ran off the edge and cut the names off. On a phone the records now + unfold inside the account menu itself; wider screens keep the side + menu. (#1372) + ## [1.47.5] — 2026-11-12 ### Fixed diff --git a/src/components/layout/__tests__/account-switcher-menu.browser.test.tsx b/src/components/layout/__tests__/account-switcher-menu.browser.test.tsx new file mode 100644 index 000000000..7b64edd10 --- /dev/null +++ b/src/components/layout/__tests__/account-switcher-menu.browser.test.tsx @@ -0,0 +1,182 @@ +/** + * The record switcher inside the account menu, at phone and desktop width + * (#1372). + * + * On a phone the account menu sits against the right edge of the top bar and + * takes most of the width. The records used to open in a side sub-menu that + * had room on neither side, so it ran off the screen and cut the names off. + * Below `sm` they now unfold inside the account menu itself; at desktop width + * the side sub-menu stays. + * + * The harness mounts the switcher the way the top bar does: a right-aligned + * trigger, `align="end"`, a `w-60` menu. + * + * Mutation check, run: forcing the side sub-menu at every width (the + * component before the fix) turns "unfolds the records inside the account + * menu" red, because the entries land outside the viewport and outside the + * account menu. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { page, userEvent } from "vitest/browser"; + +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; +import type { AccountAccessEntry } from "@/lib/sharing/account-access-view"; + +import { renderWithApp } from "../../../../tests/browser/render"; + +const switchTo = vi.fn(); +const mutate = vi.fn(); + +vi.mock("@/hooks/use-account-switch", () => ({ + useAccountSwitch: () => ({ mutate, switchTo, isPending: false }), +})); + +import { AccountSwitcherMenuItems } from "../account-switcher-menu"; + +function entry( + accountId: string, + fullName: string, + recordKind: "shared" | "managed", +): AccountAccessEntry { + return { + accountId, + username: accountId, + displayName: null, + fullName, + access: "read", + level: "read", + sections: null, + recordKind, + canWrite: false, + writableDomains: [], + manageableDomains: [], + }; +} + +const ACCOUNTS = [ + entry("acct-a", "Alexandra Konstantinopoulou-Wetherington", "shared"), + entry("acct-b", "Maximilian Oberhausen", "managed"), +]; + +async function mountMenu() { + await renderWithApp( +
+ + Account + + Settings + + + +
, + { + user: { + accountAccess: { accounts: ACCOUNTS, active: null, canSwitch: true }, + }, + }, + ); + await page.getByRole("button", { name: "Account" }).click(); + await expect + .poll(() => + document.querySelector('[data-slot="account-switcher-trigger"]'), + ) + .toBeTruthy(); +} + +function trigger(): HTMLElement { + return document.querySelector( + '[data-slot="account-switcher-trigger"]', + )!; +} + +function entries(): HTMLElement[] { + return [ + ...document.querySelectorAll( + '[data-slot="account-switcher-entry"]', + ), + ]; +} + +afterEach(() => { + switchTo.mockClear(); + mutate.mockClear(); +}); + +describe("the record switcher at phone width", () => { + it("unfolds the records inside the account menu", async () => { + await page.viewport(390, 844); + await mountMenu(); + + await userEvent.click(trigger()); + await expect.poll(() => entries().length).toBe(ACCOUNTS.length); + + const accountMenu = trigger().closest('[role="menu"]'); + for (const row of entries()) { + // In the account menu, not in a menu of its own beside it. + expect(row.closest('[role="menu"]')).toBe(accountMenu); + const box = row.getBoundingClientRect(); + expect(box.left).toBeGreaterThanOrEqual(0); + expect(box.right).toBeLessThanOrEqual(window.innerWidth); + // A finger-sized row. + expect(box.height).toBeGreaterThanOrEqual(44); + } + expect(document.querySelectorAll('[role="menu"]')).toHaveLength(1); + expect(trigger().getAttribute("aria-expanded")).toBe("true"); + }); + + it("keeps the account menu open while the list unfolds and folds", async () => { + await page.viewport(390, 844); + await mountMenu(); + + expect(entries()).toHaveLength(0); + expect(trigger().getAttribute("aria-expanded")).toBe("false"); + + await userEvent.click(trigger()); + await expect.poll(() => entries().length).toBe(ACCOUNTS.length); + + await userEvent.click(trigger()); + await expect.poll(() => entries().length).toBe(0); + // The account menu itself is still there. + expect(trigger()).toBeTruthy(); + expect(trigger().getAttribute("aria-expanded")).toBe("false"); + }); + + it("opens the chosen record from the inline list", async () => { + await page.viewport(390, 844); + await mountMenu(); + + await userEvent.click(trigger()); + await expect.poll(() => entries().length).toBe(ACCOUNTS.length); + await userEvent.click( + document.querySelector( + '[data-slot="account-switcher-entry"][data-account-id="acct-b"]', + )!, + ); + + expect(switchTo).toHaveBeenCalledTimes(1); + expect(switchTo.mock.calls[0][0]).toBe("acct-b"); + }); +}); + +describe("the record switcher at desktop width", () => { + it("keeps the side sub-menu", async () => { + await page.viewport(1280, 900); + await mountMenu(); + + expect(trigger().hasAttribute("data-presentation")).toBe(false); + await userEvent.click(trigger()); + await expect.poll(() => entries().length).toBe(ACCOUNTS.length); + + const accountMenu = trigger().closest('[role="menu"]'); + for (const row of entries()) { + const menu = row.closest('[role="menu"]'); + expect(menu).toBeTruthy(); + expect(menu).not.toBe(accountMenu); + } + }); +}); diff --git a/src/components/layout/account-switcher-menu.tsx b/src/components/layout/account-switcher-menu.tsx index 54abd94ca..98f2e98f0 100644 --- a/src/components/layout/account-switcher-menu.tsx +++ b/src/components/layout/account-switcher-menu.tsx @@ -1,8 +1,10 @@ "use client"; -import { Check, Loader2, Users } from "lucide-react"; +import { useId, useState } from "react"; +import { Check, ChevronDown, Loader2, Users } from "lucide-react"; import { + DropdownMenuGroup, DropdownMenuItem, DropdownMenuSeparator, DropdownMenuSub, @@ -11,10 +13,15 @@ import { } from "@/components/ui/dropdown-menu"; import { useAccountSwitch } from "@/hooks/use-account-switch"; import { useAuth } from "@/hooks/use-auth"; +import { useIsMobile } from "@/hooks/use-is-mobile"; import { useTranslations } from "@/lib/i18n/context"; import { resolveRecordPresentation } from "@/lib/navigation/record-presentation"; import { sharedRecordLandingHref } from "@/lib/navigation/shared-record"; -import { accountLabel } from "@/lib/sharing/account-access-view"; +import { + accountLabel, + type AccountAccess, +} from "@/lib/sharing/account-access-view"; +import { cn } from "@/lib/utils"; /** * v1.36.0 — the switcher, as a slice of the user menu. @@ -35,16 +42,83 @@ import { accountLabel } from "@/lib/sharing/account-access-view"; * "Your own record" is a first-class entry rather than only a banner action: * the way out has to be reachable from the same menu as the way in, because * that is where somebody will look for it. + * + * Below the `sm` breakpoint the records open inside the account menu rather + * than in a sub-menu beside it. The phone's account menu sits against the + * right edge and takes most of the width; a 240 px sub-menu has room on + * neither side, and Radix only shifts a side menu along its own axis, so it + * ran off the screen and cut the names off. The trigger keeps its slot and the + * list keeps `account-switcher-menu`, so a journey that taps the trigger and + * then an entry reads the same at either width. */ export function AccountSwitcherMenuItems() { const { user } = useAuth(); const { t } = useTranslations(); const switchAccount = useAccountSwitch(); + const inline = useIsMobile("sm"); + const [expanded, setExpanded] = useState(false); + const listId = useId(); const access = user?.accountAccess; if (!access?.canSwitch) return null; const activeId = access.active?.accountId ?? null; + const label = t("recordSharing.switcher.label"); + const icon = switchAccount.isPending ? ( +