fix(frontend): one card and dialog surface, kpi-tile on Opportunities

frontend/src/__tests__/kpi-tile.test.ts

@@ -0,0 +1,65 @@
+/**
+ * Issue #743: lib/kpi-tile builds the one KPI tile used by Home and
+ * Opportunities.
+ */
+import { buildKpiTile, renderKpiTiles } from '../lib/kpi-tile';
+import { NO_VALUE } from '../utils';
+
+describe('buildKpiTile', () => {
+  test('builds the full tile: title, value, detail and hidden spark slot', () => {
+    const tile = buildKpiTile({
+      kpi: 'savings', title: 'Potential Monthly Savings', value: '$300', valueSavings: true,
+      detail: '2 recommendations', spark: true,
+    });
+    expect(tile.className).toBe('card kpi-tile');
+    expect(tile.dataset['kpi']).toBe('savings');
+    expect(tile.querySelector('h3')?.textContent).toBe('Potential Monthly Savings');
+    const value = tile.querySelector('p.value')!;
+    expect(value.className).toBe('value kpi-tile-value savings');
+    expect(value.textContent).toBe('$300');
+    expect(tile.querySelector('p.detail.kpi-tile-detail')?.textContent).toBe('2 recommendations');
+    const spark = tile.querySelector('svg.kpi-tile-spark')!;
+    expect(spark.classList.contains('hidden')).toBe(true);
+    expect(spark.getAttribute('data-spark-key')).toBe('savings');
+    expect(spark.getAttribute('aria-hidden')).toBe('true');
+  });
+
+  test('omits the detail line, savings color and spark slot when not asked for', () => {
+    const tile = buildKpiTile({ kpi: 'count', title: 'Total Recommendations', value: '19' });
+    expect(tile.querySelector('p.value')?.classList.contains('savings')).toBe(false);
+    expect(tile.querySelector('.kpi-tile-detail')).toBeNull();
+    expect(tile.querySelector('svg')).toBeNull();
+    expect(tile.children).toHaveLength(2);
+  });
+
+  test('keeps an absent (NO_VALUE) savings figure neutral, not green (issue #739)', () => {
+    const tile = buildKpiTile({ kpi: 'savings', title: 'Potential Monthly Savings', value: NO_VALUE, valueSavings: true });
+    const value = tile.querySelector('p.value')!;
+    expect(value.textContent).toBe(NO_VALUE);
+    expect(value.classList.contains('savings')).toBe(false);
+  });
+
+  test('an empty-string detail still renders the line', () => {
+    const tile = buildKpiTile({ kpi: 'x', title: 'T', value: 'V', detail: '' });
+    expect(tile.querySelector('.kpi-tile-detail')).not.toBeNull();
+  });
+
+  test('treats title, value and detail as text, never markup', () => {
+    const tile = buildKpiTile({ kpi: 'x', title: '<b>t</b>', value: '<img src=x onerror=alert(1)>', detail: '<i>d</i>' });
+    expect(tile.querySelector('img, b, i')).toBeNull();
+    expect(tile.querySelector('p.value')?.textContent).toBe('<img src=x onerror=alert(1)>');
+  });
+});
+
+describe('renderKpiTiles', () => {
+  test('replaces the container contents with one tile per spec, in order', () => {
+    const container = document.createElement('section');
+    container.innerHTML = '<p class="error">old</p>';
+    renderKpiTiles(container, [
+      { kpi: 'a', title: 'A', value: '1' },
+      { kpi: 'b', title: 'B', value: '2' },
+    ]);
+    expect(container.querySelector('.error')).toBeNull();
+    expect(Array.from(container.children).map((c) => (c as HTMLElement).dataset['kpi'])).toEqual(['a', 'b']);
+  });
+});

frontend/src/__tests__/recommendations-kpi-tiles.test.ts

@@ -0,0 +1,107 @@
+/**
+ * Issue #743: the Opportunities summary renders the same .kpi-tile component
+ * as Home (it used to write plain `.card` innerHTML, so the same kind of KPI
+ * looked different on the two pages).
+ */
+import { loadRecommendations } from '../recommendations';
+import type { LocalRecommendation } from '../types';
+
+jest.mock('../api', () => ({
+  getRecommendations: jest.fn(),
+  refreshRecommendations: jest.fn(),
+  listAccounts: jest.fn().mockResolvedValue([]),
+  listAccountsMinimal: jest.fn().mockResolvedValue([]),
+  getConfig: jest.fn().mockResolvedValue({ global: {} }),
+  listAccountServiceOverrides: jest.fn().mockResolvedValue([]),
+}));
+
+jest.mock('../api/recommendations', () => ({
+  getRecommendationsFreshness: jest.fn().mockResolvedValue({
+    last_collected_at: new Date().toISOString(),
+    last_collection_error: null,
+  }),
+  refreshRecommendations: jest.fn().mockResolvedValue({}),
+}));
+
+jest.mock('../toast', () => ({
+  showToast: jest.fn().mockReturnValue({ dismiss: jest.fn() }),
+}));
+
+const RECS: LocalRecommendation[] = [
+  { id: 'a', provider: 'aws', cloud_account_id: 'acct', service: 'ec2', resource_type: 'm5.large', region: 'us-east-1', count: 1, term: 1, payment: 'all-upfront', savings: 100, upfront_cost: 1200 } as LocalRecommendation,
+  { id: 'b', provider: 'aws', cloud_account_id: 'acct', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment: 'all-upfront', savings: 200, upfront_cost: 2400 } as LocalRecommendation,
+];
+
+jest.mock('../state', () => ({
+  getCurrentProvider: jest.fn().mockReturnValue('all'),
+  setCurrentProvider: jest.fn(),
+  getCurrentAccountIDs: jest.fn().mockReturnValue([]),
+  setCurrentAccountIDs: jest.fn(),
+  getRecommendations: jest.fn().mockReturnValue([]),
+  getRecommendationByID: jest.fn().mockReturnValue(undefined),
+  setRecommendations: jest.fn(),
+  getSelectedRecommendationIDs: jest.fn().mockReturnValue(new Set()),
+  clearSelectedRecommendations: jest.fn(),
+  addSelectedRecommendation: jest.fn(),
+  removeSelectedRecommendation: jest.fn(),
+  getRecommendationsSort: jest.fn().mockReturnValue({ column: 'savings', direction: 'desc' }),
+  setRecommendationsSort: jest.fn(),
+  getRecommendationsColumnFilters: jest.fn().mockReturnValue({}),
+  setRecommendationsColumnFilter: jest.fn(),
+  clearAllRecommendationsColumnFilters: jest.fn(),
+  getVisibleRecommendations: jest.fn().mockReturnValue([]),
+  setVisibleRecommendations: jest.fn(),
+  getCostPeriod: jest.fn().mockReturnValue('monthly'),
+  setCostPeriod: jest.fn(),
+  getHiddenColumns: jest.fn().mockReturnValue(new Set()),
+  setHiddenColumns: jest.fn(),
+  // Administrators group id (see recommendations-enabled-providers.test.ts).
+  getCurrentUser: jest.fn().mockReturnValue({ id: 'u', email: 'u@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
+}));
+
+import * as api from '../api';
+
+function seedDOM(): void {
+  const tab = document.createElement('div');
+  tab.id = 'opportunities-tab';
+  tab.className = 'tab-content active';
+  for (const id of ['recommendations-summary', 'recommendations-list']) {
+    const el = document.createElement('div');
+    el.id = id;
+    tab.appendChild(el);
+  }
+  document.body.replaceChildren(tab);
+}
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  seedDOM();
+  (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: RECS, regions: [] });
+});
+
+test('summary renders four .kpi-tile cards in the Home tile markup', async () => {
+  await loadRecommendations();
+  const tiles = Array.from(document.querySelectorAll<HTMLElement>('#recommendations-summary > .card'));
+  expect(tiles.map((t) => t.dataset['kpi'])).toEqual(['count', 'savings', 'upfront', 'payback']);
+  for (const tile of tiles) {
+    expect(tile.classList.contains('kpi-tile')).toBe(true);
+    expect(tile.querySelector(':scope > h3')).not.toBeNull();
+    expect(tile.querySelector(':scope > p.value.kpi-tile-value')).not.toBeNull();
+  }
+  expect(tiles.map((t) => t.querySelector('h3')!.textContent)).toEqual([
+    'Total Recommendations', 'Potential Monthly Savings', 'Total Upfront Cost', 'Payback Period',
+  ]);
+});
+
+test('only the savings tile uses the savings color, and values are filled in', async () => {
+  await loadRecommendations();
+  const value = (kpi: string) =>
+    document.querySelector<HTMLElement>(`#recommendations-summary [data-kpi="${kpi}"] .kpi-tile-value`)!;
+  expect(value('count').textContent).toBe('2');
+  expect(value('savings').classList.contains('savings')).toBe(true);
+  for (const kpi of ['count', 'upfront', 'payback']) {
+    expect(value(kpi).classList.contains('savings')).toBe(false);
+  }
+  expect(value('savings').textContent).toContain('$');
+  expect(value('upfront').textContent).toContain('$');
+});

frontend/src/__tests__/surface-css.test.ts

@@ -0,0 +1,101 @@
+/**
+ * Issue #743: one dialog surface and one card surface.
+ *
+ * The four modal systems (.modal + .modal-content, .modal-overlay,
+ * confirmDialog's .modal-confirm, the Archera offer panel) each had their
+ * own scrim opacity, radius, shadow and z-index. The card styles differed in
+ * border, shadow and fill. These read the stylesheets (jsdom has no cascade)
+ * and pin every surface to the shared tokens.
+ */
+import fs from 'fs';
+import path from 'path';
+
+const stylesDir = path.join(__dirname, '..', 'styles');
+const all = fs.readdirSync(stylesDir).filter((f) => f.endsWith('.css'))
+  .map((f) => fs.readFileSync(path.join(stylesDir, f), 'utf8').replace(/\/\*[\s\S]*?\*\//g, ''))
+  .join('\n');
+
+/** Declarations of the rule whose whole selector list is `selector`. */
+function rule(selector: string): Record<string, string> {
+  const re = /([^{}]+)\{([^{}]*)\}/g;
+  let m: RegExpExecArray | null;
+  while ((m = re.exec(all)) !== null) {
+    if (m[1]!.trim().replace(/\s+/g, ' ') !== selector) continue;
+    const out: Record<string, string> = {};
+    for (const d of m[2]!.split(';')) {
+      const i = d.indexOf(':');
+      if (i > 0) out[d.slice(0, i).trim()] = d.slice(i + 1).trim();
+    }
+    return out;
+  }
+  throw new Error(`rule not found: ${selector}`);
+}
+
+describe('design tokens', () => {
+  test.each([
+    '--cudly-scrim', '--cudly-shadow-dialog', '--cudly-z-modal', '--cudly-z-modal-raised',
+    '--cudly-z-page-overlay', '--cudly-z-confirm', '--cudly-z-toast',
+  ])('%s is defined', (token) => {
+    expect(all).toMatch(new RegExp(`${token}:\\s*[^;]+;`));
+  });
+
+  test('the stacking ladder keeps confirm above modals and toasts on top', () => {
+    const z = (t: string) => Number(new RegExp(`${t}:\\s*(\\d+)`).exec(all)![1]);
+    expect(z('--cudly-z-modal')).toBeLessThan(z('--cudly-z-modal-raised'));
+    expect(z('--cudly-z-modal-raised')).toBeLessThan(z('--cudly-z-page-overlay'));
+    expect(z('--cudly-z-page-overlay')).toBeLessThan(z('--cudly-z-confirm'));
+    expect(z('--cudly-z-confirm')).toBeLessThan(z('--cudly-z-toast'));
+  });
+});
+
+describe('one dialog surface (issue #743)', () => {
+  test.each(['.modal', '.modal-overlay', '.modal-confirm-backdrop', '.archera-offer-backdrop'])(
+    '%s uses the shared scrim', (sel) => {
+      expect(rule(sel)['background']).toBe('var(--cudly-scrim)');
+    },
+  );
+
+  test.each(['.modal-content', '.modal-confirm', '.archera-offer-panel'])(
+    '%s uses the shared radius, shadow and surface', (sel) => {
+      const r = rule(sel);
+      expect(r['border-radius']).toBe('var(--cudly-r-lg)');
+      expect(r['box-shadow']).toBe('var(--cudly-shadow-dialog)');
+      expect(r['background']).toBe('var(--cudly-surface)');
+    },
+  );
+
+  test.each([
+    ['.modal', 'var(--cudly-z-modal)'],
+    ['.modal-overlay', 'var(--cudly-z-modal)'],
+    ['#archera-offer-modal-container', 'var(--cudly-z-modal-raised)'],
+    ['#archera-page-container', 'var(--cudly-z-page-overlay)'],
+    ['.modal-confirm-backdrop', 'var(--cudly-z-confirm)'],
+    ['.toast-container', 'var(--cudly-z-toast)'],
+  ])('%s stacks at %s', (sel, z) => {
+    expect(rule(sel)['z-index']).toBe(z);
+  });
+});
+
+describe('one card surface (issue #743)', () => {
+  test('.card is a hairline border + small shadow on the surface color', () => {
+    const r = rule('.card');
+    expect(r['border']).toBe('1px solid var(--cudly-border)');
+    expect(r['box-shadow']).toBe('var(--cudly-shadow-sm)');
+    expect(r['border-radius']).toBe('var(--cudly-r-md)');
+    expect(r['background']).toBe('var(--cudly-surface)');
+  });
+
+  test('.kpi-tile inherits the surface from .card instead of redefining it', () => {
+    const r = rule('.kpi-tile');
+    expect(r['border']).toBeUndefined();
+    expect(r['box-shadow']).toBeUndefined();
+  });
+
+  test.each(['.plan-card', '.group-card', '.upcoming-card'])('%s matches the card surface', (sel) => {
+    const r = rule(sel);
+    expect(r['border']).toBe('1px solid var(--cudly-border)');
+    expect(r['box-shadow']).toBe('var(--cudly-shadow-sm)');
+    expect(r['border-radius']).toBe('var(--cudly-r-md)');
+    expect(r['background']).toBe('var(--cudly-surface)');
+  });
+});

frontend/src/dashboard.ts

@@ -16,6 +16,7 @@ import { groupRecsByCell, pageLevelRange, formatSavingsRange, triggerAutoRefresh
 import { getRecommendationsFreshness } from './api/recommendations';
 import { buildFirstRunEmptyState, isScopeFiltered } from './first-run-empty-state';
 import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/skeleton';
+import { buildKpiTile } from './lib/kpi-tile';
 import { scopeUpcomingToProvider, upcomingEmptyMessage, UPCOMING_ACCOUNT_SCOPE_NOTE } from './lib/upcoming-scope';
 import { formatTrendAxisTick, formatTrendTooltipTitle, formatSparseTrendMessage, MIN_TREND_POINTS } from './modules/chart-utils';
 import { setChartSummary, renderChartTable, removeChartTable } from './lib/chart-a11y';
@@ -329,31 +330,9 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
     { kpi: 'ytd',         title: 'YTD Savings', value: formatCurrency(data.ytd_savings), valueSavings: true,
       detail: 'From commitment purchases' },
   ];
-  for (const t of tiles) {
-    const card = document.createElement('div');
-    card.classList.add('card', 'kpi-tile');
-    card.dataset['kpi'] = t.kpi;
-    const h3 = document.createElement('h3');
-    h3.textContent = t.title;
-    const valueP = document.createElement('p');
-    valueP.classList.add('value', 'kpi-tile-value');
-    // An absent figure is not a saving: keep the placeholder neutral rather
-    // than green (issue #739).
-    if (t.valueSavings && t.value !== NO_VALUE) valueP.classList.add('savings');
-    valueP.textContent = t.value;
-    const detailP = document.createElement('p');
-    detailP.classList.add('detail', 'kpi-tile-detail');
-    detailP.textContent = t.detail;
-    const spark = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
-    spark.classList.add('kpi-tile-spark', 'hidden');
-    spark.dataset['sparkKey'] = t.kpi;
-    spark.setAttribute('aria-hidden', 'true');
-    card.appendChild(h3);
-    card.appendChild(valueP);
-    card.appendChild(detailP);
-    card.appendChild(spark);
-    summary.appendChild(card);
-  }
+  // Shared with the Opportunities summary (issue #743). buildKpiTile keeps an
+  // absent (NO_VALUE) figure neutral rather than green (issue #739).
+  for (const t of tiles) summary.appendChild(buildKpiTile({ ...t, spark: true }));
 }
 
 /**

frontend/src/lib/kpi-tile.ts

@@ -0,0 +1,76 @@
+/**
+ * Shared KPI tile builder (issue #743).
+ *
+ * Home's summary built `.card.kpi-tile` elements by hand while Opportunities
+ * wrote plain `.card` innerHTML, so the same kind of figure looked different
+ * on the two pages. Both now build their tiles here.
+ *
+ * Markup (styles in components.css under `.kpi-tile`):
+ *   <div class="card kpi-tile" data-kpi="savings">
+ *     <h3>Title</h3>
+ *     <p class="value kpi-tile-value [savings]">Value</p>
+ *     <p class="detail kpi-tile-detail">Detail</p>        (optional)
+ *     <svg class="kpi-tile-spark hidden" data-spark-key>  (optional)
+ *   </div>
+ *
+ * The `card` / `value` / `savings` classes stay for existing selectors and
+ * tests. Text is set with textContent, never innerHTML.
+ */
+
+import { NO_VALUE } from '../utils';
+
+export interface KpiTileSpec {
+  /** Stable key, rendered as data-kpi (and data-spark-key when spark is set). */
+  kpi: string;
+  title: string;
+  value: string;
+  /**
+   * Render the value in the savings (green) color. An absent figure
+   * (NO_VALUE) is not a saving, so it stays neutral (issue #739).
+   */
+  valueSavings?: boolean;
+  /** Small line under the value; omitted when absent. */
+  detail?: string;
+  /** Reserve a hidden sparkline slot that a caller fills in later. */
+  spark?: boolean;
+}
+
+const SVG_NS = 'http://www.w3.org/2000/svg';
+
+export function buildKpiTile(spec: KpiTileSpec): HTMLDivElement {
+  const card = document.createElement('div');
+  card.classList.add('card', 'kpi-tile');
+  card.dataset['kpi'] = spec.kpi;
+
+  const h3 = document.createElement('h3');
+  h3.textContent = spec.title;
+  card.appendChild(h3);
+
+  const value = document.createElement('p');
+  value.classList.add('value', 'kpi-tile-value');
+  if (spec.valueSavings && spec.value !== NO_VALUE) value.classList.add('savings');
+  value.textContent = spec.value;
+  card.appendChild(value);
+
+  if (spec.detail !== undefined) {
+    const detail = document.createElement('p');
+    detail.classList.add('detail', 'kpi-tile-detail');
+    detail.textContent = spec.detail;
+    card.appendChild(detail);
+  }
+
+  if (spec.spark) {
+    const spark = document.createElementNS(SVG_NS, 'svg');
+    spark.classList.add('kpi-tile-spark', 'hidden');
+    spark.dataset['sparkKey'] = spec.kpi;
+    spark.setAttribute('aria-hidden', 'true');
+    card.appendChild(spark);
+  }
+
+  return card;
+}
+
+/** Replace `container`'s children with one tile per spec. */
+export function renderKpiTiles(container: HTMLElement, specs: readonly KpiTileSpec[]): void {
+  container.replaceChildren(...specs.map(buildKpiTile));
+}