apps/stats/src/views/Stats/Growth/components/growth-kpis.tsx (3)

510-514: Net change calculation logic is correct.

The calculation correctly accounts for cancelled being stored as negative, and the formatting with "+" prefix for positive values is appropriate.

One minor optimization: the net change is calculated on every formatter invocation (for both "new" and "cancelled" series), but only displayed when index === 1. Consider hoisting this calculation outside the formatter return or memoizing it to avoid redundant computation.

♻️ Optional refactor to reduce redundant calculations

 formatter={(value, name, payload, index) => {
     const rawValue = Number(value);
     let displayValue = '0';
     if (rawValue === 0) {
         displayValue = '0';
     } else {
         displayValue = rawValue < 0 ? formatNumber(rawValue * -1) : formatNumber(rawValue);
     }

-    // Calculate net change (new + cancelled, since cancelled is negative)
-    const newValue = Number(payload?.payload?.new || 0);
-    const cancelledValue = Number(payload?.payload?.cancelled || 0);
-    const netChange = newValue + cancelledValue;
-    const netChangeFormatted = netChange === 0 ? '0' : (netChange > 0 ? `+${formatNumber(netChange)}` : formatNumber(netChange));
-
     return (
         <div className='flex w-full flex-col'>
             {index === 0 &&
-                <div className="mb-1 text-sm font-medium text-foreground">
-                    {payload?.payload?.date}
-                </div>
+                <>
+                    <div className="mb-1 text-sm font-medium text-foreground">
+                        {payload?.payload?.date}
+                    </div>
+                </>
             }
             <div className='flex w-full items-center justify-between gap-4'>
                 <div className='flex items-center gap-1'>
                     <div
                         className="size-2 shrink-0 rounded-full bg-[var(--color-bg)] opacity-50"
                         style={{
                             '--color-bg': `var(--color-${name})`
                         } as React.CSSProperties}
                     />
                     <span className='text-sm text-muted-foreground'>
                         {paidChangeChartConfig[name as keyof typeof paidChangeChartConfig]?.label || name}
                     </span>
                 </div>
                 <div className="ml-auto flex items-baseline gap-0.5 font-mono font-medium tabular-nums text-foreground">
                     {displayValue}
                 </div>
             </div>
             {index === 1 &&
+                (() => {
+                    // Calculate net change only once when displaying
+                    const newValue = Number(payload?.payload?.new || 0);
+                    const cancelledValue = Number(payload?.payload?.cancelled || 0);
+                    const netChange = newValue + cancelledValue;
+                    const netChangeFormatted = netChange === 0 ? '0' : (netChange > 0 ? `+${formatNumber(netChange)}` : formatNumber(netChange));
+                    return (
-                <div className='mt-1 flex w-full items-center justify-between gap-4 border-t pt-1'>
-                    <span className='text-sm text-muted-foreground'>
-                        Net change
-                    </span>
-                    <div className="ml-auto flex items-baseline gap-0.5 font-mono font-medium tabular-nums text-foreground">
-                        {netChangeFormatted}
-                    </div>
-                </div>
+                        <div className='mt-1 flex w-full items-center justify-between gap-4 border-t pt-1'>
+                            <span className='text-sm text-muted-foreground'>
+                                Net change
+                            </span>
+                            <div className="ml-auto flex items-baseline gap-0.5 font-mono font-medium tabular-nums text-foreground">
+                                {netChangeFormatted}
+                            </div>
+                        </div>
+                    );
+                })()
             }
         </div>
     );
 }}

---

539-548: Net change display is well-implemented.

The UI presentation with the border separator and consistent styling is clean. The condition index === 1 works correctly given the fixed chart configuration with two series.

For improved maintainability, consider using a more explicit condition like checking if it's the last item in the series or comparing against the series name, rather than relying on the hardcoded index.

♻️ Optional refactor for more robust display condition

-{index === 1 &&
+{name === 'cancelled' &&
     <div className='mt-1 flex w-full items-center justify-between gap-4 border-t pt-1'>
         <span className='text-sm text-muted-foreground'>
             Net change
         </span>
         <div className="ml-auto flex items-baseline gap-0.5 font-mono font-medium tabular-nums text-foreground">
             {netChangeFormatted}
         </div>
     </div>
 }

This approach is more explicit and won't break if the series order changes.

---

510-548: Consider adding test coverage for the net change feature.

The PR description notes that automated tests have not yet been added. Given that this feature affects user-facing tooltips and involves calculation logic, test coverage would help ensure correctness across different scenarios (positive net change, negative net change, zero net change).

Would you like me to help generate test cases for this feature, or open an issue to track adding test coverage?

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro