Render the Cashflow Sankey with recharts (responsive + expense drill-down) (#670)
## What & why The Cashflow "Money Flow" diagram was a bespoke SVG Sankey that laid out nodes and links by hand. It didn't hold up on small screens (fixed 400px min-width, cramped flows) and was ~950 lines of custom geometry to maintain. This replaces it with recharts' `<Sankey>` (already a dependency) inside a `ResponsiveContainer`, letting recharts own the layout. Net **−950 / +515** lines across the component. ## What's kept - Income → **Net** hub → expense flow. - Small-category **"Other"** grouping (`groupSmallCategories`). - Privacy-mode masking on every amount (hub + categories + subcategories). - Click-through to a category's transactions. - Category colors, resolved via `categoryBarColor` (same as the breakdown cards). ## What's new - **Drill-down (expense side):** click an expense category with children to expand its subcategories into their own column; recharts re-lays-out around it. One open at a time, a chevron marks expandable parents, and the expanded parent's label moves onto its bar. Subcategories and childless categories still click through to their transactions. Data comes from the existing `GET /api/cashflow/sankey?...&parent=<id>` endpoint, fetched lazily and reset when the period changes. - **Mobile:** the chart scrolls horizontally with a comfortable min-width (same pattern as the Trend chart) instead of crushing the flows; canvas height grows with the busiest column so labels don't collide. - `align="left"` so an expanded parent's subcategories line up beside it instead of collapsing into the last column and crossing every flow. ## What's dropped (intentional) - The "Other" popover breakdown — the per-category detail already lives in the breakdown cards below the chart. - Income-side drill-down — with the central hub, expanding an income source would split the income column across two depths (visually broken); income categories rarely have children. Income keeps click-through. ## Review Two parallel reviews (technical + bugs/side-effects) ran over the diff. Applied: - **Category colors:** resolve the `CategoryColor` keyword via `categoryBarColor` instead of using it raw as an SVG `fill` — non-CSS keywords (`slate`, `neutral`, `amber`, `rose`) were rendering **black**. (Pre-existing bug carried over from the old chart.) - Skip top-level categories with `amount <= 0` so they don't become stray unlinked nodes. - Keep an expanding parent's label beside its bar until subcategories load (no flicker); collapse again if the fetch fails. - Cap right-side label width so a parent left of an expanded column can't stretch its label across it. - Memoize the cashflow `period` so the drill-down fetch effect doesn't re-run on unrelated re-renders. Dropped (with reason): re-adding the "Other" popover and income drill-down (intentional, above); wrapping aria-labels in `__()` (would add i18n keys, and matches the prior behavior); the recharts node/link prop casts (version is pinned; params are hand-typed to document the fields used). ## Testing - 6 Vitest unit tests (render, "Other" grouping, empty state, drill-down with a mocked fetch, expand affordance, privacy masking) — green. - `tsc --noEmit` clean for the touched files; ESLint + Prettier clean. > **Note on browser QA:** I did not record an automated demo video. The local DB holds real user financial data (won't log in / record), and creating a synthetic test/demo account is currently blocked by pending Spaces migrations in the local DB (`current_space_id` column missing — unrelated to this change). The chart was verified interactively during development (desktop render, mobile layout, drill-down, and the black-color fix). Happy to record a walkthrough if we seed a safe demo account.
This commit is contained in:
Víctor Falcón
GitHub
parent
cd918523e8
commit
7281b610b5
|
|
@ -1,17 +1,47 @@
|
|||
import { SankeyData } from '@/hooks/use-cashflow-data';
|
||||
import { Category } from '@/types/category';
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import * as React from 'react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { SankeyChart } from './sankey-chart';
|
||||
|
||||
// ResponsiveContainer measures its DOM parent, which is 0x0 in jsdom, so the
|
||||
// chart never lays out. Clone the child chart with a fixed size instead —
|
||||
// Sankey reports whatever width/height it receives as props.
|
||||
vi.mock('recharts', async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as typeof import('recharts');
|
||||
return {
|
||||
...actual,
|
||||
ResponsiveContainer: ({
|
||||
children,
|
||||
}: {
|
||||
children: React.ReactElement<{ width?: number; height?: number }>;
|
||||
}) => React.cloneElement(children, { width: 800, height: 400 }),
|
||||
};
|
||||
});
|
||||
|
||||
const privacyMode = {
|
||||
isPrivacyModeEnabled: false,
|
||||
togglePrivacyMode: vi.fn(),
|
||||
setPrivacyMode: vi.fn(),
|
||||
};
|
||||
|
||||
vi.mock('@/contexts/privacy-mode-context', () => ({
|
||||
usePrivacyMode: () => ({ isPrivacyModeEnabled: false }),
|
||||
usePrivacyMode: vi.fn(() => privacyMode),
|
||||
}));
|
||||
|
||||
vi.mock('@/hooks/use-locale', () => ({
|
||||
useLocale: () => 'en',
|
||||
}));
|
||||
|
||||
vi.mock('@/hooks/use-chart-color-scheme', () => ({
|
||||
useChartColors: () => ({
|
||||
cashflowIncomeColor: '#22c55e',
|
||||
cashflowExpenseColor: '#ef4444',
|
||||
categoryBarColor: (color: string) => color || '#999',
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('@inertiajs/react', () => ({
|
||||
router: { visit: vi.fn() },
|
||||
}));
|
||||
|
|
@ -20,6 +50,8 @@ vi.mock('@/actions/App/Http/Controllers/TransactionController', () => ({
|
|||
index: () => ({ url: '/transactions' }),
|
||||
}));
|
||||
|
||||
import { usePrivacyMode } from '@/contexts/privacy-mode-context';
|
||||
|
||||
function category(id: string, name: string, color = '#ccc'): Category {
|
||||
return { id, name, color } as Category;
|
||||
}
|
||||
|
|
@ -58,8 +90,8 @@ const foodChildren: SankeyData = {
|
|||
amount: 200,
|
||||
},
|
||||
{
|
||||
category: category('other-groceries', 'Other groceries'),
|
||||
category_id: 'other-groceries',
|
||||
category: category('eating-out', 'Eating out'),
|
||||
category_id: 'eating-out',
|
||||
amount: 110,
|
||||
},
|
||||
],
|
||||
|
|
@ -83,119 +115,77 @@ describe('SankeyChart', () => {
|
|||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('renders an expand affordance on parent categories with children', () => {
|
||||
it('renders income, center and expense nodes', () => {
|
||||
render(<SankeyChart data={data} period={period} />);
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Expand Food' }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('expands a category into its subcategories without replacing the chart', async () => {
|
||||
render(<SankeyChart data={data} period={period} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand Food' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Groceries')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
expect(screen.getByText('Other groceries')).toBeInTheDocument();
|
||||
// The original chart is untouched: center + parent nodes remain.
|
||||
expect(screen.getByText('Cashflow')).toBeInTheDocument();
|
||||
expect(screen.getByText('Salary')).toBeInTheDocument();
|
||||
expect(screen.getByText('Net')).toBeInTheDocument();
|
||||
expect(screen.getByText('Food')).toBeInTheDocument();
|
||||
expect(screen.getByText('Rent')).toBeInTheDocument();
|
||||
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
expect.stringContaining('parent=food'),
|
||||
);
|
||||
});
|
||||
|
||||
it('truncates long category names with an ellipsis and keeps the full name as a title', () => {
|
||||
const longName = 'Sports, and sport goods and equipment rentals';
|
||||
it('shows an empty state when there is no cashflow', () => {
|
||||
render(
|
||||
<SankeyChart
|
||||
data={{
|
||||
...data,
|
||||
expense_categories: [
|
||||
{
|
||||
category: category('long', longName),
|
||||
category_id: 'long',
|
||||
amount: 500,
|
||||
},
|
||||
],
|
||||
total_expense: 500,
|
||||
income_categories: [],
|
||||
expense_categories: [],
|
||||
total_income: 0,
|
||||
total_expense: 0,
|
||||
}}
|
||||
period={period}
|
||||
/>,
|
||||
);
|
||||
|
||||
// The full name stays in the DOM (with a title for hover); CSS handles
|
||||
// the visual ellipsis via the `truncate` utility.
|
||||
const label = screen.getByTitle(longName);
|
||||
expect(label).toHaveTextContent(longName);
|
||||
expect(label).toHaveClass('truncate');
|
||||
expect(
|
||||
screen.getByText('No cashflow data for this period'),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('collapses any other expanded category so only one stays open', async () => {
|
||||
const multiParent: SankeyData = {
|
||||
...data,
|
||||
expense_categories: [
|
||||
{
|
||||
category: category('food', 'Food'),
|
||||
category_id: 'food',
|
||||
amount: 310,
|
||||
has_children: true,
|
||||
},
|
||||
{
|
||||
category: category('rent', 'Rent'),
|
||||
category_id: 'rent',
|
||||
amount: 500,
|
||||
has_children: true,
|
||||
},
|
||||
],
|
||||
};
|
||||
it('groups small categories into an "Other" node', () => {
|
||||
const many = Array.from({ length: 8 }, (_, i) => ({
|
||||
category: category(`c${i}`, `Category ${i}`),
|
||||
category_id: `c${i}`,
|
||||
// First three are large, the rest are tiny (<3%).
|
||||
amount: i < 3 ? 300 : 5,
|
||||
}));
|
||||
|
||||
const rentChildren: SankeyData = {
|
||||
income_categories: [],
|
||||
expense_categories: [
|
||||
{
|
||||
category: category('mortgage', 'Mortgage'),
|
||||
category_id: 'mortgage',
|
||||
amount: 500,
|
||||
},
|
||||
],
|
||||
total_income: 0,
|
||||
total_expense: 500,
|
||||
};
|
||||
render(
|
||||
<SankeyChart
|
||||
data={{
|
||||
income_categories: [
|
||||
{
|
||||
category: category('salary', 'Salary'),
|
||||
category_id: 'salary',
|
||||
amount: 925,
|
||||
},
|
||||
],
|
||||
expense_categories: many,
|
||||
total_income: 925,
|
||||
total_expense: 925,
|
||||
}}
|
||||
period={period}
|
||||
/>,
|
||||
);
|
||||
|
||||
global.fetch = vi.fn().mockImplementation((url: string) => {
|
||||
const json = url.includes('parent=rent')
|
||||
? rentChildren
|
||||
: foodChildren;
|
||||
|
||||
return Promise.resolve({ json: async () => json });
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
render(<SankeyChart data={multiParent} period={period} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand Food' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Groceries')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand Rent' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Mortgage')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Food's subcategories are gone now that Rent is open.
|
||||
expect(screen.queryByText('Groceries')).not.toBeInTheDocument();
|
||||
expect(screen.getByText('Other')).toBeInTheDocument();
|
||||
// The tiny categories are folded away, not rendered individually.
|
||||
expect(screen.queryByText('Category 7')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('collapses an expanded category when toggled again', async () => {
|
||||
it('shows an expand affordance on expense categories with children', () => {
|
||||
render(<SankeyChart data={data} period={period} />);
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Expand Food' }),
|
||||
).toBeInTheDocument();
|
||||
// Rent has no children, so it links straight to its transactions.
|
||||
expect(
|
||||
screen.getByRole('link', { name: 'View Rent transactions' }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('expands an expense category into its subcategories on click', async () => {
|
||||
render(<SankeyChart data={data} period={period} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand Food' }));
|
||||
|
|
@ -204,10 +194,25 @@ describe('SankeyChart', () => {
|
|||
expect(screen.getByText('Groceries')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Collapse Food' }));
|
||||
expect(screen.getByText('Eating out')).toBeInTheDocument();
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
expect.stringContaining('parent=food'),
|
||||
);
|
||||
// The rest of the chart stays in place.
|
||||
expect(screen.getByText('Rent')).toBeInTheDocument();
|
||||
expect(screen.getByText('Net')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByText('Groceries')).not.toBeInTheDocument();
|
||||
it('masks amounts in privacy mode', () => {
|
||||
vi.mocked(usePrivacyMode).mockReturnValue({
|
||||
...privacyMode,
|
||||
isPrivacyModeEnabled: true,
|
||||
});
|
||||
|
||||
render(<SankeyChart data={data} period={period} />);
|
||||
|
||||
// No raw digits leak into the labels when privacy mode is on.
|
||||
expect(screen.getByText('Salary')).toBeInTheDocument();
|
||||
expect(document.body.textContent).not.toMatch(/1,?000/);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -74,11 +74,3 @@ export function groupSmallCategories(
|
|||
// Don't group - return all categories as main
|
||||
return { main: sortedCategories, other: null };
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates the percentage of a value relative to a total
|
||||
*/
|
||||
export function calculatePercentage(value: number, total: number): number {
|
||||
if (total === 0) return 0;
|
||||
return (value / total) * 100;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ import {
|
|||
startOfQuarter,
|
||||
startOfYear,
|
||||
} from 'date-fns';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useEffect, useMemo, useState } from 'react';
|
||||
|
||||
const breadcrumbs: BreadcrumbItem[] = [
|
||||
{
|
||||
|
|
@ -125,7 +125,12 @@ export default function CashflowPage() {
|
|||
parsePeriodParam(initialPeriod, initialPeriodType),
|
||||
);
|
||||
|
||||
const period = getPeriodRange(currentDate, periodType);
|
||||
// Memoized so children (e.g. the Sankey drill-down fetch) don't see a new
|
||||
// object identity on every render.
|
||||
const period = useMemo(
|
||||
() => getPeriodRange(currentDate, periodType),
|
||||
[currentDate, periodType],
|
||||
);
|
||||
|
||||
const {
|
||||
summary,
|
||||
|
|
|
|||
Loading…
Reference in New Issue