From 4604f3382f1aa3de779f43bb9592e37cc5ee33c6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 2 Jun 2026 20:49:01 +0200 Subject: [PATCH] feat(home): 2x height + stack Current under Potential on savings-by-service chart - Add #savings-by-service-chart CSS rule (600/560 px) scoped to that canvas only; the generic .chart-section canvas rule (300/280 px) is untouched so other charts are unaffected. - Merge all three datasets into a single "savings" stack so Current (committed) renders visually below the potential range (bottom-to-top = dataset order in Chart.js stacked bars). - Update tooltip discriminator from stack === 'current' to label match since all datasets now share the same stack name. - Extend dashboard tests: assert stacked axes, single-stack invariant, and dataset order (Current first, then Min potential, then Upside). --- frontend/src/__tests__/dashboard.test.ts | 44 +++++++++++++++++++++++- frontend/src/dashboard.ts | 10 ++++++ frontend/src/styles/charts.css | 8 +++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 87b6524be..361ccb7d6 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -1461,7 +1461,8 @@ describe('Dashboard Module', () => { const currentDs = datasets.find((d) => d.label === 'Current / Committed'); expect(currentDs).toBeDefined(); expect(currentDs?.data[0]).toBe(250); - // Current layer is in the unified savings stack. + // All three datasets share one stack so Current renders below the + // potential range (dataset order = bottom-to-top in Chart.js stacked bars). expect(currentDs?.stack).toBe('savings'); }); @@ -1513,6 +1514,47 @@ describe('Dashboard Module', () => { expect(currentDs?.data[0]).toBe(0); }); + // Issue #769 follow-up (2x height + stacking): all three datasets must + // share one stack so Current renders visually BELOW the potential range. + test('all datasets share one stack so Current renders under Potential', () => { + buildDOM(); + renderSavingsByService( + [rec('ec2', 100), rec('ec2', 400)], + { ec2: { potential_savings: 400, current_savings: 150 } }, + ); + const chartCtor = Chart as unknown as jest.Mock; + const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; + const config = lastCall?.[1] as { + data: { datasets: { label: string; stack: string }[] }; + options: { scales: { x: { stacked: boolean }; y: { stacked: boolean } } }; + }; + const datasets = config.data.datasets; + // Every dataset must share the same stack name. + const stacks = new Set(datasets.map((d) => d.stack)); + expect(stacks.size).toBe(1); + // Both axes must have stacked: true. + expect(config.options.scales.x.stacked).toBe(true); + expect(config.options.scales.y.stacked).toBe(true); + }); + + test('dataset order: Current / Committed is first so it renders at the base', () => { + buildDOM(); + renderSavingsByService( + [rec('ec2', 200)], + { ec2: { potential_savings: 200, current_savings: 80 } }, + ); + const chartCtor = Chart as unknown as jest.Mock; + const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; + const datasets = (lastCall?.[1] as { + data: { datasets: { label: string }[] }; + }).data.datasets; + // Use find to avoid direct index access (TS strict mode). + const labels = datasets.map((d) => d.label); + expect(labels[0]).toBe('Current / Committed'); + expect(labels[1]).toBe('Lowest option'); + expect(labels[2]).toBe('Upside'); + }); + test('service present in byService but absent from recs renders Current-only bar', () => { buildDOM(); // No recs for 'lambda'; only byService entry -> single Current band, no Lowest/Upside. diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index f7fc26d66..69fa07a5a 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -852,6 +852,9 @@ export function renderSavingsByService( data: { labels, datasets: [ + // Current sits at the base of the stack so already-realized savings + // are visually "under" the potential range -- dataset order in Chart.js + // stacked bars determines bottom-to-top rendering. { label: 'Current / Committed', data: currentData, @@ -889,6 +892,13 @@ export function renderSavingsByService( label: (ctx) => { const svc = ctx.label ?? ''; const s = stats.get(svc); + if (!s) return ''; + // The current-savings dataset gets a focused, single-line + // tooltip; the range datasets share the full breakdown. + if (ctx.dataset?.label === 'Current / Committed') { + const current = byService[svc]?.current_savings ?? 0; + return `Current / Committed: $${current.toLocaleString()}`; + } const current = byService[svc]?.current_savings ?? 0; const maxRec = s?.max ?? 0; const minRec = s?.min ?? 0; diff --git a/frontend/src/styles/charts.css b/frontend/src/styles/charts.css index cd681218a..acd465c82 100644 --- a/frontend/src/styles/charts.css +++ b/frontend/src/styles/charts.css @@ -13,6 +13,14 @@ min-height: 280px; } +/* Savings-by-service chart needs more vertical space to show multiple + * stacked bars legibly. Scoped to this canvas only so other charts are + * unaffected (issue #769 follow-up). */ +#savings-by-service-chart { + max-height: 600px; + min-height: 560px; +} + /* Empty state for chart sections stays centred and occupies roughly * the same vertical space as the canvas so the widget doesn't collapse * when Chart.js is destroyed during provider-filter transitions