Mobile-first app shell: bottom tab nav + slim top bar (#98) #109
@@ -1,10 +1,14 @@
|
|||||||
import { Link, Outlet, useNavigate } from '@tanstack/react-router'
|
import { useState } from 'react'
|
||||||
|
import { Link, Outlet, useMatchRoute, useNavigate } from '@tanstack/react-router'
|
||||||
import { Toaster } from '@/components/ui/toast'
|
import { Toaster } from '@/components/ui/toast'
|
||||||
import { useLogout, useMe } from '@/lib/auth'
|
import { useLogout, useMe } from '@/lib/auth'
|
||||||
|
|
||||||
const navLinks = [
|
// Top-level sections. `icon` is only used by the mobile bottom bar; the desktop
|
||||||
{ to: '/gardens', label: 'Gardens' },
|
// top bar shows labels alone. Settings is filtered to admins at render.
|
||||||
{ to: '/plants', label: 'Plants' },
|
const sections = [
|
||||||
|
{ to: '/gardens', label: 'Gardens', icon: '🏡', adminOnly: false },
|
||||||
|
{ to: '/plants', label: 'Plants', icon: '🌿', adminOnly: false },
|
||||||
|
{ to: '/settings', label: 'Settings', icon: '⚙️', adminOnly: true },
|
||||||
] as const
|
] as const
|
||||||
|
|
||||||
// TanStack Router concatenates the base className with activeProps/inactiveProps,
|
// TanStack Router concatenates the base className with activeProps/inactiveProps,
|
||||||
@@ -14,73 +18,56 @@ const navLinkBase = 'rounded-md px-3 py-1.5 text-sm font-medium transition-color
|
|||||||
const navLinkActive = 'bg-border/60 text-fg'
|
const navLinkActive = 'bg-border/60 text-fg'
|
||||||
const navLinkInactive = 'text-muted hover:bg-border/60 hover:text-fg'
|
const navLinkInactive = 'text-muted hover:bg-border/60 hover:text-fg'
|
||||||
|
|
||||||
/** Top-level chrome: a sticky nav bar plus the routed page in an <Outlet>. */
|
/**
|
||||||
|
* Top-level chrome. Mobile-first: a slim top bar (brand + account) with the
|
||||||
|
* section nav moved to a thumb-reachable bottom tab bar; the desktop breakpoint
|
||||||
|
* (`md:`) restores the inline top nav. The editor is a full-screen context, so it
|
||||||
|
* owns the bottom of the screen — the app bottom bar hides there (the brand link
|
||||||
|
* is the way back to the gardens list), leaving exactly one bottom bar per route.
|
||||||
|
*/
|
||||||
export function AppShell() {
|
export function AppShell() {
|
||||||
const me = useMe()
|
const me = useMe()
|
||||||
const logout = useLogout()
|
|
||||||
const navigate = useNavigate()
|
|
||||||
const user = me.data
|
const user = me.data
|
||||||
|
const matchRoute = useMatchRoute()
|
||||||
|
|
||||||
async function onLogout() {
|
// The garden editor route ('/gardens/$gardenId') runs full-screen. Fuzzy:false
|
||||||
try {
|
// so the '/gardens' list itself still shows the bottom bar.
|
||||||
await logout.mutateAsync()
|
const inEditor = !!matchRoute({ to: '/gardens/$gardenId', fuzzy: false })
|
||||||
await navigate({ to: '/login' })
|
const showBottomNav = !!user && !inEditor
|
||||||
} catch {
|
|
||||||
// The logout request failed, so the session is still valid server-side:
|
const visibleSections = sections.filter((s) => !s.adminOnly || user?.isAdmin)
|
||||||
// leave the user where they are (the button re-enables for a retry) rather
|
|
||||||
// than pretending they're signed out. logout.isError drives the title below.
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex min-h-full flex-col">
|
<div className="flex min-h-full flex-col">
|
||||||
<header className="sticky top-0 z-10 border-b border-border bg-surface/90 backdrop-blur">
|
<header className="sticky top-0 z-20 border-b border-border bg-surface/90 backdrop-blur">
|
||||||
<nav className="mx-auto flex max-w-5xl items-center gap-4 px-4 py-3">
|
<nav className="mx-auto flex max-w-5xl items-center gap-4 px-4 py-3">
|
||||||
<Link to="/gardens" className="text-lg font-semibold text-accent-strong">
|
<Link to="/gardens" className="text-lg font-semibold text-accent-strong">
|
||||||
🌱 pansy
|
🌱 pansy
|
||||||
</Link>
|
</Link>
|
||||||
|
|
||||||
<div className="flex flex-1 items-center gap-1">
|
{/* Desktop inline section links. Hidden on mobile, where the bottom bar
|
||||||
|
carries them. */}
|
||||||
|
<div className="hidden flex-1 items-center gap-1 md:flex">
|
||||||
{user &&
|
{user &&
|
||||||
navLinks.map((l) => (
|
visibleSections.map((s) => (
|
||||||
<Link
|
<Link
|
||||||
key={l.to}
|
key={s.to}
|
||||||
to={l.to}
|
to={s.to}
|
||||||
className={navLinkBase}
|
className={navLinkBase}
|
||||||
activeProps={{ className: navLinkActive }}
|
activeProps={{ className: navLinkActive }}
|
||||||
inactiveProps={{ className: navLinkInactive }}
|
inactiveProps={{ className: navLinkInactive }}
|
||||||
>
|
>
|
||||||
{l.label}
|
{s.label}
|
||||||
</Link>
|
</Link>
|
||||||
))}
|
))}
|
||||||
{/* Settings is admin-only, matching the server's requireAdmin gate.
|
|
||||||
A non-admin who typed /settings still gets a 403 from the API — the
|
|
||||||
hidden link is convenience, not the security boundary. */}
|
|
||||||
{user?.isAdmin && (
|
|
||||||
<Link
|
|
||||||
to="/settings"
|
|
||||||
className={navLinkBase}
|
|
||||||
activeProps={{ className: navLinkActive }}
|
|
||||||
inactiveProps={{ className: navLinkInactive }}
|
|
||||||
>
|
|
||||||
Settings
|
|
||||||
</Link>
|
|
||||||
)}
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{/* Spacer so account/sign-out sits right on mobile (the desktop links
|
||||||
|
own flex-1 above). */}
|
||||||
|
<div className="flex-1 md:hidden" />
|
||||||
|
|
||||||
{user ? (
|
{user ? (
|
||||||
<div className="flex items-center gap-2">
|
<AccountMenu displayName={user.displayName} />
|
||||||
<span className="hidden text-sm text-muted sm:inline">{user.displayName}</span>
|
|
||||||
<button
|
|
||||||
type="button"
|
|
||||||
onClick={onLogout}
|
|
||||||
disabled={logout.isPending}
|
|
||||||
title={logout.isError ? 'Sign out failed — try again' : undefined}
|
|
||||||
className="rounded-md px-3 py-1.5 text-sm font-medium text-muted transition-colors hover:text-fg disabled:opacity-60"
|
|
||||||
>
|
|
||||||
{logout.isPending ? 'Signing out…' : logout.isError ? 'Retry sign out' : 'Sign out'}
|
|
||||||
</button>
|
|
||||||
</div>
|
|
||||||
) : (
|
) : (
|
||||||
<Link
|
<Link
|
||||||
to="/login"
|
to="/login"
|
||||||
@@ -92,11 +79,123 @@ export function AppShell() {
|
|||||||
</nav>
|
</nav>
|
||||||
</header>
|
</header>
|
||||||
|
|
||||||
<main className="mx-auto w-full max-w-5xl flex-1 px-4 py-6">
|
{/* Bottom bar clearance on mobile so content isn't hidden behind it. */}
|
||||||
|
<main
|
||||||
|
className={`mx-auto w-full max-w-5xl flex-1 px-4 py-6 ${
|
||||||
|
showBottomNav ? 'pb-[calc(4.5rem+env(safe-area-inset-bottom))] md:pb-6' : ''
|
||||||
|
}`}
|
||||||
|
>
|
||||||
<Outlet />
|
<Outlet />
|
||||||
</main>
|
</main>
|
||||||
|
|
||||||
|
{showBottomNav && <BottomNav sections={visibleSections} />}
|
||||||
|
|
||||||
<Toaster />
|
<Toaster />
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Account control: a compact button that toggles a small sign-out popover. On
|
||||||
|
* desktop the display name shows inline; on mobile it lives inside the popover. */
|
||||||
|
function AccountMenu({ displayName }: { displayName: string }) {
|
||||||
|
const logout = useLogout()
|
||||||
|
const navigate = useNavigate()
|
||||||
|
const [open, setOpen] = useState(false)
|
||||||
|
|
||||||
|
async function onLogout() {
|
||||||
|
try {
|
||||||
|
await logout.mutateAsync()
|
||||||
|
await navigate({ to: '/login' })
|
||||||
|
} catch {
|
||||||
|
// The logout request failed, so the session is still valid server-side:
|
||||||
|
// leave the user where they are (the button re-enables for a retry) rather
|
||||||
|
// than pretending they're signed out. logout.isError drives the label.
|
||||||
|
setOpen(false)
|
||||||
|
|
|||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const initial = displayName.trim().charAt(0).toUpperCase() || '·'
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div className="relative">
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => setOpen((v) => !v)}
|
||||||
|
aria-haspopup="menu"
|
||||||
|
aria-expanded={open}
|
||||||
|
className="flex items-center gap-2 rounded-md px-2 py-1.5 text-sm text-muted transition-colors hover:text-fg"
|
||||||
|
>
|
||||||
|
<span className="hidden sm:inline">{displayName}</span>
|
||||||
|
<span
|
||||||
|
aria-hidden
|
||||||
|
className="flex size-8 items-center justify-center rounded-full bg-border/60 text-sm font-semibold text-fg"
|
||||||
|
>
|
||||||
|
{initial}
|
||||||
|
</span>
|
||||||
|
</button>
|
||||||
|
|
||||||
|
{open && (
|
||||||
|
<>
|
||||||
|
{/* Full-screen click-away backdrop; reliably closes on an outside tap
|
||||||
|
without a document listener. */}
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
aria-label="Close menu"
|
||||||
|
className="fixed inset-0 z-30 cursor-default"
|
||||||
|
onClick={() => setOpen(false)}
|
||||||
|
/>
|
||||||
|
<div
|
||||||
|
role="menu"
|
||||||
|
className="absolute right-0 z-40 mt-2 w-48 rounded-lg border border-border bg-surface p-1 shadow-lg"
|
||||||
|
>
|
||||||
|
<p className="truncate px-3 py-2 text-xs text-muted">
|
||||||
|
Signed in as <span className="text-fg">{displayName}</span>
|
||||||
|
</p>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
role="menuitem"
|
||||||
|
onClick={onLogout}
|
||||||
|
disabled={logout.isPending}
|
||||||
|
title={logout.isError ? 'Sign out failed — try again' : undefined}
|
||||||
|
className="w-full rounded-md px-3 py-2 text-left text-sm font-medium text-muted transition-colors hover:bg-border/60 hover:text-fg disabled:opacity-60"
|
||||||
|
>
|
||||||
|
{logout.isPending ? 'Signing out…' : logout.isError ? 'Retry sign out' : 'Sign out'}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Mobile bottom tab bar for the top-level sections (thumb zone, safe-area aware). */
|
||||||
|
function BottomNav({
|
||||||
|
sections,
|
||||||
|
}: {
|
||||||
|
sections: ReadonlyArray<{ to: string; label: string; icon: string }>
|
||||||
|
gitea-actions
commented
🟡 BottomNav sections prop type is a hand-written partial copy of the sections array shape, prone to drift maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **BottomNav sections prop type is a hand-written partial copy of the sections array shape, prone to drift**
_maintainability · flagged by 2 models_
- `web/src/components/layout/AppShell.tsx:175` — `BottomNav`'s `sections` prop is typed by hand as `ReadonlyArray<{ to: string; label: string; icon: string }>` rather than derived from the actual source of truth, the `sections` array at lines 8-12 (`typeof sections[number]` or a `Pick`). Confirmed: the real array also carries `adminOnly`, so this is already a partial, hand-copied duplicate of the shape — the kind of thing that silently drifts when a field is added.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
}) {
|
||||||
|
return (
|
||||||
|
<nav
|
||||||
|
aria-label="Sections"
|
||||||
|
className="fixed inset-x-0 bottom-0 z-20 border-t border-border bg-surface/95 pb-[env(safe-area-inset-bottom)] backdrop-blur md:hidden"
|
||||||
|
gitea-actions
commented
🟡 Second backdrop-blur layer adds mobile scroll/compositor cost performance · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Second backdrop-blur layer adds mobile scroll/compositor cost**
_performance · flagged by 1 model_
- `web/src/components/layout/AppShell.tsx:180` — The mobile bottom nav adds `backdrop-blur` alongside the header's existing blur, creating **two simultaneous expensive compositor layers** on mobile. During scroll, the browser must continuously capture and blur content behind both fixed elements, which can cause frame drops and increased battery usage on lower-end devices. The visual benefit is marginal because `bg-surface/95` is already nearly opaque. **Fix:** Remove `backdrop-blur` from the bot…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
>
|
||||||
|
<ul className="mx-auto flex max-w-5xl items-stretch justify-around">
|
||||||
|
{sections.map((s) => (
|
||||||
|
<li key={s.to} className="flex-1">
|
||||||
|
<Link
|
||||||
|
to={s.to}
|
||||||
|
className="flex min-h-[3.25rem] flex-col items-center justify-center gap-0.5 py-1.5 text-xs font-medium text-muted transition-colors"
|
||||||
|
gitea-actions
commented
🔴 BottomNav active tab has conflicting text-muted/text-accent-strong classes correctness, maintainability · flagged by 3 models
🪰 Gadfly · advisory 🔴 **BottomNav active tab has conflicting text-muted/text-accent-strong classes**
_correctness, maintainability · flagged by 3 models_
- **`web/src/components/layout/AppShell.tsx:187`** — The `BottomNav` `Link` base `className` includes `text-muted`, but `activeProps` only adds `text-accent-strong`. The file's own comment (line 14-16) explains that TanStack Router concatenates base + state classes, so conflicting color utilities must live *only* in `activeProps`/`inactiveProps` to avoid ambiguous overrides. Because `text-muted` is present in the base string, both classes end up on the active tab element; whichever Tailwind emit…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
activeProps={{ className: 'text-accent-strong' }}
|
||||||
|
inactiveProps={{ className: 'text-muted hover:text-fg' }}
|
||||||
|
>
|
||||||
|
<span aria-hidden className="text-lg leading-none">
|
||||||
|
{s.icon}
|
||||||
|
</span>
|
||||||
|
{s.label}
|
||||||
|
</Link>
|
||||||
|
</li>
|
||||||
|
))}
|
||||||
|
</ul>
|
||||||
|
</nav>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user
🟠 Logout failure closes the popover, hiding the only retry/error affordance with no toast fallback
correctness, error-handling, maintainability · flagged by 4 models
web/src/components/layout/AppShell.tsx:113— On logout failure,onLogout's catch block callssetOpen(false), closing the account popover. That popover is the only place the failure is surfaced (title="Sign out failed — try again"and the "Retry sign out" label live inside it, at lines 159/162), and the comment directly above the call still says "leave the user where they are (the button re-enables for a retry)" — the code no longer does that; it hides the retry button instead. `use…🪰 Gadfly · advisory