fix: address critical and major PR review issues

- Lazy-load Recharts via SparklineChart component with React.lazy + Suspense
- Gate mock auth fallback behind VITE_MOCK_AUTH env var in Login and Register
- Add ProtectedRoute component to guard authenticated routes
- Fix touch target size on New Alert button (min-h-10 -> min-h-12)
- Replace invalid safe-area-pb class with pb-[env(safe-area-inset-bottom)]
- Fix theme toggle button touch targets (min-h-10 -> min-h-12)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Frankie
2026-03-17 16:36:12 +00:00
parent 6d576bad40
commit 5563b5f145
9 changed files with 90 additions and 48 deletions
+12 -9
View File
@@ -1,6 +1,7 @@
import { BrowserRouter, Routes, Route } from 'react-router-dom'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { Layout } from './components/Layout.tsx'
import { ProtectedRoute } from './components/ProtectedRoute.tsx'
import { Dashboard } from './pages/Dashboard.tsx'
import { Purchases } from './pages/Purchases.tsx'
import { PurchaseDetail } from './pages/PurchaseDetail.tsx'
@@ -31,15 +32,17 @@ export default function App() {
<Routes>
<Route element={<Layout />}>
<Route index element={<Dashboard />} />
<Route path="purchases" element={<Purchases />} />
<Route path="purchases/:id" element={<PurchaseDetail />} />
<Route path="products" element={<Products />} />
<Route path="products/:id" element={<ProductDetail />} />
<Route path="compare/:productId" element={<StoreComparison />} />
<Route path="coupons" element={<Coupons />} />
<Route path="alerts" element={<Alerts />} />
<Route path="settings" element={<Settings />} />
<Route path="account-linking" element={<AccountLinking />} />
<Route element={<ProtectedRoute />}>
<Route path="purchases" element={<Purchases />} />
<Route path="purchases/:id" element={<PurchaseDetail />} />
<Route path="products" element={<Products />} />
<Route path="products/:id" element={<ProductDetail />} />
<Route path="compare/:productId" element={<StoreComparison />} />
<Route path="coupons" element={<Coupons />} />
<Route path="alerts" element={<Alerts />} />
<Route path="settings" element={<Settings />} />
<Route path="account-linking" element={<AccountLinking />} />
</Route>
</Route>
<Route path="login" element={<Login />} />
<Route path="register" element={<Register />} />
+1 -1
View File
@@ -10,7 +10,7 @@ const navItems = [
export function BottomNav() {
return (
<nav className="fixed bottom-0 left-0 right-0 z-50 border-t border-gray-200 bg-white safe-area-pb">
<nav className="fixed bottom-0 left-0 right-0 z-50 border-t border-gray-200 bg-white pb-[env(safe-area-inset-bottom)]">
<div className="mx-auto flex max-w-lg items-center justify-around">
{navItems.map(({ to, label, icon: Icon }) => (
<NavLink
+12
View File
@@ -0,0 +1,12 @@
import { Navigate, Outlet } from 'react-router-dom'
import { useAuthStore } from '../stores/auth.ts'
export function ProtectedRoute() {
const isAuthenticated = useAuthStore((s) => s.isAuthenticated)
if (!isAuthenticated) {
return <Navigate to="/login" replace />
}
return <Outlet />
}
+33
View File
@@ -0,0 +1,33 @@
import { LineChart, Line, ResponsiveContainer } from 'recharts'
export function SparklineCard({
label,
data,
current,
}: {
label: string
data: { date: string; price: number }[]
current: string
}) {
return (
<div className="flex items-center gap-4 rounded-xl bg-white p-4 shadow-sm">
<div className="min-w-0 flex-1">
<p className="text-sm font-medium text-gray-900">{label}</p>
<p className="text-lg font-bold text-gray-900">{current}</p>
</div>
<div className="h-10 w-24">
<ResponsiveContainer width="100%" height="100%">
<LineChart data={data}>
<Line
type="monotone"
dataKey="price"
stroke="#1e40af"
strokeWidth={2}
dot={false}
/>
</LineChart>
</ResponsiveContainer>
</div>
</div>
)
}
+1 -1
View File
@@ -20,7 +20,7 @@ export function Alerts() {
<h1 className="text-2xl font-bold text-gray-900">Price Alerts</h1>
<button
onClick={() => setShowCreate(!showCreate)}
className="min-h-10 rounded-full bg-brand-blue px-4 text-sm font-medium text-white active:bg-brand-blue/90"
className="min-h-12 rounded-full bg-brand-blue px-4 text-sm font-medium text-white active:bg-brand-blue/90"
>
+ New Alert
</button>
+14 -28
View File
@@ -1,9 +1,13 @@
import React, { Suspense } from 'react'
import { Link } from 'react-router-dom'
import { LineChart, Line, ResponsiveContainer } from 'recharts'
import { useAuthStore } from '../stores/auth.ts'
import { mockPurchases, mockAlerts, getMockPriceHistory } from '../lib/mock-data.ts'
import { StoreIcon } from '../components/StoreIcon.tsx'
const LazySparklineCard = React.lazy(() =>
import('../components/SparklineChart.tsx').then((mod) => ({ default: mod.SparklineCard }))
)
const sparklineData = getMockPriceHistory('prod10').filter((p) => p.storeId === 'meijer').slice(-8)
const milkSparkline = getMockPriceHistory('prod1').filter((p) => p.storeId === 'kroger').slice(-8)
@@ -84,8 +88,10 @@ export function Dashboard() {
<section className="mt-6">
<h2 className="mb-3 text-lg font-semibold text-gray-700">Price Trends</h2>
<div className="space-y-3">
<SparklineCard label="Eggs (dozen)" data={sparklineData} current="$5.44" />
<SparklineCard label="Whole Milk (1 gal)" data={milkSparkline} current="$3.29" />
<Suspense fallback={<SparklinePlaceholder />}>
<LazySparklineCard label="Eggs (dozen)" data={sparklineData} current="$5.44" />
<LazySparklineCard label="Whole Milk (1 gal)" data={milkSparkline} current="$3.29" />
</Suspense>
</div>
</section>
@@ -145,34 +151,14 @@ export function Dashboard() {
)
}
function SparklineCard({
label,
data,
current,
}: {
label: string
data: { date: string; price: number }[]
current: string
}) {
function SparklinePlaceholder() {
return (
<div className="flex items-center gap-4 rounded-xl bg-white p-4 shadow-sm">
<div className="flex items-center gap-4 rounded-xl bg-white p-4 shadow-sm animate-pulse">
<div className="min-w-0 flex-1">
<p className="text-sm font-medium text-gray-900">{label}</p>
<p className="text-lg font-bold text-gray-900">{current}</p>
</div>
<div className="h-10 w-24">
<ResponsiveContainer width="100%" height="100%">
<LineChart data={data}>
<Line
type="monotone"
dataKey="price"
stroke="#1e40af"
strokeWidth={2}
dot={false}
/>
</LineChart>
</ResponsiveContainer>
<div className="h-4 w-24 rounded bg-gray-200" />
<div className="mt-2 h-6 w-16 rounded bg-gray-200" />
</div>
<div className="h-10 w-24 rounded bg-gray-100" />
</div>
)
}
+8 -4
View File
@@ -27,10 +27,14 @@ export function Login() {
const res = await api.post<{ user: User; token: string }>('/auth/login', { email, password })
setAuth(res.user, res.token)
navigate('/')
} catch {
// Fallback to mock auth for demo
setAuth(mockUser, 'mock-jwt-token')
navigate('/')
} catch (err) {
if (import.meta.env.VITE_MOCK_AUTH === 'true') {
// Fallback to mock auth for demo
setAuth(mockUser, 'mock-jwt-token')
navigate('/')
} else {
setError('Invalid email or password. Please try again.')
}
} finally {
setLoading(false)
}
+8 -4
View File
@@ -33,10 +33,14 @@ export function Register() {
const res = await api.post<{ user: User; token: string }>('/auth/register', { name, email, password })
setAuth(res.user, res.token)
navigate('/')
} catch {
// Fallback to mock auth for demo
setAuth({ ...mockUser, name, email }, 'mock-jwt-token')
navigate('/')
} catch (err) {
if (import.meta.env.VITE_MOCK_AUTH === 'true') {
// Fallback to mock auth for demo
setAuth({ ...mockUser, name, email }, 'mock-jwt-token')
navigate('/')
} else {
setError('Registration failed. Please try again.')
}
} finally {
setLoading(false)
}
+1 -1
View File
@@ -85,7 +85,7 @@ export function Settings() {
<button
key={t}
onClick={() => setTheme(t)}
className={`min-h-10 flex-1 rounded-lg px-3 py-2 text-sm font-medium capitalize ${
className={`min-h-12 flex-1 rounded-lg px-3 py-2 text-sm font-medium capitalize ${
theme === t
? 'bg-brand-blue text-white'
: 'bg-gray-100 text-gray-700 active:bg-gray-200'