-
Notifications
You must be signed in to change notification settings - Fork 276
Feature/insights cards #263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,229 @@ | ||
| import { countries } from '@/translations/countries'; | ||
| import type { RouterOutputs } from '@/trpc/client'; | ||
| import { cn } from '@/utils/cn'; | ||
| import type { InsightPayload } from '@openpanel/validation'; | ||
| import { ArrowDown, ArrowUp, FilterIcon, RotateCcwIcon } from 'lucide-react'; | ||
| import { last } from 'ramda'; | ||
| import { useState } from 'react'; | ||
| import { SerieIcon } from '../report-chart/common/serie-icon'; | ||
| import { Badge } from '../ui/badge'; | ||
|
|
||
| function formatWindowKind(windowKind: string): string { | ||
| switch (windowKind) { | ||
| case 'yesterday': | ||
| return 'Yesterday'; | ||
| case 'rolling_7d': | ||
| return '7 Days'; | ||
| case 'rolling_30d': | ||
| return '30 Days'; | ||
| } | ||
| return windowKind; | ||
| } | ||
|
|
||
| interface InsightCardProps { | ||
| insight: RouterOutputs['insight']['list'][number]; | ||
| className?: string; | ||
| onFilter?: () => void; | ||
| } | ||
|
|
||
| export function InsightCard({ | ||
| insight, | ||
| className, | ||
| onFilter, | ||
| }: InsightCardProps) { | ||
| const payload = insight.payload; | ||
| const dimensions = payload?.dimensions; | ||
| const availableMetrics = Object.entries(payload?.metrics ?? {}); | ||
|
|
||
| // Pick what to display: prefer share if available (geo/devices), else primaryMetric | ||
| const [metricIndex, setMetricIndex] = useState( | ||
| availableMetrics.findIndex(([key]) => key === payload?.primaryMetric), | ||
| ); | ||
| const currentMetricKey = availableMetrics[metricIndex][0]; | ||
| const currentMetricEntry = availableMetrics[metricIndex][1]; | ||
|
|
||
| const metricUnit = currentMetricEntry?.unit; | ||
| const currentValue = currentMetricEntry?.current ?? null; | ||
| const compareValue = currentMetricEntry?.compare ?? null; | ||
|
|
||
| const direction = currentMetricEntry?.direction ?? 'flat'; | ||
| const isIncrease = direction === 'up'; | ||
| const isDecrease = direction === 'down'; | ||
|
|
||
| const deltaText = | ||
| metricUnit === 'ratio' | ||
| ? `${Math.abs((currentMetricEntry?.delta ?? 0) * 100).toFixed(1)}pp` | ||
| : `${Math.abs((currentMetricEntry?.changePct ?? 0) * 100).toFixed(1)}%`; | ||
|
|
||
| // Format metric values | ||
| const formatValue = (value: number | null): string => { | ||
| if (value == null) return '-'; | ||
| if (metricUnit === 'ratio') return `${(value * 100).toFixed(1)}%`; | ||
| return Math.round(value).toLocaleString(); | ||
| }; | ||
|
|
||
| // Get the metric label | ||
| const metricKeyToLabel = (key: string) => | ||
| key === 'share' ? 'Share' : key === 'pageviews' ? 'Pageviews' : 'Sessions'; | ||
|
|
||
| const metricLabel = metricKeyToLabel(currentMetricKey); | ||
|
|
||
| const renderTitle = () => { | ||
| if ( | ||
| dimensions[0]?.key === 'country' || | ||
| dimensions[0]?.key === 'referrer_name' || | ||
| dimensions[0]?.key === 'device' | ||
| ) { | ||
| return ( | ||
| <span className="capitalize flex items-center gap-2"> | ||
| <SerieIcon name={dimensions[0]?.value} /> {insight.displayName} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| if (insight.displayName.startsWith('http')) { | ||
| return ( | ||
| <span className="flex items-center gap-2"> | ||
| <SerieIcon | ||
| name={dimensions[0]?.displayName ?? dimensions[0]?.value} | ||
| /> | ||
| <span className="line-clamp-2">{dimensions[1]?.displayName}</span> | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| return insight.displayName; | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn( | ||
| 'card p-4 h-full flex flex-col hover:bg-def-50 transition-colors group/card', | ||
| className, | ||
| )} | ||
| > | ||
| <div | ||
| className={cn( | ||
| 'row justify-between h-4 items-center', | ||
| onFilter && 'group-hover/card:hidden', | ||
| )} | ||
| > | ||
| <Badge variant="outline" className="-ml-2"> | ||
| {formatWindowKind(insight.windowKind)} | ||
| </Badge> | ||
| {/* Severity: subtle dot instead of big pill */} | ||
| {insight.severityBand && ( | ||
| <div className="flex items-center gap-1 shrink-0"> | ||
| <span | ||
| className={cn( | ||
| 'h-2 w-2 rounded-full', | ||
| insight.severityBand === 'severe' | ||
| ? 'bg-red-500' | ||
| : insight.severityBand === 'moderate' | ||
| ? 'bg-yellow-500' | ||
| : 'bg-blue-500', | ||
| )} | ||
| /> | ||
| <span className="text-[11px] text-muted-foreground capitalize"> | ||
| {insight.severityBand} | ||
| </span> | ||
| </div> | ||
| )} | ||
| </div> | ||
| {onFilter && ( | ||
| <div className="row group-hover/card:flex hidden h-4 justify-between gap-2"> | ||
| {availableMetrics.length > 1 ? ( | ||
| <button | ||
| type="button" | ||
| className="text-[11px] text-muted-foreground capitalize flex items-center gap-1" | ||
| onClick={() => | ||
| setMetricIndex((metricIndex + 1) % availableMetrics.length) | ||
| } | ||
| > | ||
| <RotateCcwIcon className="size-2" /> | ||
| Show{' '} | ||
| {metricKeyToLabel( | ||
| availableMetrics[ | ||
| (metricIndex + 1) % availableMetrics.length | ||
| ][0], | ||
| )} | ||
| </button> | ||
|
Comment on lines
+135
to
+150
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Division by zero risk in metric rotation. The metric rotation logic uses 🤖 Prompt for AI Agents |
||
| ) : ( | ||
| <div /> | ||
| )} | ||
| <button | ||
| type="button" | ||
| className="text-[11px] text-muted-foreground capitalize flex items-center gap-1" | ||
| onClick={onFilter} | ||
| > | ||
| Filter <FilterIcon className="size-2" /> | ||
| </button> | ||
| </div> | ||
| )} | ||
| <div className="font-semibold text-sm leading-snug line-clamp-2 mt-2"> | ||
| {renderTitle()} | ||
| </div> | ||
|
|
||
| {/* Metric row */} | ||
| <div className="mt-auto pt-2"> | ||
| <div className="flex items-end justify-between gap-3"> | ||
| <div className="min-w-0"> | ||
| <div className="text-[11px] text-muted-foreground mb-1"> | ||
| {metricLabel} | ||
| </div> | ||
|
|
||
| <div className="col gap-1"> | ||
| <div className="text-2xl font-semibold tracking-tight"> | ||
| {formatValue(currentValue)} | ||
| </div> | ||
|
|
||
| {/* Inline compare, smaller */} | ||
| {compareValue != null && ( | ||
| <div className="text-xs text-muted-foreground"> | ||
| vs {formatValue(compareValue)} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Delta chip */} | ||
| <DeltaChip | ||
| isIncrease={isIncrease} | ||
| isDecrease={isDecrease} | ||
| deltaText={deltaText} | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function DeltaChip({ | ||
| isIncrease, | ||
| isDecrease, | ||
| deltaText, | ||
| }: { | ||
| isIncrease: boolean; | ||
| isDecrease: boolean; | ||
| deltaText: string; | ||
| }) { | ||
| return ( | ||
| <div | ||
| className={cn( | ||
| 'flex items-center gap-1 rounded-full px-2 py-1 text-sm font-semibold', | ||
| isIncrease | ||
| ? 'bg-emerald-500/10 text-emerald-600 dark:text-emerald-400' | ||
| : isDecrease | ||
| ? 'bg-red-500/10 text-red-600 dark:text-red-400' | ||
| : 'bg-muted text-muted-foreground', | ||
| )} | ||
| > | ||
| {isIncrease ? ( | ||
| <ArrowUp size={16} className="shrink-0" /> | ||
| ) : isDecrease ? ( | ||
| <ArrowDown size={16} className="shrink-0" /> | ||
| ) : null} | ||
| <span>{deltaText}</span> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import { useEventQueryFilters } from '@/hooks/use-event-query-filters'; | ||
| import { useTRPC } from '@/integrations/trpc/react'; | ||
| import { useQuery } from '@tanstack/react-query'; | ||
| import { InsightCard } from '../insights/insight-card'; | ||
| import { Skeleton } from '../skeleton'; | ||
| import { | ||
| Carousel, | ||
| CarouselContent, | ||
| CarouselItem, | ||
| CarouselNext, | ||
| CarouselPrevious, | ||
| } from '../ui/carousel'; | ||
|
|
||
| interface OverviewInsightsProps { | ||
| projectId: string; | ||
| } | ||
|
|
||
| export default function OverviewInsights({ projectId }: OverviewInsightsProps) { | ||
| const trpc = useTRPC(); | ||
| const [filters, setFilter] = useEventQueryFilters(); | ||
| const { data: insights, isLoading } = useQuery( | ||
| trpc.insight.list.queryOptions({ | ||
| projectId, | ||
| limit: 20, | ||
| }), | ||
| ); | ||
|
|
||
| if (isLoading) { | ||
| const keys = Array.from({ length: 4 }, (_, i) => `insight-skeleton-${i}`); | ||
| return ( | ||
| <div className="col-span-6"> | ||
| <Carousel opts={{ align: 'start' }} className="w-full"> | ||
| <CarouselContent className="-ml-4"> | ||
| {keys.map((key) => ( | ||
| <CarouselItem | ||
| key={key} | ||
| className="pl-4 basis-full sm:basis-1/2 lg:basis-1/4" | ||
| > | ||
| <Skeleton className="h-36 w-full" /> | ||
| </CarouselItem> | ||
| ))} | ||
| </CarouselContent> | ||
| </Carousel> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (!insights || insights.length === 0) return null; | ||
|
|
||
| return ( | ||
| <div className="col-span-6 -mx-4"> | ||
| <Carousel opts={{ align: 'start' }} className="w-full group"> | ||
| <CarouselContent className="mr-4"> | ||
| {insights.map((insight) => ( | ||
| <CarouselItem | ||
| key={insight.id} | ||
| className="pl-4 basis-full sm:basis-1/2 lg:basis-1/4" | ||
| > | ||
| <InsightCard | ||
| insight={insight} | ||
| onFilter={() => { | ||
| insight.payload.dimensions.forEach((dim) => { | ||
| void setFilter(dim.key, dim.value, 'is'); | ||
| }); | ||
| }} | ||
| /> | ||
| </CarouselItem> | ||
| ))} | ||
| </CarouselContent> | ||
| <CarouselPrevious className="!opacity-0 pointer-events-none transition-opacity group-hover:!opacity-100 group-hover:pointer-events-auto group-focus:opacity-100 group-focus:pointer-events-auto" /> | ||
| <CarouselNext className="!opacity-0 pointer-events-none transition-opacity group-hover:!opacity-100 group-hover:pointer-events-auto group-focus:opacity-100 group-focus:pointer-events-auto" /> | ||
| </Carousel> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add bounds checking for metrics array access.
The code doesn't handle the case where
availableMetricsis empty or whenfindIndexreturns -1. This will cause runtime errors when accessingavailableMetrics[metricIndex]if no metrics are available or the primary metric is not found.🔎 Apply this diff to add proper bounds checking:
const availableMetrics = Object.entries(payload?.metrics ?? {}); + // Guard against empty metrics or missing primary metric + if (availableMetrics.length === 0) { + return null; // or render a fallback UI + } + // Pick what to display: prefer share if available (geo/devices), else primaryMetric const [metricIndex, setMetricIndex] = useState( availableMetrics.findIndex(([key]) => key === payload?.primaryMetric), ); + + // Fallback to first metric if primary not found + const safeMetricIndex = metricIndex === -1 ? 0 : metricIndex; - const currentMetricKey = availableMetrics[metricIndex][0]; - const currentMetricEntry = availableMetrics[metricIndex][1]; + const currentMetricKey = availableMetrics[safeMetricIndex][0]; + const currentMetricEntry = availableMetrics[safeMetricIndex][1];📝 Committable suggestion
🤖 Prompt for AI Agents