diff --git a/components/analytics/runs-table.tsx b/components/analytics/runs-table.tsx index d0eafd8b1..3361c7d03 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 ? ( @@ -399,7 +402,7 @@ function StepLogRow({ step }: StepLogRowProps): ReactNode { ) : null} - + ); } @@ -424,16 +427,16 @@ function ExpandedStepRows({
- +
- +
- +
- + ) )} @@ -561,19 +564,19 @@ function ExpandableRunRow({ run }: ExpandableRunRowProps): ReactNode { - + - + {formatDuration(run.durationMs)} {formatNetworks(run.networks, chains)} - + {runGasDisplay(run, chains)} @@ -671,16 +674,27 @@ function RunsTableContent({ return (
- +
- - - - + {/* Secondary columns stay on desktop; on a phone they are what + 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-sidebar.tsx b/components/navigation-sidebar.tsx index 296757866..acd2d41ea 100644 --- a/components/navigation-sidebar.tsx +++ b/components/navigation-sidebar.tsx @@ -49,6 +49,13 @@ import { type WorkflowTriggerType, } from "@/lib/workflow/store"; import { FLYOUT_WIDTH, FlyoutPanel, STRIP_WIDTH } from "./flyout-panel"; +import { + ACTION_ITEM_IDS, + NAV_ITEMS_DATA, + type NavItemData, + type NavItemId, + SETTINGS_NAV_ITEM_DATA, +} from "./navigation/nav-items-data"; export const COLLAPSED_WIDTH = 60; export const EXPANDED_WIDTH = 200; @@ -455,24 +462,8 @@ function SidebarHeader({ ); } -const ACTION_ITEM_IDS: ReadonlySet = new Set([ - "workflows", - "address-book", - "activity", -]); - -type NavItemDef = { - id: string; +type NavItemDef = NavItemData & { icon: typeof Plus; - label: string; - href: string | null; - requireAuth: boolean; - // Visible only to organization owners/admins (the audit feed is gated the - // same way server-side). - adminOnly?: boolean; - // Visible only to organization owners (fund-moving surfaces like held - // payments; enforced server-side too). - ownerOnly?: boolean; }; function NavItem({ @@ -542,71 +533,32 @@ function NavItem({ ); } -const NAV_ITEMS: NavItemDef[] = [ - { - id: "hub", - icon: Globe, - label: "Hub", - href: "/hub", - requireAuth: false, - }, - { - id: "workflows", - icon: WorkflowIcon, - label: "Workflows", - href: null, - 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: "address-book", - icon: Bookmark, - label: "Address Book", - href: null, - requireAuth: true, - }, - { - // Visible to everyone and routable while signed-out: the page itself shows - // an in-page sign-in for guests, a labelled sample for members, and the - // real feed for owners/admins. So this is neither requireAuth nor adminOnly. - id: "activity", - icon: Activity, - label: "Activity", - href: "/activity", - requireAuth: false, - }, -]; +// Icons are resolved here, in the surface component, from the shared nav data +// (nav-items-data.ts holds no icons so tests can import it without the +// React/lucide runtime). Keyed by NavItemId so a destination added to +// NAV_ITEMS_DATA without an icon entry is a compile error, not a render crash. +const NAV_ICONS: Record = { + hub: Globe, + workflows: WorkflowIcon, + analytics: BarChart3, + earnings: DollarSign, + "held-payments": Clock, + "address-book": Bookmark, + activity: Activity, + settings: Settings, +}; + +const NAV_ITEMS: NavItemDef[] = NAV_ITEMS_DATA.map((item) => ({ + ...item, + icon: NAV_ICONS[item.id], +})); // Settings is a destination, not a workspace view, so it sits at the foot of // the nav column rather than among Hub / Workflows / Analytics -- above the // divider that starts the external links, but pushed clear of Activity. const SETTINGS_NAV_ITEM: NavItemDef = { - id: "settings", + ...SETTINGS_NAV_ITEM_DATA, icon: Settings, - label: "Settings", - href: "/settings", - requireAuth: true, }; export function NavigationSidebar(): React.ReactNode { diff --git a/components/navigation/mobile-nav-items.ts b/components/navigation/mobile-nav-items.ts new file mode 100644 index 000000000..0b090e347 --- /dev/null +++ b/components/navigation/mobile-nav-items.ts @@ -0,0 +1,100 @@ +import type { LucideIcon } from "lucide-react"; +import { isAnonymousUser } from "@/lib/is-anonymous"; +import { + NAV_ITEMS_DATA, + type NavItemId, + SETTINGS_NAV_ITEM_DATA, +} from "./nav-items-data"; + +export type MobileNavItem = { + 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; + label: string; + href: string; + requireAuth: boolean; + ownerOnly?: boolean; + adminOnly?: boolean; +}; + +// The mobile nav derives from the same NAV_ITEMS_DATA the desktop sidebar +// renders, so a destination added to that one list either appears on both +// surfaces or fails the parity tests. Derivation rule: an item appears on +// mobile when it has a routable surface there - a desktop page (href) or a +// mobile route (mobileHref, used where desktop treats the item as a flyout +// with a null href). Items with neither (address-book) are desktop-only +// flyouts and stay off mobile. Settings is a destination on both surfaces and +// is appended from its own shared entry, matching its separate position at +// the foot of the desktop nav. +export const MOBILE_NAV_ITEMS: MobileNavItem[] = [ + ...NAV_ITEMS_DATA.flatMap((item) => { + const href = item.href ?? item.mobileHref; + if (!href) { + // Desktop-only flyout (address-book): no routable surface on mobile. + return []; + } + return [ + { + id: item.id, + label: item.label, + 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, + requireAuth: SETTINGS_NAV_ITEM_DATA.requireAuth, + }, +]; + +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 +): NavDecision { + if (item.requireAuth && isAnonymousUser(sessionUser)) { + return { kind: "auth-prompt" }; + } + return { kind: "route" }; +} diff --git a/components/navigation/mobile-nav-sheet.tsx b/components/navigation/mobile-nav-sheet.tsx new file mode 100644 index 000000000..ceae426b7 --- /dev/null +++ b/components/navigation/mobile-nav-sheet.tsx @@ -0,0 +1,136 @@ +"use client"; + +import type { LucideIcon } from "lucide-react"; +import { + Activity, + BarChart3, + Clock, + DollarSign, + Globe, + Menu, + Settings, + Workflow as WorkflowIcon, +} from "lucide-react"; +import { usePathname, useRouter } from "next/navigation"; +import { useEffect, 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 { cn } from "@/lib/utils"; +import { + decideMobileNavAction, + isMobileNavActive, + type MobileNavItem, + visibleMobileNavItems, +} from "./mobile-nav-items"; +import type { NavItemId } 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> = { + hub: Globe, + workflows: WorkflowIcon, + analytics: BarChart3, + earnings: DollarSign, + "held-payments": Clock, + activity: Activity, + settings: Settings, +}; + +export function MobileNavSheet(): React.ReactNode { + const [open, setOpen] = useState(false); + 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(); + + 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; + } + + // While the active-org membership is still loading, isOwner is false, which + // would make an owner-only destination (Held Payments) pop in after the fact. + // Render the non-owner set during the load; the owner set appears in the + // next render once the member record resolves. The only item that can appear + // later is one the user is entitled to see, so nothing flashes wrongly. + const visible = memberLoading + ? visibleMobileNavItems({ isAdmin: false, isOwner: false }) + : visibleMobileNavItems({ isAdmin, isOwner }); + + const handleNavigate = (item: MobileNavItem): void => { + setOpen(false); + const user = session?.user ?? null; + if (decideMobileNavAction(item, user).kind === "auth-prompt") { + openAuthPrompt({ action: `nav:${item.id}`, redirectTo: item.href }); + return; + } + router.push(item.href); + }; + + return ( + + + + + + + Navigate + + KeeperHub sections + + + + + + ); +} diff --git a/components/navigation/nav-items-data.ts b/components/navigation/nav-items-data.ts new file mode 100644 index 000000000..662e9dcca --- /dev/null +++ b/components/navigation/nav-items-data.ts @@ -0,0 +1,99 @@ +/** + * Single source of truth for the navigation destinations. + * + * The desktop sidebar (navigation-sidebar.tsx) and the mobile navigation sheet + * (mobile-nav-items.ts) both render from this list, so a destination added + * here appears on both surfaces or fails the parity tests. Icons are NOT part + * of this module: they are presentation, resolved per surface (the sidebar + * maps id -> Lucide icon; the mobile sheet does the same). Keeping icons out + * lets tests import this module without pulling the React/lucide runtime. + */ +export type NavItemId = + | "hub" + | "workflows" + | "analytics" + | "earnings" + | "held-payments" + | "address-book" + | "activity" + | "settings"; + +export type NavItemData = { + id: NavItemId; + label: string; + /** Desktop route. null means the desktop sidebar treats it as an action + * item (flyout/overlay) rather than a page link. */ + href: string | null; + /** Route on mobile when it differs from desktop. Workflows is a flyout on + * desktop (null href) but routes to its list page on mobile. */ + mobileHref?: string; + requireAuth: boolean; + // Visible only to organization owners/admins (the audit feed is gated the + // same way server-side). + adminOnly?: boolean; + // Visible only to organization owners (fund-moving surfaces like held + // payments; enforced server-side too). + ownerOnly?: boolean; + /** Desktop sidebar action items: workflows and address-book open flyouts, + * activity is a page but has special click handling. */ + actionItem?: boolean; +}; + +export const NAV_ITEMS_DATA: NavItemData[] = [ + { id: "hub", label: "Hub", href: "/hub", requireAuth: false }, + { + id: "workflows", + label: "Workflows", + href: null, + mobileHref: "/workflows", + requireAuth: false, + actionItem: true, + }, + { + 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: "address-book", + label: "Address Book", + href: null, + requireAuth: true, + actionItem: true, + }, + { + // Visible to everyone and routable while signed-out: the page itself shows + // an in-page sign-in for guests, a labelled sample for members, and the + // real feed for owners/admins. So this is neither requireAuth nor adminOnly. + id: "activity", + label: "Activity", + href: "/activity", + requireAuth: false, + actionItem: true, + }, +]; + +// Settings is a destination, not a workspace view, so it sits at the foot of +// the desktop nav column rather than among Hub / Workflows / Analytics. It is +// a normal routable destination on mobile. Its href is non-null (unlike the +// flyout entries above), which the mobile derivation relies on. +export const SETTINGS_NAV_ITEM_DATA: NavItemData & { href: string } = { + id: "settings", + label: "Settings", + href: "/settings", + requireAuth: true, +}; + +/** Desktop items whose click opens a flyout or has special handling. */ +export const ACTION_ITEM_IDS: ReadonlySet = new Set( + NAV_ITEMS_DATA.filter((item) => item.actionItem).map((item) => item.id) +); 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 ? ( diff --git a/tests/unit/mobile-nav-sheet.test.ts b/tests/unit/mobile-nav-sheet.test.ts new file mode 100644 index 000000000..9054f83ba --- /dev/null +++ b/tests/unit/mobile-nav-sheet.test.ts @@ -0,0 +1,211 @@ +// House style: this codebase does not depend on @testing-library/react, so the +// MobileNavSheet's pure decision logic (visible items, active state, tap +// action) is tested via the helper module rather than a DOM render — the same +// pattern settings-nav-search.test.ts uses. + +import { describe, expect, it } from "vitest"; +import { + decideMobileNavAction, + isMobileNavActive, + MOBILE_NAV_ITEMS, + type MobileNavItem, + visibleMobileNavItems, +} from "@/components/navigation/mobile-nav-items"; +import { + NAV_ITEMS_DATA, + SETTINGS_NAV_ITEM_DATA, +} from "@/components/navigation/nav-items-data"; + +const OWNER = { isAdmin: true, isOwner: true }; +const ADMIN = { isAdmin: true, isOwner: false }; +const MEMBER = { isAdmin: false, isOwner: false }; + +function ids(items: MobileNavItem[]): string[] { + return items.map((i) => i.id); +} + +describe("visibleMobileNavItems", () => { + it("a member sees the monitoring + account destinations but not owner-only ones", () => { + const visible = visibleMobileNavItems(MEMBER); + expect(visible).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "hub" }), + expect.objectContaining({ id: "workflows" }), + expect.objectContaining({ id: "analytics" }), + expect.objectContaining({ id: "earnings" }), + expect.objectContaining({ id: "activity" }), + expect.objectContaining({ id: "settings" }), + ]) + ); + expect(ids(visible)).not.toContain("held-payments"); + }); + + it("an owner additionally sees owner-only destinations", () => { + expect(ids(visibleMobileNavItems(OWNER))).toContain("held-payments"); + }); + + it("an admin (non-owner) does not see owner-only destinations", () => { + expect(ids(visibleMobileNavItems(ADMIN))).not.toContain("held-payments"); + }); + + it("every visible item has a routable href (no flyout/overlay-only entries leak)", () => { + for (const item of visibleMobileNavItems(MEMBER)) { + expect(item.href.startsWith("/")).toBe(true); + } + }); + + it("the mobile set excludes the desktop flyout/overlay-only actions", () => { + // Address Book is an overlay on desktop and has no page to route to on + // mobile; it must not appear as a dead link. + expect(ids(MOBILE_NAV_ITEMS)).not.toContain("address-book"); + }); + + // Parity with the desktop sidebar. Both surfaces derive from the same + // NAV_ITEMS_DATA (nav-items-data.ts), so these tests assert the derivation + // rule itself against the real source rather than a hand-copied list: a + // destination added to NAV_ITEMS_DATA with a routable surface appears on + // mobile, and one without (desktop-only flyout) does not. + it("covers every desktop destination that has a routable surface", () => { + const desktopRoutable = NAV_ITEMS_DATA.filter( + (item) => item.href !== null || item.mobileHref !== undefined + ); + for (const item of desktopRoutable) { + expect( + MOBILE_NAV_ITEMS.find((i) => i.id === item.id), + `mobile nav is missing the desktop destination "${item.id}"` + ).toBeDefined(); + } + // Settings is a separate shared entry appended to both surfaces. + expect( + MOBILE_NAV_ITEMS.find((i) => i.id === SETTINGS_NAV_ITEM_DATA.id) + ).toBeDefined(); + }); + + it("derives mobile auth gating from the shared source, not a copy", () => { + const expected: Record = {}; + for (const item of NAV_ITEMS_DATA) { + expected[item.id] = item.requireAuth; + } + expected[SETTINGS_NAV_ITEM_DATA.id] = SETTINGS_NAV_ITEM_DATA.requireAuth; + for (const item of MOBILE_NAV_ITEMS) { + expect( + expected[item.id], + `no shared-source entry known for mobile item "${item.id}"` + ).toBeDefined(); + expect( + item.requireAuth, + `${item.id} requireAuth diverges from the shared source` + ).toBe(expected[item.id]); + } + }); + + it("keeps owner-only gating aligned with the shared source", () => { + const ownerOnlyIds: string[] = NAV_ITEMS_DATA.filter( + (item) => item.ownerOnly + ).map((item) => item.id); + for (const item of MOBILE_NAV_ITEMS) { + expect(item.ownerOnly === true).toBe(ownerOnlyIds.includes(item.id)); + } + }); + + it("keeps admin-only gating aligned with the shared source", () => { + // 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. 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 + const dataAdminOnlyIds: string[] = NAV_ITEMS_DATA.filter( + (d) => d.adminOnly + ).map((d) => d.id); + for (const item of MOBILE_NAV_ITEMS) { + 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"); + }); +}); + +describe("isMobileNavActive", () => { + it("marks the exact route active", () => { + expect(isMobileNavActive("/analytics", "/analytics")).toBe(true); + }); + + it("marks a subroute of a section active", () => { + expect(isMobileNavActive("/workflows", "/workflows/abc123")).toBe(true); + expect(isMobileNavActive("/settings", "/settings/org-1/organization")).toBe( + true + ); + }); + + it("does not mark a sibling route active", () => { + expect(isMobileNavActive("/analytics", "/earnings")).toBe(false); + expect(isMobileNavActive("/workflows", "/workflow-other")).toBe(false); + }); + + it("root href matches only the exact root", () => { + expect(isMobileNavActive("/", "/")).toBe(true); + expect(isMobileNavActive("/", "/analytics")).toBe(false); + }); +}); + +describe("decideMobileNavAction", () => { + const signedIn = { name: "Ada", email: "ada@keeperhub.com" }; + const signedOut: { name?: string | null; email?: string | null } | null = + null; + const anonymous = { name: "Anonymous", email: "temp-abc@keeperhub.com" }; + + function itemById(id: string): MobileNavItem { + const found = MOBILE_NAV_ITEMS.find((i) => i.id === id); + if (!found) { + throw new Error(`no mobile nav item with id ${id}`); + } + return found; + } + + it("signed-in user routes everywhere", () => { + for (const item of MOBILE_NAV_ITEMS) { + expect(decideMobileNavAction(item, signedIn)).toEqual({ + kind: "route", + }); + } + }); + + it("signed-out user is auth-prompted on requireAuth destinations", () => { + const analytics = itemById("analytics"); + const hub = itemById("hub"); + expect(decideMobileNavAction(analytics, signedOut)).toEqual({ + kind: "auth-prompt", + }); + expect(decideMobileNavAction(hub, signedOut)).toEqual({ + kind: "route", + }); + }); + + it("anonymous user is treated like signed-out on requireAuth destinations", () => { + const analytics = itemById("analytics"); + expect(decideMobileNavAction(analytics, anonymous)).toEqual({ + kind: "auth-prompt", + }); + }); +});
Name StatusSourceDurationNetworkGas + Source + + Duration + + Network + Gas Time