Skip to content

Commit 8376163

Browse files
committed
fix: improve how previous state is shown for funnels
1 parent 885f722 commit 8376163

5 files changed

Lines changed: 365 additions & 152 deletions

File tree

apps/start/src/components/delta-chip.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { type VariantProps, cva } from 'class-variance-authority';
33
import { ArrowDownIcon, ArrowUpIcon } from 'lucide-react';
44

55
const deltaChipVariants = cva(
6-
'flex items-center gap-1 rounded-full px-2 py-1 text-sm font-semibold',
6+
'flex items-center justify-center gap-1 rounded-full font-semibold',
77
{
88
variants: {
99
variant: {
@@ -12,9 +12,10 @@ const deltaChipVariants = cva(
1212
default: 'bg-muted text-muted-foreground',
1313
},
1414
size: {
15-
sm: 'text-xs',
16-
md: 'text-sm',
17-
lg: 'text-base',
15+
xs: 'px-1.5 py-0 leading-none text-[10px]',
16+
sm: 'px-2 py-1 text-xs',
17+
md: 'px-2 py-1 text-sm',
18+
lg: 'px-2 py-1 text-base',
1819
},
1920
},
2021
defaultVariants: {
@@ -30,6 +31,7 @@ type DeltaChipProps = VariantProps<typeof deltaChipVariants> & {
3031
};
3132

3233
const iconVariants: Record<NonNullable<DeltaChipProps['size']>, number> = {
34+
xs: 8,
3335
sm: 12,
3436
md: 16,
3537
lg: 20,

apps/start/src/components/report-chart/common/previous-diff-indicator.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ interface PreviousDiffIndicatorPureProps {
9797
diff?: number | null | undefined;
9898
state?: string | null | undefined;
9999
inverted?: boolean;
100-
size?: 'sm' | 'lg' | 'md';
100+
size?: 'xs' | 'sm' | 'lg' | 'md';
101101
className?: string;
102102
showPrevious?: boolean;
103103
}

apps/start/src/components/report-chart/funnel/breakdown-list.tsx

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import { useNumber } from '@/hooks/use-numer-formatter';
33
import type { RouterOutputs } from '@/trpc/client';
44
import { cn } from '@/utils/cn';
55
import { getChartColor } from '@/utils/theme';
6+
import { getPreviousMetric } from '@openpanel/common';
67
import { ChevronDown, ChevronRight } from 'lucide-react';
78
import { useState } from 'react';
9+
import { PreviousDiffIndicatorPure } from '../common/previous-diff-indicator';
810
import { Tables } from './chart';
911

1012
interface BreakdownListProps {
@@ -48,10 +50,9 @@ export function BreakdownList({
4850
});
4951
};
5052

51-
// Get the color index for a breakdown based on its position in the
52-
// visible series list (so colors match the chart bars)
53-
const getVisibleIndex = (id: string) => {
54-
return visibleSeriesIds.indexOf(id);
53+
// Get the stable color index for a breakdown (position in full list, matches chart)
54+
const getStableColorIndex = (id: string) => {
55+
return allBreakdowns.findIndex((b) => b.id === id);
5556
};
5657

5758
if (allBreakdowns.length === 0) {
@@ -81,14 +82,12 @@ export function BreakdownList({
8182
{allBreakdowns.map((item, index) => {
8283
const isExpanded = expandedIds.has(item.id);
8384
const isVisible = visibleSeriesIds.includes(item.id);
84-
const visibleIndex = getVisibleIndex(item.id);
85+
const stableColorIndex = getStableColorIndex(item.id);
8586
const previousItem = previousData[index] ?? null;
8687
const hasBreakdownName =
8788
item.breakdowns && item.breakdowns.length > 0;
8889
const color =
89-
isVisible && visibleIndex !== -1
90-
? getChartColor(visibleIndex)
91-
: undefined;
90+
stableColorIndex >= 0 ? getChartColor(stableColorIndex) : undefined;
9291

9392
return (
9493
<div key={item.id} className="col">
@@ -107,7 +106,7 @@ export function BreakdownList({
107106
className="shrink-0"
108107
style={{
109108
borderColor: color,
110-
backgroundColor: isVisible ? color : 'transparent',
109+
backgroundColor: isVisible && color ? color : 'transparent',
111110
}}
112111
/>
113112
)}
@@ -141,6 +140,14 @@ export function BreakdownList({
141140
'%',
142141
)}
143142
</div>
143+
{previousItem && (
144+
<PreviousDiffIndicatorPure
145+
{...getPreviousMetric(
146+
item.lastStep.percent,
147+
previousItem.lastStep.percent,
148+
)}
149+
/>
150+
)}
144151
</div>
145152
<div className="text-right row gap-2 items-center">
146153
<div className="text-muted-foreground text-sm">
@@ -149,6 +156,14 @@ export function BreakdownList({
149156
<div className="font-mono font-semibold text-sm">
150157
{number.format(item.lastStep.count)}
151158
</div>
159+
{previousItem && (
160+
<PreviousDiffIndicatorPure
161+
{...getPreviousMetric(
162+
item.lastStep.count,
163+
previousItem.lastStep.count,
164+
)}
165+
/>
166+
)}
152167
</div>
153168
</div>
154169
</div>
@@ -160,6 +175,7 @@ export function BreakdownList({
160175
current: item,
161176
previous: previousItem,
162177
}}
178+
noTopBorderRadius
163179
/>
164180
)}
165181
</div>

0 commit comments

Comments
 (0)