[PULSE-36] Funnels UI - builder and report #8
@@ -1,12 +1,10 @@
|
||||
'use client'
|
||||
|
|
||||
|
||||
import { useAuth } from '@/lib/auth/context'
|
||||
import { useEffect, useState } from 'react'
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react'
|
||||
Unused auth import
Prompt To Fix With AI**Unused auth import**
`useAuth` is imported here but never referenced, which will fail lint/typecheck in projects enforcing `no-unused-vars`. Remove the import to avoid CI failures.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/[funnelId]/page.tsx
Line: 1:3
Comment:
**Unused auth import**
`useAuth` is imported here but never referenced, which will fail lint/typecheck in projects enforcing `no-unused-vars`. Remove the import to avoid CI failures.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Already fixed. Already fixed.
|
||||
import { useParams, useRouter } from 'next/navigation'
|
||||
import { getFunnel, getFunnelStats, deleteFunnel, type Funnel, type FunnelStats } from '@/lib/api/funnels'
|
||||
import { toast, LoadingOverlay, Select, DatePicker, ChevronLeftIcon, ArrowRightIcon, TrashIcon, useTheme } from '@ciphera-net/ui'
|
||||
import Link from 'next/link'
|
||||
import { useMemo } from 'react'
|
||||
import {
|
||||
BarChart,
|
||||
Bar,
|
||||
@@ -45,9 +43,10 @@ export default function FunnelReportPage() {
|
||||
const [stats, setStats] = useState<FunnelStats | null>(null)
|
||||
const [loading, setLoading] = useState(true)
|
||||
const [dateRange, setDateRange] = useState(getDateRange(30))
|
||||
const [datePreset, setDatePreset] = useState<'7' | '30' | 'custom'>('30')
|
||||
const [isDatePickerOpen, setIsDatePickerOpen] = useState(false)
|
||||
|
||||
const loadData = async () => {
|
||||
const loadData = useCallback(async () => {
|
||||
try {
|
||||
setLoading(true)
|
||||
const [funnelData, statsData] = await Promise.all([
|
||||
@@ -61,11 +60,11 @@ export default function FunnelReportPage() {
|
||||
} finally {
|
||||
setLoading(false)
|
||||
}
|
||||
}
|
||||
}, [siteId, funnelId, dateRange])
|
||||
|
||||
useEffect(() => {
|
||||
loadData()
|
||||
}, [siteId, funnelId, dateRange])
|
||||
}, [loadData])
|
||||
|
||||
const { resolvedTheme } = useTheme()
|
||||
const chartColors = useMemo(
|
||||
@@ -129,17 +128,17 @@ export default function FunnelReportPage() {
|
||||
|
||||
<div className="flex items-center gap-2">
|
||||
<Select
|
||||
value={
|
||||
dateRange.start === getDateRange(7).start
|
||||
? '7'
|
||||
: dateRange.start === getDateRange(30).start
|
||||
? '30'
|
||||
: 'custom'
|
||||
}
|
||||
value={datePreset}
|
||||
onChange={(value) => {
|
||||
if (value === '7') setDateRange(getDateRange(7))
|
||||
else if (value === '30') setDateRange(getDateRange(30))
|
||||
else if (value === 'custom') setIsDatePickerOpen(true)
|
||||
if (value === '7') {
|
||||
|
Misleading error state When Prompt To Fix With AI**Misleading error state**
When `loadData()` fails (network/5xx/permission), `funnel`/`stats` remain `null` and the render falls through to `if (!funnel || !stats)` showing "Funnel not found". That message is only correct for a 404; for transient errors it incorrectly looks like a permanent missing resource and provides no retry path. Consider tracking an explicit `error`/`notFound` state and only rendering "not found" on an actual 404 response.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/[funnelId]/page.tsx
Line: 91:96
Comment:
**Misleading error state**
When `loadData()` fails (network/5xx/permission), `funnel`/`stats` remain `null` and the render falls through to `if (!funnel || !stats)` showing "Funnel not found". That message is only correct for a 404; for transient errors it incorrectly looks like a permanent missing resource and provides no retry path. Consider tracking an explicit `error`/`notFound` state and only rendering "not found" on an actual 404 response.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Change: Change:
Added loadError state: 'not_found' | 'error' | null, default null.
In loadData: at the start, setLoadError(null); in the catch, if error instanceof ApiError && error.status === 404 then setLoadError('not_found'), else setLoadError('error') and toast.error('Failed to load funnel data') (toast only for non-404).
Render logic:
loadError === 'not_found' or (!funnel && !stats && !loadError) → show "Funnel not found" (no retry).
loadError === 'error' → show "Failed to load funnel data" and a "Try again" button that calls loadData().
Otherwise, if !funnel || !stats → fallback "Funnel not found".
Why: 404 is shown as “Funnel not found”; other failures (network/5xx/permission) are shown as “Failed to load funnel data” with a retry, so transient errors are no longer treated as a permanent missing resource.
|
||||
setDateRange(getDateRange(7))
|
||||
setDatePreset('7')
|
||||
} else if (value === '30') {
|
||||
setDateRange(getDateRange(30))
|
||||
setDatePreset('30')
|
||||
} else if (value === 'custom') {
|
||||
setIsDatePickerOpen(true)
|
||||
}
|
||||
}}
|
||||
options={[
|
||||
{ value: '7', label: 'Last 7 days' },
|
||||
@@ -215,7 +214,7 @@ export default function FunnelReportPage() {
|
||||
/>
|
||||
<Bar dataKey="visitors" radius={[4, 4, 0, 0]} barSize={60}>
|
||||
{chartData.map((entry, index) => (
|
||||
<Cell key={`cell-${index}`} fill={BRAND_ORANGE} fillOpacity={1 - (index * 0.15)} />
|
||||
<Cell key={`cell-${index}`} fill={BRAND_ORANGE} fillOpacity={Math.max(0.1, 1 - index * 0.15)} />
|
||||
))}
|
||||
</Bar>
|
||||
</BarChart>
|
||||
@@ -285,6 +284,7 @@ export default function FunnelReportPage() {
|
||||
onClose={() => setIsDatePickerOpen(false)}
|
||||
onApply={(range) => {
|
||||
setDateRange(range)
|
||||
setDatePreset('custom')
|
||||
setIsDatePickerOpen(false)
|
||||
}}
|
||||
initialRange={dateRange}
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
'use client'
|
||||
|
Unused import Prompt To Fix With AIUnused import `useAuth` - not referenced in the component.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/new/page.tsx
Line: 3:3
Comment:
Unused import `useAuth` - not referenced in the component.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Change: Removed import { useAuth } from '@/lib/auth/context' from app/sites/[id]/funnels/new/page.tsx. Change: Removed import { useAuth } from '@/lib/auth/context' from app/sites/[id]/funnels/new/page.tsx.
Why: The component doesn’t use useAuth.
|
||||
|
||||
import { useAuth } from '@/lib/auth/context'
|
||||
import { useState } from 'react'
|
||||
|
Unused auth import
Prompt To Fix With AI**Unused auth import**
`useAuth` is imported but not used in this page, which will trip `no-unused-vars`/typecheck in many setups. Remove the import.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/new/page.tsx
Line: 1:3
Comment:
**Unused auth import**
`useAuth` is imported but not used in this page, which will trip `no-unused-vars`/typecheck in many setups. Remove the import.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Already fixed. Already fixed.
|
||||
import { useParams, useRouter } from 'next/navigation'
|
||||
import { createFunnel, type CreateFunnelRequest, type FunnelStep } from '@/lib/api/funnels'
|
||||
|
||||
@@ -1,14 +1,12 @@
|
||||
'use client'
|
||||
|
ReferenceError on mount Same pattern as the report page: Prompt To Fix With AI**ReferenceError on mount**
Same pattern as the report page: `useEffect` calls `loadFunnels()` before `const loadFunnels = async () => { ... }` is initialized, so this page will throw `ReferenceError: Cannot access 'loadFunnels' before initialization` on mount. Define `loadFunnels` above the effect (or use a hoisted function declaration).
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/page.tsx
Line: 15:21
Comment:
**ReferenceError on mount**
Same pattern as the report page: `useEffect` calls `loadFunnels()` before `const loadFunnels = async () => { ... }` is initialized, so this page will throw `ReferenceError: Cannot access 'loadFunnels' before initialization` on mount. Define `loadFunnels` above the effect (or use a hoisted function declaration).
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Change: The loadFunnels definition was moved above the useEffect that calls it. Change: The loadFunnels definition was moved above the useEffect that calls it.
Why: Same pattern as the report page: the effect was calling loadFunnels() before the const loadFunnels = async () => { ... } was initialized. Defining loadFunnels before the useEffect fixes the ReferenceError on mount.
Missing Prompt To Fix With AIMissing `loadFunnels` in dependency array causes React exhaustive-deps warning and stale closure issues. Either add `loadFunnels` to the dependency array or use `useCallback` to memoize `loadFunnels`.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/page.tsx
Line: 19:21
Comment:
Missing `loadFunnels` in dependency array causes React exhaustive-deps warning and stale closure issues. Either add `loadFunnels` to the dependency array or use `useCallback` to memoize `loadFunnels`.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Unused variable Prompt To Fix With AIUnused variable `user` destructured from `useAuth()` - not referenced in the component.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: app/sites/[id]/funnels/page.tsx
Line: 11:11
Comment:
Unused variable `user` destructured from `useAuth()` - not referenced in the component.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Change: Removed useAuth() and its import from app/sites/[id]/funnels/page.tsx (the list page). Change: Removed useAuth() and its import from app/sites/[id]/funnels/page.tsx (the list page).
Why: Only user was destructured and it wasn’t used, so the whole hook and import were removed.
Change: Wrapped loadFunnels in useCallback with deps [siteId], and set the effect dependency array to [loadFunnels]. Change: Wrapped loadFunnels in useCallback with deps [siteId], and set the effect dependency array to [loadFunnels].
Why: Same as above: correct deps and no stale closure.
|
||||
|
||||
import { useAuth } from '@/lib/auth/context'
|
||||
import { useEffect, useState } from 'react'
|
||||
import { useCallback, useEffect, useState } from 'react'
|
||||
import { useParams, useRouter } from 'next/navigation'
|
||||
import { listFunnels, deleteFunnel, type Funnel } from '@/lib/api/funnels'
|
||||
import { toast, LoadingOverlay, PlusIcon, ArrowRightIcon, ChevronLeftIcon, TrashIcon } from '@ciphera-net/ui'
|
||||
import Link from 'next/link'
|
||||
|
||||
export default function FunnelsPage() {
|
||||
const { user } = useAuth()
|
||||
const params = useParams()
|
||||
const router = useRouter()
|
||||
const siteId = params.id as string
|
||||
@@ -16,7 +14,7 @@ export default function FunnelsPage() {
|
||||
const [funnels, setFunnels] = useState<Funnel[]>([])
|
||||
const [loading, setLoading] = useState(true)
|
||||
|
||||
const loadFunnels = async () => {
|
||||
const loadFunnels = useCallback(async () => {
|
||||
try {
|
||||
setLoading(true)
|
||||
const data = await listFunnels(siteId)
|
||||
@@ -26,11 +24,11 @@ export default function FunnelsPage() {
|
||||
} finally {
|
||||
setLoading(false)
|
||||
}
|
||||
}
|
||||
}, [siteId])
|
||||
|
||||
useEffect(() => {
|
||||
loadFunnels()
|
||||
}, [siteId])
|
||||
}, [loadFunnels])
|
||||
|
||||
const handleDelete = async (e: React.MouseEvent, funnelId: string) => {
|
||||
e.preventDefault() // Prevent navigation
|
||||
|
||||
Reference in New Issue
Block a user
ReferenceError on mount
useEffectcallsloadData()beforeloadDatais defined (const loadData = async () => { ... }), which throwsReferenceError: Cannot access 'loadData' before initializationon first render. Move theloadDatadefinition above theuseEffect(or switch to a hoistedfunction loadData() {}) so the page doesn’t crash.Prompt To Fix With AI
Change: The loadData definition was moved above the useEffect that calls it.
Why: const loadData = async () => { ... } is not hoisted, so when the effect ran on the first render it was still in the temporal dead zone and threw “Cannot access 'loadData' before initialization”. Defining loadData before the useEffect ensures it exists when the effect runs.
Missing
loadDatain dependency array causes React exhaustive-deps warning and stale closure issues. Either addloadDatato the dependency array or useuseCallbackto memoizeloadData.Prompt To Fix With AI
Date comparison using string equality (
dateRange.start === getDateRange(7).start) creates new Date objects on every render, causing unnecessary re-renders and always evaluating to false since objects are different references.Prompt To Fix With AI
Unused import
useAuth- not referenced in the component.Prompt To Fix With AI
Opacity calculation can go negative for funnels with more than 6 steps, causing render issues. Wrap in
Math.max(0.1, ...)to prevent negative opacity.Prompt To Fix With AI
Change: Replaced fillOpacity={1 - (index * 0.15)} with fillOpacity={Math.max(0.1, 1 - index * 0.15)}.
Why: For index ≥ 7, 1 - index * 0.15 becomes negative; capping with Math.max(0.1, ...) keeps opacity in a valid range and avoids render issues.
Change: Removed import { useAuth } from '@/lib/auth/context' from app/sites/[id]/funnels/[funnelId]/page.tsx.
Why: The report page doesn’t use useAuth.
Change: Introduced datePreset state: '7' | '30' | 'custom', default '30'. The Select uses value={datePreset}. In onChange: for '7'/'30' we call setDateRange(getDateRange(7)) / setDateRange(getDateRange(30)) and setDatePreset('7') / setDatePreset('30'); for 'custom' we only open the picker. In the DatePicker onApply we call setDatePreset('custom') when applying a custom range.
Why: We no longer call getDateRange(7) / getDateRange(30) on every render to derive the Select value; the value comes from datePreset, so no extra Date allocations or reference issues.
Change: Wrapped the loader in useCallback with deps [siteId, funnelId, dateRange], and made the effect depend only on [loadData].
Why: loadData is now stable per (siteId, funnelId, dateRange), so the effect runs when any of those change and satisfies exhaustive-deps without stale closures.