From 034f12d0aa2109c8f68a657b9645836cd3f1abc8 Mon Sep 17 00:00:00 2001
From: Frankie <frankie@cartsnitch.com>
Date: Tue, 17 Mar 2026 16:36:12 +0000
Subject: [PATCH] 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>
---
 src/App.tsx                       | 21 +++++++++-------
 src/components/BottomNav.tsx      |  2 +-
 src/components/ProtectedRoute.tsx | 12 +++++++++
 src/components/SparklineChart.tsx | 33 ++++++++++++++++++++++++
 src/pages/Alerts.tsx              |  2 +-
 src/pages/Dashboard.tsx           | 42 +++++++++++--------------------
 src/pages/Login.tsx               | 12 ++++++---
 src/pages/Register.tsx            | 12 ++++++---
 src/pages/Settings.tsx            |  2 +-
 9 files changed, 90 insertions(+), 48 deletions(-)
 create mode 100644 src/components/ProtectedRoute.tsx
 create mode 100644 src/components/SparklineChart.tsx

diff --git a/src/App.tsx b/src/App.tsx
index f7829da..bfd515a 100644
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -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 />} />
diff --git a/src/components/BottomNav.tsx b/src/components/BottomNav.tsx
index 136fa77..4c632b8 100644
--- a/src/components/BottomNav.tsx
+++ b/src/components/BottomNav.tsx
@@ -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
diff --git a/src/components/ProtectedRoute.tsx b/src/components/ProtectedRoute.tsx
new file mode 100644
index 0000000..b6bb0fa
--- /dev/null
+++ b/src/components/ProtectedRoute.tsx
@@ -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 />
+}
diff --git a/src/components/SparklineChart.tsx b/src/components/SparklineChart.tsx
new file mode 100644
index 0000000..6f81760
--- /dev/null
+++ b/src/components/SparklineChart.tsx
@@ -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>
+  )
+}
diff --git a/src/pages/Alerts.tsx b/src/pages/Alerts.tsx
index 2712d16..737bdf6 100644
--- a/src/pages/Alerts.tsx
+++ b/src/pages/Alerts.tsx
@@ -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>
diff --git a/src/pages/Dashboard.tsx b/src/pages/Dashboard.tsx
index 11f2c64..aff8a91 100644
--- a/src/pages/Dashboard.tsx
+++ b/src/pages/Dashboard.tsx
@@ -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>
   )
 }
diff --git a/src/pages/Login.tsx b/src/pages/Login.tsx
index 82a2316..68009ca 100644
--- a/src/pages/Login.tsx
+++ b/src/pages/Login.tsx
@@ -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)
     }
diff --git a/src/pages/Register.tsx b/src/pages/Register.tsx
index 1e33959..a20afad 100644
--- a/src/pages/Register.tsx
+++ b/src/pages/Register.tsx
@@ -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)
     }
diff --git a/src/pages/Settings.tsx b/src/pages/Settings.tsx
index b51efcd..43e5d2d 100644
--- a/src/pages/Settings.tsx
+++ b/src/pages/Settings.tsx
@@ -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'