diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 9a296bec6..88131204e 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -1426,12 +1426,11 @@ describe('Dashboard Module', () => { // Import the public helpers from dashboard. The module is already // loaded above via the jest.mock chain, so we can import directly. // eslint-disable-next-line @typescript-eslint/no-var-requires - const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs, darkenHexColor, parseHexColor } = require('../dashboard') as { + const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs, SAVINGS_LAYER_COLORS } = require('../dashboard') as { renderSavingsByService: (recs: unknown[], byService?: Record, filterDesc?: string) => void; computeServiceStats: (dataPoints: unknown[]) => Map; computeServiceStatsFromRecs: (recs: unknown[]) => Map; - darkenHexColor: (hex: string, factor?: number) => string; - parseHexColor: (hex: string) => { r: number; g: number; b: number }; + SAVINGS_LAYER_COLORS: { current: string; lowestOption: string; upside: string }; }; function buildDOM(): void { @@ -1703,98 +1702,56 @@ describe('Dashboard Module', () => { expect(totalLine).toMatch(/\$400/); }); - describe('stable per-service colours and matching legend (#700)', () => { + describe('one colour per layer, matching the legend (#189)', () => { type ChartCfg = { - data: { labels: string[]; datasets: { label: string; backgroundColor: string[] }[] }; - options: { plugins: { legend: { labels: { generateLabels?: (c: unknown) => { text: string; fillStyle: string }[] } } } }; + data: { labels: string[]; datasets: { label: string; backgroundColor: unknown }[] }; + options: { + plugins: { legend: { labels: { generateLabels?: unknown } } }; + scales: { x: { title?: { display?: boolean } } }; + }; }; const lastCfg = (): ChartCfg => { const calls = (Chart as unknown as jest.Mock).mock.calls; return calls[calls.length - 1]?.[1] as ChartCfg; }; - const colourOf = (cfg: ChartCfg, svc: string): string => { - const ds = cfg.data.datasets.find((d) => d.label === 'Lowest option'); - return ds?.backgroundColor[cfg.data.labels.indexOf(svc)] ?? ''; - }; - test('a service keeps its colour when its rank changes between renders', () => { + test('each layer is one colour on every bar, so the default legend swatch matches it', () => { buildDOM(); - renderSavingsByService([rec('ec2', 500), rec('rds', 300), rec('elasticache', 100)]); - const before = lastCfg(); - expect(before.data.labels[0]).toBe('ec2'); - const ec2Before = colourOf(before, 'ec2'); - const rdsBefore = colourOf(before, 'rds'); - const cacheBefore = colourOf(before, 'elasticache'); - - // Data refresh: rds overtakes ec2, elasticache jumps to rank 2. - renderSavingsByService([rec('ec2', 120), rec('rds', 900), rec('elasticache', 400)]); - const after = lastCfg(); - expect(after.data.labels[0]).toBe('rds'); - expect(colourOf(after, 'ec2')).toBe(ec2Before); - expect(colourOf(after, 'rds')).toBe(rdsBefore); - expect(colourOf(after, 'elasticache')).toBe(cacheBefore); - expect(new Set([ec2Before, rdsBefore, cacheBefore]).size).toBe(3); - }); - - test('every known service slug has a distinct colour per family, twins share one', () => { - const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; - const families: string[][] = [ - ['compute', 'ec2', 'vm'], - ['relational-db', 'rds', 'sql', 'cloudsql'], - ['cache', 'elasticache', 'redis', 'memorystore'], - ['savingsplans', 'savings-plans', 'savings-plans-compute', 'savings-plans-ec2instance', 'savings-plans-sagemaker', 'savings-plans-database'], - ['search', 'opensearch'], - ['data-warehouse', 'redshift'], - ['memorydb'], - ]; - const colours = families.map((slugs) => { - const set = new Set(slugs.map((x) => serviceBarColor(x))); - expect([...set]).toHaveLength(1); - return [...set][0]; + renderSavingsByService( + [rec('ec2', 100), rec('ec2', 500, 3), rec('rds', 300), rec('elasticache', 80)], + { ec2: { potential_savings: 500, current_savings: 50 } }, + ); + const cfg = lastCfg(); + const colours = Object.fromEntries(cfg.data.datasets.map((d) => [d.label, d.backgroundColor])); + expect(colours).toEqual({ + 'Current / Committed': SAVINGS_LAYER_COLORS.current, + 'Lowest option': SAVINGS_LAYER_COLORS.lowestOption, + Upside: SAVINGS_LAYER_COLORS.upside, }); - expect(new Set(colours).size).toBe(families.length); + // A single string (not a per-bar array) is what Chart.js's default + // legend draws, so swatch and bars cannot diverge. + for (const d of cfg.data.datasets) expect(typeof d.backgroundColor).toBe('string'); + expect(cfg.options.plugins.legend.labels.generateLabels).toBeUndefined(); }); - test('unknown slugs never reuse a known family colour', () => { - const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; - // Fallback colours must not reuse a known family's colour. - const known = new Set(['compute', 'relational-db', 'cache', 'savingsplans', 'search', 'data-warehouse', 'memorydb'].map((x) => serviceBarColor(x))); - for (const unknown of ['alpha', 'zeta-service', 'nosuch', 'x1', 'foo-bar']) { - expect(known.has(serviceBarColor(unknown))).toBe(false); - } + test('the three layers use three distinct colours', () => { + expect(new Set(Object.values(SAVINGS_LAYER_COLORS)).size).toBe(3); }); - test('an unknown slug gets a deterministic colour from the name hash', () => { - const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; - expect(serviceBarColor('mystery-service')).toBe(serviceBarColor('mystery-service')); - expect(serviceBarColor('Mystery-Service')).toBe(serviceBarColor('mystery-service')); - // The hash must actually vary: a fixed slot for every unknown slug is a bug. - const seen = new Set(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].map((x) => serviceBarColor(`svc-${x}`))); - expect(seen.size).toBeGreaterThan(1); - }); - - test('legend swatches carry a border so the pale Upside swatch stays visible', () => { + test('colours do not change when a service changes rank', () => { buildDOM(); - renderSavingsByService([rec('ec2', 500)]); - const cfg = lastCfg(); - const items = cfg.options.plugins.legend.labels.generateLabels?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) as unknown as { strokeStyle: string; lineWidth: number }[]; - for (const item of items) { - expect(item.lineWidth).toBeGreaterThanOrEqual(1); - expect(item.strokeStyle).toBe('#546e7a'); - } + renderSavingsByService([rec('ec2', 500), rec('rds', 300)]); + const before = lastCfg().data.datasets.map((d) => d.backgroundColor); + renderSavingsByService([rec('ec2', 120), rec('rds', 900)]); + const after = lastCfg(); + expect(after.data.labels[0]).toBe('rds'); + expect(after.data.datasets.map((d) => d.backgroundColor)).toEqual(before); }); - test('legend swatches use neutral tones matching the three bar layers, not the first bar colour', () => { + test('drops the redundant "Service" x-axis title', () => { buildDOM(); - renderSavingsByService([rec('ec2', 500), rec('rds', 300)]); - const cfg = lastCfg(); - const gen = cfg.options.plugins.legend.labels.generateLabels; - expect(typeof gen).toBe('function'); - const items = gen?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) ?? []; - expect(items.map((i) => i.text)).toEqual(['Current / Committed', 'Lowest option', 'Upside']); - const barColours = cfg.data.datasets.flatMap((d) => d.backgroundColor); - for (const item of items) expect(barColours).not.toContain(item.fillStyle); - expect(new Set(items.map((i) => i.fillStyle)).size).toBe(3); + renderSavingsByService([rec('ec2', 500)]); + expect(lastCfg().options.scales.x.title?.display).not.toBe(true); }); }); @@ -1851,26 +1808,6 @@ describe('Dashboard Module', () => { expect(lowestDs?.data[0]).toBe(0); }); - test('current underlay uses a DARKER variant of each service base hue', () => { - buildDOM(); - renderSavingsByService( - [rec('ec2', 500)], - { ec2: { potential_savings: 500, current_savings: 200 } }, - ); - 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; backgroundColor: string[] }[] }; - }).data.datasets; - const lowestOptionColor = datasets.find((d) => d.label === 'Lowest option')?.backgroundColor[0] as string; - const currentColor = datasets.find((d) => d.label === 'Current / Committed')?.backgroundColor[0] as string; - // The current colour is the darkened form of the base (lowest-option) colour. - expect(currentColor).toBe(darkenHexColor(lowestOptionColor)); - // And it is genuinely darker: each channel sum is lower. - const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; }; - expect(sum(currentColor)).toBeLessThan(sum(lowestOptionColor)); - }); - test('current layer defaults to 0 for a service absent from byService', () => { buildDOM(); renderSavingsByService([rec('rds', 300)], {}); // no rds entry @@ -2087,36 +2024,6 @@ describe('Dashboard Module', () => { expect(lastChartData().labels[0]).toBe('rds'); }); }); - - // Issue #908: colour-derivation helpers for the current-savings underlay. - describe('colour helpers (issue #908)', () => { - test('parseHexColor parses #rrggbb', () => { - expect(parseHexColor('#1a73e8')).toEqual({ r: 26, g: 115, b: 232 }); - }); - - test('parseHexColor tolerates a missing leading hash', () => { - expect(parseHexColor('34a853')).toEqual({ r: 52, g: 168, b: 83 }); - }); - - test('parseHexColor falls back to the default blue for malformed input', () => { - expect(parseHexColor('not-a-color')).toEqual({ r: 26, g: 115, b: 232 }); - }); - - test('darkenHexColor returns a strictly darker same-hue colour', () => { - const base = '#34a853'; - const darker = darkenHexColor(base); - expect(darker).toMatch(/^#[0-9a-f]{6}$/); - const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; }; - expect(sum(darker)).toBeLessThan(sum(base)); - // 30% reduction by default (factor 0.7): green channel 168 -> ~118. - expect(parseHexColor(darker).g).toBe(Math.round(168 * 0.7)); - }); - - test('darkenHexColor honours an explicit factor', () => { - // factor 0.5 halves each channel. - expect(darkenHexColor('#646464', 0.5)).toBe('#323232'); - }); - }); }); // H-3 regression: absent API coverage/count fields must render '--', not '$0'/'80%'/'0'. diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 2a8e61716..dd3cc561f 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -31,88 +31,17 @@ let savingsByServiceChart: Chart | null = null; // Maximum number of services to show in the bar chart before truncating. const SAVINGS_BY_SERVICE_MAX = 10; -// Palette for per-service bars. A service's colour is fixed by its name (see -// serviceBarColor), never by its rank, so it survives data refreshes (#700). -const SERVICE_BAR_COLORS = [ - '#1a73e8', // blue - '#34a853', // green - '#fbbc04', // yellow - '#ea4335', // red - '#9c27b0', // purple - '#00bcd4', // cyan - '#ff5722', // deep-orange - '#607d8b', // blue-grey - '#795548', // brown - '#e91e63', // pink -]; - -// One palette slot per service family, keyed on the slugs the app emits -// (purchase mapServiceSlug, plan-service-ids.ts). A canonical slug and its -// legacy twin share a slot, so the same service is one colour whichever -// spelling a provider returns. Slots 0-6 are reserved for these families. -const KNOWN_SERVICE_COLOR_INDEX: Readonly> = { - compute: 0, ec2: 0, vm: 0, - 'relational-db': 1, rds: 1, sql: 1, cloudsql: 1, - cache: 2, elasticache: 2, redis: 2, memorystore: 2, - savingsplans: 3, 'savings-plans': 3, 'savings-plans-compute': 3, - 'savings-plans-ec2instance': 3, 'savings-plans-sagemaker': 3, 'savings-plans-database': 3, - search: 4, opensearch: 4, - 'data-warehouse': 5, redshift: 5, - memorydb: 6, -}; -const KNOWN_SERVICE_SLOTS = 7; - -/** Stable colour for a service name: identical on every render, regardless of rank. */ -export function serviceBarColor(service: string): string { - const key = service.toLowerCase(); - let idx = KNOWN_SERVICE_COLOR_INDEX[key]; - if (idx === undefined) { - // Unknown slugs hash into the spare slots; two unknown slugs may collide. - let h = 0; - for (let i = 0; i < key.length; i++) h = (h * 31 + key.charCodeAt(i)) >>> 0; - idx = KNOWN_SERVICE_SLOTS + (h % (SERVICE_BAR_COLORS.length - KNOWN_SERVICE_SLOTS)); - } - return SERVICE_BAR_COLORS[idx] ?? '#1a73e8'; -} - -// The legend names the three stacked layers, which every service shares, so -// its swatches are neutral tones that mirror the bar shading (dark / solid / -// 35% alpha) instead of the first bar's service colour. -const LEGEND_SWATCH_COLORS = ['#37474f', '#78909c', 'rgba(120,144,156,0.35)']; -// Border keeps the pale Upside swatch at >= 3:1 against the white card. -const LEGEND_SWATCH_BORDER = '#546e7a'; - -/** - * Parse a #rrggbb hex string into its r/g/b components. Falls back to the - * default service bar blue (#1a73e8) for malformed input so the chart never - * renders a NaN colour. Exported for unit testing. - */ -export function parseHexColor(hex: string): { r: number; g: number; b: number } { - const m = /^#?([0-9a-fA-F]{6})$/.exec(hex.trim()); - const clean = m ? m[1]! : '1a73e8'; - return { - r: parseInt(clean.substring(0, 2), 16), - g: parseInt(clean.substring(2, 4), 16), - b: parseInt(clean.substring(4, 6), 16), - }; -} - -/** - * Derive a darker shade of a base #rrggbb colour by reducing its perceived - * lightness. Used for the current-savings underlay so it reads as the - * "already-realized" portion beneath the lighter potential-range bar of the - * same hue (issue #908). The factor (default 0.7 → ~30% darker) is applied - * multiplicatively per channel so the hue is preserved; the result is - * programmatic, not hardcoded per service. Exported for unit testing. - */ -export function darkenHexColor(hex: string, factor = 0.7): string { - const { r, g, b } = parseHexColor(hex); - const scale = (c: number): string => { - const v = Math.max(0, Math.min(255, Math.round(c * factor))); - return v.toString(16).padStart(2, '0'); - }; - return `#${scale(r)}${scale(g)}${scale(b)}`; -} +// One colour per stacked layer, the same on every bar, so the legend swatches +// are exactly the colours drawn (#189). The x axis already names each service, +// so hue encodes the layer, not the service: no colour can change with rank +// (#700) and the layers differ by hue rather than by shades of one (#744). +// Slots 1-3 of the validated categorical palette, CVD-distinct in every pair. +// Upside (aqua) sits below 3:1 on white; the tooltip carries its value. +export const SAVINGS_LAYER_COLORS = { + current: '#2a78d6', // blue + lowestOption: '#eb6834', // orange + upside: '#1baf7a', // aqua +} as const; // In-memory index of the currently-rendered upcoming purchases, keyed by // execution_id. The "View Details" affordance renders from this — the @@ -985,21 +914,6 @@ export function renderSavingsByService( const totalSavings = positive.reduce((acc, e) => acc + e.visibleTotal, 0); - // Colour: one stable base colour per service name, then derive lighter/darker shades. - const bgColors = visible.map(e => serviceBarColor(e.svc)); - - // Current / Committed: darkest shade (30% darker than base). - const currentColors = bgColors.map(c => darkenHexColor(c)); - - // Lowest option: the base (solid) colour. - const lowestOptionColors = bgColors; - - // Upside: same hue at 35% opacity for the variability band. - const upsideColors = bgColors.map(c => { - const { r, g, b } = parseHexColor(c); - return `rgba(${r},${g},${b},0.35)`; - }); - savingsByServiceChart = new Chart(canvas, { type: 'bar', data: { @@ -1011,21 +925,21 @@ export function renderSavingsByService( { label: 'Current / Committed', data: currentData, - backgroundColor: currentColors, + backgroundColor: SAVINGS_LAYER_COLORS.current, borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 4, bottomRight: 4 }, stack: 'savings', }, { label: 'Lowest option', data: lowestOptionData, - backgroundColor: lowestOptionColors, + backgroundColor: SAVINGS_LAYER_COLORS.lowestOption, borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 0, bottomRight: 0 }, stack: 'savings', }, { label: 'Upside', data: upsideData, - backgroundColor: upsideColors, + backgroundColor: SAVINGS_LAYER_COLORS.upside, borderRadius: { topLeft: 4, topRight: 4, bottomLeft: 0, bottomRight: 0 }, stack: 'savings', }, @@ -1038,17 +952,8 @@ export function renderSavingsByService( legend: { display: true, position: 'top', - labels: { - boxWidth: 12, - generateLabels: (chart) => chart.data.datasets.map((ds, i) => ({ - text: ds.label ?? '', - fillStyle: LEGEND_SWATCH_COLORS[i] ?? LEGEND_SWATCH_COLORS[1] ?? '#78909c', - strokeStyle: LEGEND_SWATCH_BORDER, - lineWidth: 1, - hidden: !chart.isDatasetVisible(i), - datasetIndex: i, - })), - }, + // Default swatches: each one is its dataset's single bar colour. + labels: { boxWidth: 12 }, }, tooltip: { callbacks: { @@ -1088,7 +993,7 @@ export function renderSavingsByService( scales: { x: { stacked: true, - title: { display: true, text: 'Service' }, + // No axis title: the tick labels are the service names (#189). // Keep every service label on narrow screens (rotate instead of skipping). ticks: { autoSkip: false, maxRotation: 60 }, }, diff --git a/frontend/src/index.html b/frontend/src/index.html index ebbbdd24c..6ddf9ca76 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -115,7 +115,7 @@

Savings over time

Potential savings range per service

- +