From 6a0a4989809529ba2f371e842cb4ed3c76f89cb9 Mon Sep 17 00:00:00 2001 From: subheeksh5599 Date: Thu, 3 Sep 2026 21:18:43 +0530 Subject: [PATCH 1/8] feat(analytics): make monitoring reachable and readable on mobile Two gaps from #2295 (accepted, read-only monitoring scope): 1. Navigation: on a phone the entire sidebar (the only nav to /analytics, /activity, /earnings, /settings) returned null, so the monitoring surfaces were unreachable. Add a MobileNavSheet (hamburger + left Sheet) mounted in the persistent toolbar, visible only below the lg breakpoint, mirroring the sidebar's destinations with the same auth/owner gating (useSession + useActiveMember + openAuthPrompt). Reuses the existing useIsMobile + shadcn Sheet pattern per the issue. 2. Analytics runs table: the 700px-min, 7-column table forced sideways panning on a phone. Hide the secondary columns (Source, Duration, Network, Gas) below md so the essential status/time/name fit without panning; the full detail stays in the expandable per-run rows. Desktop is unchanged (hidden md:table-cell). --- components/analytics/runs-table.tsx | 42 ++++-- components/navigation/mobile-nav-sheet.tsx | 166 +++++++++++++++++++++ components/workflow/workflow-toolbar.tsx | 4 +- 3 files changed, 196 insertions(+), 16 deletions(-) create mode 100644 components/navigation/mobile-nav-sheet.tsx diff --git a/components/analytics/runs-table.tsx b/components/analytics/runs-table.tsx index d0eafd8b1..357bab5ca 100644 --- a/components/analytics/runs-table.tsx +++ b/components/analytics/runs-table.tsx @@ -383,13 +383,16 @@ function StepLogRow({ step }: StepLogRowProps): ReactNode { {step.error ? : null} - + {/* On a phone these three columns would push the row past the viewport. + The step's name/status/error (the monitoring essentials) stay; its + duration/network/gas return on desktop. */} + {formatDuration(step.durationMs)} - + {step.network ? chains.name(step.network) : NO_VALUE} - + {formatGasNativeExact(step.gasCostWei, step.network, chains)} {step.sponsored ? ( @@ -424,13 +427,13 @@ function ExpandedStepRows({
- +
- +
- +
@@ -561,19 +564,19 @@ function ExpandableRunRow({ run }: ExpandableRunRowProps): ReactNode { - + - + {formatDuration(run.durationMs)} {formatNetworks(run.networks, chains)} - + {runGasDisplay(run, chains)} @@ -671,16 +674,25 @@ function RunsTableContent({ return (
- +
- - - - + {/* Secondary columns stay on desktop; on a phone they are what + force the 700px pan. They remain reachable in the expanded + per-run rows, which carry the full detail. */} + + + + diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx new file mode 100644 index 000000000..ecf3a1dfb --- /dev/null +++ b/components/navigation/mobile-nav-sheet.tsx @@ -0,0 +1,166 @@ +"use client"; + +import { + Activity, + BarChart3, + Clock, + DollarSign, + Globe, + Menu, + Settings, + Workflow as WorkflowIcon, +} from "lucide-react"; +import { usePathname, useRouter } from "next/navigation"; +import { useState } from "react"; +import { useAuthPrompt } from "@/components/auth/provider"; +import { + Sheet, + SheetContent, + SheetDescription, + SheetHeader, + SheetTitle, + SheetTrigger, +} from "@/components/ui/sheet"; +import { useIsMobile } from "@/hooks/use-mobile"; +import { useSession } from "@/lib/auth-client"; +import { useActiveMember } from "@/lib/hooks/use-organization"; +import { isAnonymousUser } from "@/lib/is-anonymous"; +import { cn } from "@/lib/utils"; + +type MobileNavItem = { + id: string; + icon: typeof Globe; + label: string; + href: string; + requireAuth: boolean; + ownerOnly?: boolean; + adminOnly?: boolean; +}; + +// The read-only monitoring + account destinations. "Workflows" and +// "Address Book" are flyout/overlay actions in the desktop sidebar (they +// open panels, not pages) and have no equivalent as a bare link, so they +// are intentionally not here — the workflow you are monitoring is reachable +// via its run history and the list via /workflows. +const MOBILE_NAV_ITEMS: MobileNavItem[] = [ + { id: "hub", icon: Globe, label: "Hub", href: "/hub", requireAuth: false }, + { + id: "workflows", + icon: WorkflowIcon, + label: "Workflows", + href: "/workflows", + requireAuth: false, + }, + { + id: "analytics", + icon: BarChart3, + label: "Analytics", + href: "/analytics", + requireAuth: true, + }, + { + id: "earnings", + icon: DollarSign, + label: "Earnings", + href: "/earnings", + requireAuth: true, + }, + { + id: "held-payments", + icon: Clock, + label: "Held Payments", + href: "/held-payments", + requireAuth: true, + ownerOnly: true, + }, + { + id: "activity", + icon: Activity, + label: "Activity", + href: "/activity", + requireAuth: false, + }, + { + id: "settings", + icon: Settings, + label: "Settings", + href: "/settings", + requireAuth: true, + }, +]; + +export function MobileNavSheet(): React.ReactNode { + const [open, setOpen] = useState(false); + const router = useRouter(); + const pathname = usePathname(); + const isMobile = useIsMobile(); + const { data: session } = useSession(); + const { openAuthPrompt } = useAuthPrompt(); + const { isAdmin, isOwner } = useActiveMember(); + + if (!isMobile) { + return null; + } + + const isActive = (href: string): boolean => + href === "/" + ? pathname === "/" + : pathname === href || pathname.startsWith(`${href}/`); + + const visible = MOBILE_NAV_ITEMS.filter( + (item) => (!item.adminOnly || isAdmin) && (!item.ownerOnly || isOwner) + ); + + const handleNavigate = (item: MobileNavItem): void => { + setOpen(false); + if (item.requireAuth && (!session?.user || isAnonymousUser(session.user))) { + openAuthPrompt({ action: `nav:${item.id}`, redirectTo: item.href }); + return; + } + router.push(item.href); + }; + + return ( + + + + + + + Navigate + + KeeperHub sections + + + + + + ); +} diff --git a/components/workflow/workflow-toolbar.tsx b/components/workflow/workflow-toolbar.tsx index d5b97146a..908228258 100644 --- a/components/workflow/workflow-toolbar.tsx +++ b/components/workflow/workflow-toolbar.tsx @@ -25,6 +25,7 @@ import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { ButtonGroup } from "@/components/ui/button-group"; import { OrgSwitcher } from "@/components/organization/org-switcher"; +import { MobileNavSheet } from "@/components/navigation/mobile-nav-sheet"; import { GoLiveOverlay } from "@/components/overlays/go-live-overlay"; import { ListingOverlay } from "@/components/overlays/listing-overlay"; import { Switch } from "@/components/ui/switch"; @@ -1991,8 +1992,9 @@ export const WorkflowToolbar = ({ return (
- {/* Left side: Logo + Menu + Org Switcher */} + {/* Left side: Mobile nav + Logo + Menu + Org Switcher */}
+ {(() => { const CustomLogo = getCustomLogo(); return CustomLogo ? ( From b5999ff4c44a26d5d068bd3e2e936a1f3a63ea7f Mon Sep 17 00:00:00 2001 From: subheeksh5599 Date: Thu, 3 Sep 2026 21:54:02 +0530 Subject: [PATCH 2/8] test(nav): add unit tests for the mobile nav decision logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the MobileNavSheet's pure logic (visible items by access level, active-route detection, tap action) into mobile-nav-items.ts so it is testable without pulling the React/Sheet/Sentry tree into jsdom — the repo's established pattern (settings-nav-search, use-persisted-nav-state test pure helpers, not rendered components). 12 tests cover: member/admin/owner visibility (owner-only Held Payments hidden for non-owners), no flyout/overlay-only entries leaking into the mobile set, active-route matching incl. subroutes and the root edge, and auth-prompt-vs-route decisions for signed-in / signed-out / anonymous users on requireAuth destinations. --- components/navigation/mobile-nav-items.ts | 91 ++++++++++++++ components/navigation/mobile-nav-sheet.tsx | 93 ++++---------- tests/unit/mobile-nav-sheet.test.ts | 139 +++++++++++++++++++++ 3 files changed, 252 insertions(+), 71 deletions(-) create mode 100644 components/navigation/mobile-nav-items.ts create mode 100644 tests/unit/mobile-nav-sheet.test.ts diff --git a/components/navigation/mobile-nav-items.ts b/components/navigation/mobile-nav-items.ts new file mode 100644 index 000000000..d01cb85a2 --- /dev/null +++ b/components/navigation/mobile-nav-items.ts @@ -0,0 +1,91 @@ +import type { LucideIcon } from "lucide-react"; + +export type MobileNavItem = { + id: string; + /** Presentation-only — resolved to a Lucide icon by the component. Kept off + * the data module so tests import zero React/lucide runtime. */ + icon?: LucideIcon; + label: string; + href: string; + requireAuth: boolean; + ownerOnly?: boolean; + adminOnly?: boolean; +}; + +// The read-only monitoring + account destinations. "Workflows" and +// "Address Book" are flyout/overlay actions in the desktop sidebar (they +// open panels, not pages) and have no equivalent as a bare link, so they +// are intentionally not here — the workflow you are monitoring is reachable +// via its run history and the list via /workflows. +export const MOBILE_NAV_ITEMS: MobileNavItem[] = [ + { id: "hub", label: "Hub", href: "/hub", requireAuth: false }, + { + id: "workflows", + label: "Workflows", + href: "/workflows", + requireAuth: false, + }, + { + id: "analytics", + label: "Analytics", + href: "/analytics", + requireAuth: true, + }, + { id: "earnings", label: "Earnings", href: "/earnings", requireAuth: true }, + { + id: "held-payments", + label: "Held Payments", + href: "/held-payments", + requireAuth: true, + ownerOnly: true, + }, + { id: "activity", label: "Activity", href: "/activity", requireAuth: false }, + { id: "settings", label: "Settings", href: "/settings", requireAuth: true }, +]; + +export type NavAccess = { + isAdmin: boolean; + isOwner: boolean; +}; + +/** The nav items a caller with this access level may see. */ +export function visibleMobileNavItems( + access: NavAccess, + items: MobileNavItem[] = MOBILE_NAV_ITEMS +): MobileNavItem[] { + return items.filter( + (item) => + (!item.adminOnly || access.isAdmin) && (!item.ownerOnly || access.isOwner) + ); +} + +/** Whether a route is the active one for a nav destination. */ +export function isMobileNavActive(href: string, pathname: string): boolean { + if (href === "/") { + return pathname === "/"; + } + return pathname === href || pathname.startsWith(`${href}/`); +} + +export type NavDecision = { kind: "route" } | { kind: "auth-prompt" }; + +export type SessionUser = { + name?: string | null; + email?: string | null; +}; + +/** + * What tapping a nav item does: signed-out/anonymous users on a requireAuth + * destination are sent to the auth prompt; everyone else routes. + * `sessionUser` is the session's user object (may be null when signed out). + */ +export function decideMobileNavAction( + item: MobileNavItem, + sessionUser: SessionUser | null | undefined, + isAnonymous: (user: SessionUser | null | undefined) => boolean +): NavDecision { + if (item.requireAuth && isAnonymous(sessionUser)) { + return { kind: "auth-prompt" }; + } + return { kind: "route" }; +} diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx index ecf3a1dfb..0a2d723ce 100644 --- a/components/navigation/mobile-nav-sheet.tsx +++ b/components/navigation/mobile-nav-sheet.tsx @@ -26,69 +26,23 @@ import { useSession } from "@/lib/auth-client"; import { useActiveMember } from "@/lib/hooks/use-organization"; import { isAnonymousUser } from "@/lib/is-anonymous"; import { cn } from "@/lib/utils"; +import { + decideMobileNavAction, + isMobileNavActive, + type MobileNavItem, + visibleMobileNavItems, +} from "./mobile-nav-items"; -type MobileNavItem = { - id: string; - icon: typeof Globe; - label: string; - href: string; - requireAuth: boolean; - ownerOnly?: boolean; - adminOnly?: boolean; +const ICONS: Record = { + hub: Globe, + workflows: WorkflowIcon, + analytics: BarChart3, + earnings: DollarSign, + "held-payments": Clock, + activity: Activity, + settings: Settings, }; -// The read-only monitoring + account destinations. "Workflows" and -// "Address Book" are flyout/overlay actions in the desktop sidebar (they -// open panels, not pages) and have no equivalent as a bare link, so they -// are intentionally not here — the workflow you are monitoring is reachable -// via its run history and the list via /workflows. -const MOBILE_NAV_ITEMS: MobileNavItem[] = [ - { id: "hub", icon: Globe, label: "Hub", href: "/hub", requireAuth: false }, - { - id: "workflows", - icon: WorkflowIcon, - label: "Workflows", - href: "/workflows", - requireAuth: false, - }, - { - id: "analytics", - icon: BarChart3, - label: "Analytics", - href: "/analytics", - requireAuth: true, - }, - { - id: "earnings", - icon: DollarSign, - label: "Earnings", - href: "/earnings", - requireAuth: true, - }, - { - id: "held-payments", - icon: Clock, - label: "Held Payments", - href: "/held-payments", - requireAuth: true, - ownerOnly: true, - }, - { - id: "activity", - icon: Activity, - label: "Activity", - href: "/activity", - requireAuth: false, - }, - { - id: "settings", - icon: Settings, - label: "Settings", - href: "/settings", - requireAuth: true, - }, -]; - export function MobileNavSheet(): React.ReactNode { const [open, setOpen] = useState(false); const router = useRouter(); @@ -102,18 +56,14 @@ export function MobileNavSheet(): React.ReactNode { return null; } - const isActive = (href: string): boolean => - href === "/" - ? pathname === "/" - : pathname === href || pathname.startsWith(`${href}/`); - - const visible = MOBILE_NAV_ITEMS.filter( - (item) => (!item.adminOnly || isAdmin) && (!item.ownerOnly || isOwner) - ); + const visible = visibleMobileNavItems({ isAdmin, isOwner }); const handleNavigate = (item: MobileNavItem): void => { setOpen(false); - if (item.requireAuth && (!session?.user || isAnonymousUser(session.user))) { + const user = session?.user ?? null; + if ( + decideMobileNavAction(item, user, isAnonymousUser).kind === "auth-prompt" + ) { openAuthPrompt({ action: `nav:${item.id}`, redirectTo: item.href }); return; } @@ -141,7 +91,8 @@ export function MobileNavSheet(): React.ReactNode {
{/* Secondary columns stay on desktop; on a phone they are what - force the 700px pan. They remain reachable in the expanded - per-run rows, which carry the full detail. */} + force the 700px pan, so they are hidden below md per the + mobile issue (2295). The expanded per-run rows repeat the + primary fields; the values hidden here are not re-exposed on a + phone. Desktop is unaffected. */} diff --git a/components/navigation/mobile-nav-items.ts b/components/navigation/mobile-nav-items.ts index 97b476e51..ae949e72f 100644 --- a/components/navigation/mobile-nav-items.ts +++ b/components/navigation/mobile-nav-items.ts @@ -11,6 +11,7 @@ export type MobileNavItem = { href: string; requireAuth: boolean; ownerOnly?: boolean; + adminOnly?: boolean; }; // The mobile nav derives from the same NAV_ITEMS_DATA the desktop sidebar @@ -36,13 +37,14 @@ export const MOBILE_NAV_ITEMS: MobileNavItem[] = [ href, requireAuth: item.requireAuth, ownerOnly: item.ownerOnly, + adminOnly: item.adminOnly, }, ]; }), { id: SETTINGS_NAV_ITEM_DATA.id, label: SETTINGS_NAV_ITEM_DATA.label, - href: SETTINGS_NAV_ITEM_DATA.href as string, + href: SETTINGS_NAV_ITEM_DATA.href, requireAuth: SETTINGS_NAV_ITEM_DATA.requireAuth, }, ]; @@ -57,7 +59,10 @@ export function visibleMobileNavItems( access: NavAccess, items: MobileNavItem[] = MOBILE_NAV_ITEMS ): MobileNavItem[] { - return items.filter((item) => !item.ownerOnly || access.isOwner); + return items.filter( + (item) => + (!item.adminOnly || access.isAdmin) && (!item.ownerOnly || access.isOwner) + ); } /** Whether a route is the active one for a nav destination. */ diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx index 745f11446..0ba3281d2 100644 --- a/components/navigation/mobile-nav-sheet.tsx +++ b/components/navigation/mobile-nav-sheet.tsx @@ -1,5 +1,6 @@ "use client"; +import type { LucideIcon } from "lucide-react"; import { Activity, BarChart3, @@ -31,8 +32,13 @@ import { type MobileNavItem, visibleMobileNavItems, } from "./mobile-nav-items"; +import type { NavItemId } from "./nav-items-data"; -const ICONS: Record = { +// Exhaustive over the mobile-reachable destinations (all NavItemId except the +// desktop-only address-book flyout, which never appears on mobile). Keyed by +// the shared union so a destination added to NAV_ITEMS_DATA without an icon is +// a compile error here, not a silent Globe at runtime. +const ICONS: Record, LucideIcon> = { hub: Globe, workflows: WorkflowIcon, analytics: BarChart3, @@ -96,7 +102,7 @@ export function MobileNavSheet(): React.ReactNode { ); } @@ -436,7 +436,7 @@ function ExpandedStepRows({ - ) )} @@ -674,7 +674,7 @@ function RunsTableContent({ return (
-
Name StatusSourceDurationNetworkGas + Source + + Duration + + Network + Gas Time
Name Status Source +
+
+
diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx index 0ba3281d2..6f49f399e 100644 --- a/components/navigation/mobile-nav-sheet.tsx +++ b/components/navigation/mobile-nav-sheet.tsx @@ -12,7 +12,7 @@ import { Workflow as WorkflowIcon, } from "lucide-react"; import { usePathname, useRouter } from "next/navigation"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { useAuthPrompt } from "@/components/auth/provider"; import { Sheet, @@ -32,13 +32,14 @@ import { type MobileNavItem, visibleMobileNavItems, } from "./mobile-nav-items"; -import type { NavItemId } from "./nav-items-data"; +import type { MobileReachableNavItemId } from "./nav-items-data"; -// Exhaustive over the mobile-reachable destinations (all NavItemId except the -// desktop-only address-book flyout, which never appears on mobile). Keyed by -// the shared union so a destination added to NAV_ITEMS_DATA without an icon is -// a compile error here, not a silent Globe at runtime. -const ICONS: Record, LucideIcon> = { +// Exhaustive over the mobile-reachable destinations (see +// MobileReachableNavItemId). Keyed by that id set so a destination that can +// render on mobile added to NAV_ITEMS_DATA without an icon is a compile error +// here, not a silent Globe at runtime - while a desktop-only flyout does not +// force an icon on a surface it cannot appear on. +const ICONS: Record = { hub: Globe, workflows: WorkflowIcon, analytics: BarChart3, @@ -53,11 +54,20 @@ export function MobileNavSheet(): React.ReactNode { const router = useRouter(); const pathname = usePathname(); const isMobile = useIsMobile(); + const [mounted, setMounted] = useState(false); const { data: session } = useSession(); const { openAuthPrompt } = useAuthPrompt(); const { isAdmin, isOwner, isLoading: memberLoading } = useActiveMember(); - if (!isMobile) { + useEffect(() => { + setMounted(true); + }, []); + + // useIsMobile starts undefined (false) until the matchMedia effect runs, so + // the first paint would otherwise hide the trigger and pop it in after + // hydration. Render nothing until mounted, matching the sidebar's + // hasMounted guard. + if (!(mounted && isMobile)) { return null; } diff --git a/components/navigation/nav-items-data.ts b/components/navigation/nav-items-data.ts index 662e9dcca..43af3cd37 100644 --- a/components/navigation/nav-items-data.ts +++ b/components/navigation/nav-items-data.ts @@ -39,6 +39,20 @@ export type NavItemData = { actionItem?: boolean; }; +/** Ids with a routable mobile surface - a desktop href or a mobileHref. + * Desktop-only flyouts (both null, e.g. address-book) never render on mobile + * and are excluded so adding one does not force an icon on a surface it + * cannot appear on. Keep in sync with the mobile derivation rule in + * mobile-nav-items.ts (href ?? mobileHref). */ +export type MobileReachableNavItemId = + | "hub" + | "workflows" + | "analytics" + | "earnings" + | "held-payments" + | "activity" + | "settings"; + export const NAV_ITEMS_DATA: NavItemData[] = [ { id: "hub", label: "Hub", href: "/hub", requireAuth: false }, { diff --git a/tests/unit/mobile-nav-sheet.test.ts b/tests/unit/mobile-nav-sheet.test.ts index 505c12ed7..325be0268 100644 --- a/tests/unit/mobile-nav-sheet.test.ts +++ b/tests/unit/mobile-nav-sheet.test.ts @@ -112,29 +112,37 @@ describe("visibleMobileNavItems", () => { // adminOnly must be carried onto mobile items and honoured by the // visibility filter, or an admin-only destination added to the shared // data would fail open on mobile (shown to every signed-in user) while - // the desktop sidebar hides it. - const adminOnlyIds: string[] = NAV_ITEMS_DATA.filter( - (item) => item.adminOnly - ).map((item) => item.id); + // the desktop sidebar hides it. No NAV_ITEMS_DATA entry sets adminOnly + // today, so pin a fixture item to exercise the filter branch directly. + const adminOnlyItem: MobileNavItem = { + id: "analytics", + label: "Analytics", + href: "/analytics", + requireAuth: true, + adminOnly: true, + }; + const base: MobileNavItem[] = [ + { id: "hub", label: "Hub", href: "/hub", requireAuth: false }, + ]; + + // Carried flag: the data -> mobile derivation preserves adminOnly. + const carried = MOBILE_NAV_ITEMS.find((i) => i.id === "analytics"); + expect(carried?.adminOnly).toBeUndefined(); // no real admin-only item today for (const item of MOBILE_NAV_ITEMS) { - expect(item.adminOnly === true).toBe(adminOnlyIds.includes(item.id)); - } - // And the visibility filter honours it: an admin-only item is hidden from - // members but shown to admins/owners. - const memberVisible = visibleMobileNavItems(MEMBER); - for (const id of adminOnlyIds) { - expect( - memberVisible.find((i) => i.id === id), - `admin-only destination "${id}" must not show to members` - ).toBeUndefined(); - } - const adminVisible = visibleMobileNavItems(ADMIN); - for (const id of adminOnlyIds) { - expect( - adminVisible.find((i) => i.id === id), - `admin-only destination "${id}" must show to admins` - ).toBeDefined(); + const dataAdminOnlyIds: string[] = NAV_ITEMS_DATA.filter( + (d) => d.adminOnly + ).map((d) => d.id); + expect(item.adminOnly === true).toBe(dataAdminOnlyIds.includes(item.id)); } + + // Filter honours it: hidden from members, shown to admins. + const memberVisible = visibleMobileNavItems(MEMBER, [ + adminOnlyItem, + ...base, + ]); + expect(ids(memberVisible)).not.toContain("analytics"); + const adminVisible = visibleMobileNavItems(ADMIN, [adminOnlyItem, ...base]); + expect(ids(adminVisible)).toContain("analytics"); }); }); From 338c505fc9d191681cd6299b3b870251369f6700 Mon Sep 17 00:00:00 2001 From: subheeksh5599 Date: Tue, 8 Sep 2026 10:26:07 +0530 Subject: [PATCH 8/8] fix(nav): revert hand-written mobile id union to Exclude, type MobileNavItem.id as NavItemId --- components/navigation/mobile-nav-items.ts | 8 ++++++-- components/navigation/mobile-nav-sheet.tsx | 13 ++++++------- components/navigation/nav-items-data.ts | 14 -------------- tests/unit/mobile-nav-sheet.test.ts | 6 +++--- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/components/navigation/mobile-nav-items.ts b/components/navigation/mobile-nav-items.ts index ae949e72f..0b090e347 100644 --- a/components/navigation/mobile-nav-items.ts +++ b/components/navigation/mobile-nav-items.ts @@ -1,9 +1,13 @@ import type { LucideIcon } from "lucide-react"; import { isAnonymousUser } from "@/lib/is-anonymous"; -import { NAV_ITEMS_DATA, SETTINGS_NAV_ITEM_DATA } from "./nav-items-data"; +import { + NAV_ITEMS_DATA, + type NavItemId, + SETTINGS_NAV_ITEM_DATA, +} from "./nav-items-data"; export type MobileNavItem = { - id: string; + id: NavItemId; /** Presentation-only — resolved to a Lucide icon by the component. Kept off * the data module so tests import zero React/lucide runtime. */ icon?: LucideIcon; diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx index 6f49f399e..ceae426b7 100644 --- a/components/navigation/mobile-nav-sheet.tsx +++ b/components/navigation/mobile-nav-sheet.tsx @@ -32,14 +32,13 @@ import { type MobileNavItem, visibleMobileNavItems, } from "./mobile-nav-items"; -import type { MobileReachableNavItemId } from "./nav-items-data"; +import type { NavItemId } from "./nav-items-data"; -// Exhaustive over the mobile-reachable destinations (see -// MobileReachableNavItemId). Keyed by that id set so a destination that can -// render on mobile added to NAV_ITEMS_DATA without an icon is a compile error -// here, not a silent Globe at runtime - while a desktop-only flyout does not -// force an icon on a surface it cannot appear on. -const ICONS: Record = { +// Exhaustive over the mobile-reachable destinations (all NavItemId except the +// desktop-only address-book flyout, which never appears on mobile). Keyed by +// the shared union so a destination added to NAV_ITEMS_DATA without an icon is +// a compile error here, not a silent Globe at runtime. +const ICONS: Record, LucideIcon> = { hub: Globe, workflows: WorkflowIcon, analytics: BarChart3, diff --git a/components/navigation/nav-items-data.ts b/components/navigation/nav-items-data.ts index 43af3cd37..662e9dcca 100644 --- a/components/navigation/nav-items-data.ts +++ b/components/navigation/nav-items-data.ts @@ -39,20 +39,6 @@ export type NavItemData = { actionItem?: boolean; }; -/** Ids with a routable mobile surface - a desktop href or a mobileHref. - * Desktop-only flyouts (both null, e.g. address-book) never render on mobile - * and are excluded so adding one does not force an icon on a surface it - * cannot appear on. Keep in sync with the mobile derivation rule in - * mobile-nav-items.ts (href ?? mobileHref). */ -export type MobileReachableNavItemId = - | "hub" - | "workflows" - | "analytics" - | "earnings" - | "held-payments" - | "activity" - | "settings"; - export const NAV_ITEMS_DATA: NavItemData[] = [ { id: "hub", label: "Hub", href: "/hub", requireAuth: false }, { diff --git a/tests/unit/mobile-nav-sheet.test.ts b/tests/unit/mobile-nav-sheet.test.ts index 325be0268..9054f83ba 100644 --- a/tests/unit/mobile-nav-sheet.test.ts +++ b/tests/unit/mobile-nav-sheet.test.ts @@ -128,10 +128,10 @@ describe("visibleMobileNavItems", () => { // Carried flag: the data -> mobile derivation preserves adminOnly. const carried = MOBILE_NAV_ITEMS.find((i) => i.id === "analytics"); expect(carried?.adminOnly).toBeUndefined(); // no real admin-only item today + const dataAdminOnlyIds: string[] = NAV_ITEMS_DATA.filter( + (d) => d.adminOnly + ).map((d) => d.id); for (const item of MOBILE_NAV_ITEMS) { - const dataAdminOnlyIds: string[] = NAV_ITEMS_DATA.filter( - (d) => d.adminOnly - ).map((d) => d.id); expect(item.adminOnly === true).toBe(dataAdminOnlyIds.includes(item.id)); }