mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 14:40:52 +00:00
fix(nav): open the records inside the account menu on a phone
The record switcher opened as a side sub-menu next to the account menu. On a phone the account menu sits against the right edge and takes most of the width, so the 240 px sub-menu had room on neither side and ran off the screen with the names cut off. Below the sm breakpoint the trigger now unfolds the records in place inside the account menu; wider screens keep the sub-menu. The trigger, list and entry slots are unchanged, so the journeys read the same at either width. Refs #1372
This commit is contained in:
1 parent
deb7db2ffa
commit
cfa834510e
3 files changed
+347
-63
No files matched your search
@@ -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
|
||||
|
||||
@@ -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(
|
||||
<div className="flex justify-end">
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger>Account</DropdownMenuTrigger>
|
||||
<DropdownMenuContent align="end" className="w-60">
|
||||
<DropdownMenuItem>Settings</DropdownMenuItem>
|
||||
<AccountSwitcherMenuItems />
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</div>,
|
||||
{
|
||||
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<HTMLElement>(
|
||||
'[data-slot="account-switcher-trigger"]',
|
||||
)!;
|
||||
}
|
||||
|
||||
function entries(): HTMLElement[] {
|
||||
return [
|
||||
...document.querySelectorAll<HTMLElement>(
|
||||
'[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<HTMLElement>(
|
||||
'[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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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 ? (
|
||||
<Loader2
|
||||
className="size-4 animate-spin motion-reduce:animate-none"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
) : (
|
||||
<Users className="size-4" aria-hidden="true" />
|
||||
);
|
||||
const rows = (
|
||||
<RecordRows
|
||||
access={access}
|
||||
activeId={activeId}
|
||||
inset={inline}
|
||||
switchAccount={switchAccount}
|
||||
/>
|
||||
);
|
||||
|
||||
if (inline) {
|
||||
return (
|
||||
<>
|
||||
<DropdownMenuSeparator />
|
||||
<DropdownMenuItem
|
||||
data-slot="account-switcher-trigger"
|
||||
data-presentation="inline"
|
||||
aria-expanded={expanded}
|
||||
aria-controls={expanded ? listId : undefined}
|
||||
className="cursor-pointer"
|
||||
// Keep the account menu open: the trigger unfolds the list in place.
|
||||
onSelect={(event) => {
|
||||
event.preventDefault();
|
||||
setExpanded((open) => !open);
|
||||
}}
|
||||
>
|
||||
{icon}
|
||||
<span className="ml-2">{label}</span>
|
||||
<ChevronDown
|
||||
aria-hidden="true"
|
||||
className={cn(
|
||||
"ml-auto size-4 transition-transform motion-reduce:transition-none",
|
||||
expanded && "rotate-180",
|
||||
)}
|
||||
/>
|
||||
</DropdownMenuItem>
|
||||
{expanded && (
|
||||
<DropdownMenuGroup
|
||||
id={listId}
|
||||
data-slot="account-switcher-menu"
|
||||
aria-label={label}
|
||||
>
|
||||
{rows}
|
||||
</DropdownMenuGroup>
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -54,72 +128,14 @@ export function AccountSwitcherMenuItems() {
|
||||
data-slot="account-switcher-trigger"
|
||||
className="min-h-11 py-2 sm:min-h-9"
|
||||
>
|
||||
{switchAccount.isPending ? (
|
||||
<Loader2
|
||||
className="size-4 animate-spin motion-reduce:animate-none"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
) : (
|
||||
<Users className="size-4" aria-hidden="true" />
|
||||
)}
|
||||
<span className="ml-2">{t("recordSharing.switcher.label")}</span>
|
||||
{icon}
|
||||
<span className="ml-2">{label}</span>
|
||||
</DropdownMenuSubTrigger>
|
||||
<DropdownMenuSubContent
|
||||
data-slot="account-switcher-menu"
|
||||
className="max-h-(--radix-dropdown-menu-content-available-height) w-60 overflow-x-hidden overflow-y-auto"
|
||||
>
|
||||
<DropdownMenuItem
|
||||
data-slot="account-switcher-own"
|
||||
className="cursor-pointer"
|
||||
disabled={switchAccount.isPending}
|
||||
onClick={() => switchAccount.mutate(null)}
|
||||
>
|
||||
<span className="truncate">
|
||||
{t("recordSharing.switcher.ownRecord")}
|
||||
</span>
|
||||
{activeId === null && (
|
||||
<span className="text-primary ml-auto text-xs">✓</span>
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
{access.accounts.map((account) => {
|
||||
const presentation = resolveRecordPresentation(account);
|
||||
|
||||
return (
|
||||
<DropdownMenuItem
|
||||
key={account.accountId}
|
||||
data-slot="account-switcher-entry"
|
||||
data-account-id={account.accountId}
|
||||
data-account-username={account.username}
|
||||
data-access-level={presentation.access}
|
||||
data-record-kind={presentation.recordKind}
|
||||
className="cursor-pointer"
|
||||
disabled={switchAccount.isPending}
|
||||
onClick={() =>
|
||||
switchAccount.switchTo(
|
||||
account.accountId,
|
||||
sharedRecordLandingHref(account.sections),
|
||||
)
|
||||
}
|
||||
>
|
||||
{/* Two lines, not three. The access level had a line of its
|
||||
own here and it never fit — a 240 px menu truncated
|
||||
"Kann verwalten" as often as it showed it, and the entry
|
||||
still said which kind of record this is on the line below.
|
||||
The level is what the banner states once you are inside,
|
||||
where it governs what the page lets you do. It stays on
|
||||
the row as `data-access-level` for the browser journeys. */}
|
||||
<div className="flex min-w-0 flex-1 flex-col">
|
||||
<span className="truncate">{accountLabel(account)}</span>
|
||||
<span className="text-muted-foreground truncate text-xs">
|
||||
{recordKindLabel(presentation.recordKind, t)}
|
||||
</span>
|
||||
</div>
|
||||
{activeId === account.accountId && (
|
||||
<Check className="text-primary ml-2 size-4 shrink-0" />
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
);
|
||||
})}
|
||||
{rows}
|
||||
</DropdownMenuSubContent>
|
||||
</DropdownMenuSub>
|
||||
</>
|
||||
@@ -134,3 +150,81 @@ function recordKindLabel(
|
||||
? t("recordSharing.lookingAfter.kindManaged")
|
||||
: t("recordSharing.lookingAfter.kindShared");
|
||||
}
|
||||
|
||||
/**
|
||||
* The rows themselves: your own record first, then every record the server
|
||||
* published. The same rows in the side sub-menu and in the inline list; the
|
||||
* inline list indents them under the trigger (`inset`).
|
||||
*/
|
||||
function RecordRows({
|
||||
access,
|
||||
activeId,
|
||||
inset,
|
||||
switchAccount,
|
||||
}: {
|
||||
access: AccountAccess;
|
||||
activeId: string | null;
|
||||
inset: boolean;
|
||||
switchAccount: ReturnType<typeof useAccountSwitch>;
|
||||
}) {
|
||||
const { t } = useTranslations();
|
||||
|
||||
return (
|
||||
<>
|
||||
<DropdownMenuItem
|
||||
data-slot="account-switcher-own"
|
||||
inset={inset}
|
||||
className="cursor-pointer"
|
||||
disabled={switchAccount.isPending}
|
||||
onClick={() => switchAccount.mutate(null)}
|
||||
>
|
||||
<span className="truncate">
|
||||
{t("recordSharing.switcher.ownRecord")}
|
||||
</span>
|
||||
{activeId === null && (
|
||||
<span className="text-primary ml-auto text-xs">✓</span>
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
{access.accounts.map((account) => {
|
||||
const presentation = resolveRecordPresentation(account);
|
||||
|
||||
return (
|
||||
<DropdownMenuItem
|
||||
key={account.accountId}
|
||||
data-slot="account-switcher-entry"
|
||||
data-account-id={account.accountId}
|
||||
data-account-username={account.username}
|
||||
data-access-level={presentation.access}
|
||||
data-record-kind={presentation.recordKind}
|
||||
inset={inset}
|
||||
className="cursor-pointer"
|
||||
disabled={switchAccount.isPending}
|
||||
onClick={() =>
|
||||
switchAccount.switchTo(
|
||||
account.accountId,
|
||||
sharedRecordLandingHref(account.sections),
|
||||
)
|
||||
}
|
||||
>
|
||||
{/* Two lines, not three. The access level had a line of its
|
||||
own here and it never fit — a 240 px menu truncated
|
||||
"Kann verwalten" as often as it showed it, and the entry
|
||||
still said which kind of record this is on the line below.
|
||||
The level is what the banner states once you are inside,
|
||||
where it governs what the page lets you do. It stays on
|
||||
the row as `data-access-level` for the browser journeys. */}
|
||||
<div className="flex min-w-0 flex-1 flex-col">
|
||||
<span className="truncate">{accountLabel(account)}</span>
|
||||
<span className="text-muted-foreground truncate text-xs">
|
||||
{recordKindLabel(presentation.recordKind, t)}
|
||||
</span>
|
||||
</div>
|
||||
{activeId === account.accountId && (
|
||||
<Check className="text-primary ml-2 size-4 shrink-0" />
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
);
|
||||
})}
|
||||
</>
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user