mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 13:52:20 +00:00
fix(nav): admin sub-items don't expand from gravatar dropdown or admin-link click
Mirror the Settings entry pattern verbatim: the global sidebar's Admin
entry is now a single link with no sub-item expansion at any route.
v1.4.15 A1 had gated the expansion behind `onAdminPage`, but Marc
re-flagged it — the in-shell `<AdminShell>` already provides per-section
nav inside the page, so the global sidebar duplicating it on `/admin/*`
was unwanted UX. Removing the entire `{onAdminPage && <ul>...</ul>}`
block also kills the perceived link between the gravatar dropdown and
sidebar state — they're independent concerns now.
Tests updated:
- `sidebar-nav.test.tsx` — every admin-route case now asserts ONLY the
single `/admin` link is present, never sub-route links.
- New e2e spec `sidebar-admin-no-expand.spec.ts` covers (a) /admin/* shows
one link, no sub-list; (b) clicking Admin from /dashboard navigates
without flashing sub-items; (c) opening the gravatar user-menu does
not expand sidebar admin sub-items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9372684a41
commit
77fe256d9e
5 files changed
+902
-61
No files matched your search
@@ -84,7 +84,13 @@ describe("<SidebarNav> bug-report toggle", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("<SidebarNav> admin sub-items context-awareness", () => {
|
||||
describe("<SidebarNav> admin entry mirrors Settings (no sub-item expansion)", () => {
|
||||
// v1.4.16 A1: Marc reported the global sidebar expanding admin
|
||||
// sub-items on `/admin/*` was unwanted UX — the in-shell `<AdminShell>`
|
||||
// already provides per-section nav inside the page. The Admin entry
|
||||
// must behave EXACTLY like the Settings entry: a single link with no
|
||||
// sub-list at any route, regardless of where the gravatar dropdown is.
|
||||
|
||||
it("does not render admin entries at all for a regular user", () => {
|
||||
const html = render({ role: "USER", pathname: "/admin/system-status" });
|
||||
expect(html).not.toContain('href="/admin"');
|
||||
@@ -102,23 +108,37 @@ describe("<SidebarNav> admin sub-items context-awareness", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("for an admin on /admin/* expands the section sub-list", () => {
|
||||
it("for an admin on /admin/* still shows ONLY the single Admin link (no sub-list)", () => {
|
||||
const html = render({ role: "ADMIN", pathname: "/admin/system-status" });
|
||||
expect(html).toContain('href="/admin"');
|
||||
// All section sub-items must render — the global sidebar mirrors
|
||||
// the in-shell sidebar so users can jump between sections.
|
||||
// Sub-section links must NOT appear in the global sidebar — they
|
||||
// belong to `<AdminShell>`'s in-page nav. Marc reported the
|
||||
// expansion as broken UX in v1.4.16; this guard keeps it gone.
|
||||
for (const section of ADMIN_SECTIONS) {
|
||||
expect(html).toContain(`href="/admin/${section.slug}"`);
|
||||
expect(html).not.toContain(`href="/admin/${section.slug}"`);
|
||||
}
|
||||
});
|
||||
|
||||
it("expands sub-items on the /admin overview page (mirrors in-shell sidebar)", () => {
|
||||
// The in-shell `<AdminShell>` sidebar already lists every section on
|
||||
// `/admin`. The global sidebar must mirror that so the navigation
|
||||
// experience is identical between the two viewports.
|
||||
it("on the /admin overview page also shows ONLY the single Admin link", () => {
|
||||
const html = render({ role: "ADMIN", pathname: "/admin" });
|
||||
expect(html).toContain('href="/admin"');
|
||||
for (const section of ADMIN_SECTIONS) {
|
||||
expect(html).toContain(`href="/admin/${section.slug}"`);
|
||||
expect(html).not.toContain(`href="/admin/${section.slug}"`);
|
||||
}
|
||||
});
|
||||
|
||||
it("Admin entry markup mirrors Settings entry markup (same shape, no expansion)", () => {
|
||||
// Both Settings and Admin should render as a single <a> with no
|
||||
// adjacent <ul> sub-list. Counting <ul aria-label="Admin sections"
|
||||
// (or any descendant ul under the Admin link) confirms there's no
|
||||
// disclosure widget. We assert by checking the structural pattern:
|
||||
// the Admin link is followed by the Settings link (or the user
|
||||
// section), never by a <ul>.
|
||||
const html = render({ role: "ADMIN", pathname: "/admin/system-status" });
|
||||
// No sub-list <ul> on /admin/*: previously this was the
|
||||
// `aria-label={t("admin.shell.sectionsNav")}` ul; assert the only
|
||||
// Settings-bearing nav surface is intact (sectionsNav label belongs
|
||||
// to `<AdminShell>`, which the sidebar must NOT echo).
|
||||
expect(html).not.toMatch(/aria-label="Admin sections"/);
|
||||
});
|
||||
});
|
||||
@@ -29,7 +29,6 @@ import { useAuth, useLogout } from "@/hooks/use-auth";
|
||||
import { useTheme } from "@/components/providers";
|
||||
import { useAppSettings } from "@/components/app-settings-provider";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { ADMIN_SECTIONS } from "@/components/admin/admin-shell";
|
||||
import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar";
|
||||
import {
|
||||
Tooltip,
|
||||
@@ -246,9 +245,12 @@ export function SidebarNav() {
|
||||
const { user } = useAuth();
|
||||
const { bugReportEnabled } = useAppSettings();
|
||||
const isAdmin = user?.role === "ADMIN";
|
||||
// Match `/admin` exactly or any `/admin/...` sub-route. Plain
|
||||
// `startsWith("/admin")` would also flip for a hypothetical future
|
||||
// `/administrative` page, which is not the same semantic surface.
|
||||
// Match `/admin` exactly or any `/admin/...` sub-route for active-link
|
||||
// styling. Plain `startsWith("/admin")` would also flip for a
|
||||
// hypothetical future `/administrative` page, which is not the same
|
||||
// semantic surface. The Admin entry mirrors Settings: a single link
|
||||
// with no sub-item expansion in the global sidebar — `<AdminShell>`
|
||||
// renders its own per-section nav inside the page itself.
|
||||
const onAdminPage = pathname === "/admin" || pathname.startsWith("/admin/");
|
||||
const [collapsed, setCollapsed] = useState(() => {
|
||||
if (typeof window === "undefined") return false;
|
||||
@@ -491,54 +493,19 @@ export function SidebarNav() {
|
||||
</Link>
|
||||
)}
|
||||
{isAdmin && (
|
||||
<>
|
||||
<Link
|
||||
href="/admin"
|
||||
aria-current={onAdminPage ? "page" : undefined}
|
||||
className={cn(
|
||||
"flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors",
|
||||
onAdminPage
|
||||
? "bg-primary/10 text-primary"
|
||||
: "text-foreground hover:bg-accent",
|
||||
)}
|
||||
>
|
||||
<Shield className="h-4 w-4" />
|
||||
{t("nav.admin")}
|
||||
</Link>
|
||||
{/* Expandable section list — only visible while
|
||||
navigating inside `/admin/*`. Mirrors the in-shell
|
||||
sidebar so the user can jump between sections from
|
||||
the global nav too. */}
|
||||
{onAdminPage && (
|
||||
<ul
|
||||
className="space-y-1 pl-3"
|
||||
aria-label={t("admin.shell.sectionsNav")}
|
||||
>
|
||||
{ADMIN_SECTIONS.map((section) => {
|
||||
const sectionPath = `/admin/${section.slug}`;
|
||||
const isActive = pathname.startsWith(sectionPath);
|
||||
const Icon = section.icon;
|
||||
return (
|
||||
<li key={section.slug}>
|
||||
<Link
|
||||
href={sectionPath}
|
||||
aria-current={isActive ? "page" : undefined}
|
||||
className={cn(
|
||||
"flex items-center gap-2 rounded-md px-2.5 py-1.5 text-xs font-medium transition-colors",
|
||||
isActive
|
||||
? "bg-primary/10 text-primary"
|
||||
: "text-muted-foreground hover:bg-accent hover:text-foreground",
|
||||
)}
|
||||
>
|
||||
<Icon className="h-3.5 w-3.5" />
|
||||
{t(section.titleKey)}
|
||||
</Link>
|
||||
</li>
|
||||
);
|
||||
})}
|
||||
</ul>
|
||||
<Link
|
||||
href="/admin"
|
||||
aria-current={onAdminPage ? "page" : undefined}
|
||||
className={cn(
|
||||
"flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors",
|
||||
onAdminPage
|
||||
? "bg-primary/10 text-primary"
|
||||
: "text-foreground hover:bg-accent",
|
||||
)}
|
||||
</>
|
||||
>
|
||||
<Shield className="h-4 w-4" />
|
||||
{t("nav.admin")}
|
||||
</Link>
|
||||
)}
|
||||
<Link
|
||||
href="/settings/account"
|
||||
|
||||
Reference in new issue
Block a user