[PULSE-36] Funnels UI - builder and report #8

Merged
uz1mani merged 12 commits from staging into main 2026-02-04 23:16:04 +00:00
7 changed files with 717 additions and 5 deletions
Showing only changes of commit c98286c4e3 - Show all commits

View File

@@ -68,7 +68,7 @@ export default function FunnelReportPage() {
if (!funnel || !stats) {
return (
<div className="container mx-auto px-4 py-8">
<div className="w-full max-w-6xl mx-auto px-4 sm:px-6 py-8">
<p className="text-neutral-600 dark:text-neutral-400">Funnel not found</p>
</div>
)
@@ -88,7 +88,7 @@ export default function FunnelReportPage() {
<div className="flex items-center gap-4">
<Link
href={`/sites/${siteId}/funnels`}
className="p-2 -ml-2 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white rounded-lg hover:bg-neutral-100 dark:hover:bg-neutral-800 transition-colors"
className="p-2 -ml-2 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white rounded-xl hover:bg-neutral-100 dark:hover:bg-neutral-800 transition-colors"
>
<ChevronLeftIcon className="w-5 h-5" />
</Link>
@@ -127,7 +127,8 @@ export default function FunnelReportPage() {
<button
onClick={handleDelete}
className="p-2 text-neutral-400 hover:text-red-500 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-lg transition-colors"
className="p-2 text-neutral-400 hover:text-red-500 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-xl transition-colors"
aria-label="Delete funnel"
>
<TrashIcon className="w-5 h-5" />
greptile-apps[bot] commented 2026-02-04 23:03:01 +00:00 (Migrated from github.com)
Review

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.

Prompt To Fix With AI
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.
**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>
uz1mani commented 2026-02-04 23:05:05 +00:00 (Migrated from github.com)
Review

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.

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.
</button>
@@ -136,7 +137,7 @@ export default function FunnelReportPage() {
{/* Chart */}
<div className="bg-white dark:bg-neutral-900 border border-neutral-200 dark:border-neutral-800 rounded-xl p-6 mb-8">
<h3 className="text-lg font-medium text-neutral-900 dark:text-white mb-6">
<h3 className="text-lg font-semibold text-neutral-900 dark:text-white mb-6">
Funnel Visualization
</h3>
<div className="h-[400px] w-full">
@@ -162,7 +163,7 @@ export default function FunnelReportPage() {
if (active && payload && payload.length) {
const data = payload[0].payload;
return (
<div className="bg-white dark:bg-neutral-900 border border-neutral-200 dark:border-neutral-800 p-3 rounded-lg shadow-lg">
<div className="bg-white dark:bg-neutral-900 border border-neutral-200 dark:border-neutral-800 p-3 rounded-xl shadow-lg">
<p className="font-medium text-neutral-900 dark:text-white mb-1">{label}</p>
<p className="text-brand-orange font-bold text-lg">
{data.visitors.toLocaleString()} visitors

View File

@@ -76,7 +76,7 @@ export default function CreateFunnelPage() {
<div className="mb-8">
<Link
href={`/sites/${siteId}/funnels`}
className="inline-flex items-center gap-2 text-sm text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white mb-6 transition-colors"
className="inline-flex items-center gap-2 text-sm text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white mb-6 rounded-xl hover:bg-neutral-100 dark:hover:bg-neutral-800 px-2 py-1.5 -ml-2 transition-colors"
>
<ChevronLeftIcon className="w-4 h-4" />
Back to Funnels
@@ -119,7 +119,7 @@ export default function CreateFunnelPage() {
<div className="space-y-4 mb-8">
<div className="flex items-center justify-between">
<h3 className="text-lg font-medium text-neutral-900 dark:text-white">
<h3 className="text-lg font-semibold text-neutral-900 dark:text-white">
Funnel Steps
</h3>
</div>
@@ -152,7 +152,7 @@ export default function CreateFunnelPage() {
<select
value={step.type}
onChange={(e) => handleUpdateStep(index, 'type', e.target.value)}
className="w-24 px-2 py-2 bg-white dark:bg-neutral-900 border border-neutral-200 dark:border-neutral-800 rounded-lg text-sm focus:ring-2 focus:ring-brand-orange/20 focus:border-brand-orange outline-none"
className="w-24 px-2 py-2 bg-white dark:bg-neutral-900 border border-neutral-200 dark:border-neutral-800 rounded-xl text-sm focus:ring-2 focus:ring-brand-orange/20 focus:border-brand-orange outline-none"
>
<option value="exact">Exact</option>
<option value="contains">Contains</option>
@@ -172,7 +172,8 @@ export default function CreateFunnelPage() {
type="button"
onClick={() => handleRemoveStep(index)}
disabled={steps.length <= 1}
className={`mt-3 p-2 rounded-lg transition-colors ${
aria-label="Remove step"
className={`mt-3 p-2 rounded-xl transition-colors ${
steps.length <= 1
? 'text-neutral-300 cursor-not-allowed'
: 'text-neutral-400 hover:text-red-500 hover:bg-red-50 dark:hover:bg-red-900/20'
@@ -197,14 +198,14 @@ export default function CreateFunnelPage() {
<div className="flex justify-end gap-4">
<Link
href={`/sites/${siteId}/funnels`}
className="px-4 py-2 text-neutral-600 dark:text-neutral-400 hover:text-neutral-900 dark:hover:text-white font-medium"
className="btn-secondary"
>
Cancel
</Link>
<Button
type="submit"
disabled={saving}
className="bg-brand-orange hover:bg-brand-orange/90 text-white"
className="btn-primary"
>
{saving ? 'Creating...' : 'Create Funnel'}
</Button>

View File

@@ -55,7 +55,7 @@ export default function FunnelsPage() {
<div className="flex items-center gap-4 mb-6">
<Link
href={`/sites/${siteId}`}
className="p-2 -ml-2 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white rounded-lg hover:bg-neutral-100 dark:hover:bg-neutral-800 transition-colors"
className="p-2 -ml-2 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-white rounded-xl hover:bg-neutral-100 dark:hover:bg-neutral-800 transition-colors"
>
<ChevronLeftIcon className="w-5 h-5" />
</Link>
@@ -70,7 +70,7 @@ export default function FunnelsPage() {
<div className="ml-auto">
<Link
href={`/sites/${siteId}/funnels/new`}
className="flex items-center gap-2 px-4 py-2 bg-brand-orange text-white rounded-lg hover:bg-brand-orange/90 transition-colors font-medium"
className="btn-primary inline-flex items-center gap-2"
>
<PlusIcon className="w-4 h-4" />
<span>Create Funnel</span>
@@ -83,7 +83,7 @@ export default function FunnelsPage() {
<div className="w-16 h-16 mx-auto mb-4 bg-neutral-100 dark:bg-neutral-800 rounded-full flex items-center justify-center">
<ArrowRightIcon className="w-8 h-8 text-neutral-400" />
</div>
<h3 className="text-lg font-medium text-neutral-900 dark:text-white mb-2">
<h3 className="text-lg font-semibold text-neutral-900 dark:text-white mb-2">
No funnels yet
</h3>
<p className="text-neutral-600 dark:text-neutral-400 mb-6 max-w-md mx-auto">
@@ -91,7 +91,7 @@ export default function FunnelsPage() {
</p>
<Link
href={`/sites/${siteId}/funnels/new`}
className="inline-flex items-center gap-2 px-4 py-2 bg-brand-orange text-white rounded-lg hover:bg-brand-orange/90 transition-colors font-medium"
className="btn-primary inline-flex items-center gap-2"
>
<PlusIcon className="w-4 h-4" />
<span>Create Funnel</span>
@@ -132,7 +132,8 @@ export default function FunnelsPage() {
<div className="flex items-center gap-4">
<button
onClick={(e) => handleDelete(e, funnel.id)}
className="p-2 text-neutral-400 hover:text-red-500 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-lg transition-colors"
className="p-2 text-neutral-400 hover:text-red-500 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-xl transition-colors"
aria-label="Delete funnel"
>
<TrashIcon className="w-5 h-5" />
</button>