From e4525e4c0bc06411300c0fcfec74196fde8a0ba5 Mon Sep 17 00:00:00 2001 From: Roman Mitasov Date: Wed, 15 Jul 2026 13:18:40 +0300 Subject: [PATCH 1/2] =?UTF-8?q?Revert=20"fix(team-view):=20grow=20Members?= =?UTF-8?q?=20=C3=97=20metrics=20grid=20columns=20to=20the=20full=20metric?= =?UTF-8?q?=20set=20(#198)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 1a444929fcface6ad55a2378b0fac85b58f112b5. Signed-off-by: Roman Mitasov --- .../widgets/v2/members-heatmap/index.test.tsx | 78 -------- .../widgets/v2/members-heatmap/index.tsx | 173 +++--------------- 2 files changed, 28 insertions(+), 223 deletions(-) diff --git a/src/components/widgets/v2/members-heatmap/index.test.tsx b/src/components/widgets/v2/members-heatmap/index.test.tsx index af21dba..7cd49f3 100644 --- a/src/components/widgets/v2/members-heatmap/index.test.tsx +++ b/src/components/widgets/v2/members-heatmap/index.test.tsx @@ -268,84 +268,6 @@ describe("", () => { expect(screen.queryByText(/^\d+ issues?$/)).not.toBeInTheDocument(); }); - it("grid grows a column per roster bullet and unified entry, deduped against the curated set (#1729)", async () => { - fetchCatalog.mockResolvedValue( - buildCatalogResponse([ - { - metric_key: "task_delivery_bullet_rows.mean_time_to_resolution", - higher_is_better: false, - schema_status: "ok", - }, - { - metric_key: "collaboration_bullet_rows.code_review_speed", - higher_is_better: true, - schema_status: "ok", - }, - ]), - ); - const bulletsByPerson = new Map([ - [ - "alice@example.com", - [ - // Covered by the curated MTTR column → no second column. - makeBullet({ value: "30" }), - makeBullet({ - section: "collaboration", - metric_key: "code_review_speed", - label: "Code review speed", - value: "9", - unit: "", - }), - ], - ], - ]); - const metricEntriesByPerson = new Map([ - [ - "alice@example.com", - [ - makeEntry(), - // Dot-suffix collides with the team_row `prs_merged` column → deduped. - makeEntry({ key: "git.prs_merged", label: "PRs merged", value: 99 }), - ], - ], - ]); - renderWithCatalogClient( - , - ); - - // Non-curated bullet and unified entries become sortable columns. - expect( - await screen.findByRole("button", { - name: "Code review speed — sort by this column", - }), - ).toBeInTheDocument(); - expect( - screen.getByRole("button", { name: "Commits — sort by this column" }), - ).toBeInTheDocument(); - // Key/label collisions collapse into the curated columns. - expect( - screen.getAllByRole("button", { - name: "Mean time to resolution — sort by this column", - }), - ).toHaveLength(1); - expect( - screen.getAllByRole("button", { - name: "PRs merged — sort by this column", - }), - ).toHaveLength(1); - // Unified cell renders the entry's value with its own-cohort status. - expect( - screen.getByRole("button", { name: "Alice — Commits: 12 — Top 25%" }), - ).toBeInTheDocument(); - }); - it("details sheet shows the full metric set: grid columns + remaining bullets + unified entries, deduped", async () => { const user = userEvent.setup(); fetchCatalog.mockResolvedValue( diff --git a/src/components/widgets/v2/members-heatmap/index.tsx b/src/components/widgets/v2/members-heatmap/index.tsx index f4729ea..be996d3 100644 --- a/src/components/widgets/v2/members-heatmap/index.tsx +++ b/src/components/widgets/v2/members-heatmap/index.tsx @@ -20,7 +20,6 @@ import { bulletCatalogKey, type CatalogByKey, } from "@/lib/insight/v2/peer-status"; -import type { MetricFormat } from "@/api/metric-results-client"; import type { PeerStoryEntry } from "@/lib/metrics/peer-story"; import { MIN_DEPT_COHORT_N } from "@/lib/insight/v2/team-member-status"; import { @@ -102,13 +101,7 @@ interface BulletColumn extends BaseColumn { metricKey: string; } -interface MetricColumn extends BaseColumn { - source: "metric"; - entryKey: string; - format: MetricFormat; -} - -type ColumnDef = TeamRowColumn | BulletColumn | MetricColumn; +type ColumnDef = TeamRowColumn | BulletColumn; // FE-controlled column layout: `label`/`short`/`unit`/`mobile`/`source` // are display concerns the wire response doesn't carry, and the @@ -119,12 +112,7 @@ type ColumnDef = TeamRowColumn | BulletColumn | MetricColumn; // A future wave can fold the bullet-source `higher_is_better` entries // into a catalog lookup once team_row policy thresholds also move to // the wire. -// -// These are only the CURATED core columns (stable order, mobile flags, -// hand-picked short labels). The full column set is derived at render -// time: every legacy bullet and unified-path (git/ai) entry present in -// the roster's data becomes an additional column (#1729). -const CORE_COLUMNS: ColumnDef[] = [ +const COLUMNS: ColumnDef[] = [ { key: "tasks_closed", label: "Tasks closed", @@ -197,16 +185,6 @@ const CORE_COLUMNS: ColumnDef[] = [ }, ]; -/** - * Compact one-letter units ("d", "h", "%") glue to the number the way the - * curated columns always rendered; word units from bullet rows ("meetings", - * "files") get a separating space. - */ -function unitSuffix(unit: string | undefined | null): string { - if (!unit) return ""; - return unit.length > 1 ? ` ${unit}` : unit; -} - function getNumericTeamRow(m: TeamMember, key: TeamRowKey): number | null { const raw = m[key]; return typeof raw === "number" && Number.isFinite(raw) ? raw : null; @@ -223,7 +201,7 @@ function getNumericBullet( } function valueForColumn( - col: TeamRowColumn | BulletColumn, + col: ColumnDef, member: TeamMember, bullets: BulletMetric[] | undefined, ): number | null { @@ -233,16 +211,9 @@ function valueForColumn( return getNumericBullet(bullets, col.metricKey); } -/** The `metric_key` a heatmap column is identified / colored by. */ +/** The dept-distribution `metric_key` a heatmap column is colored against. */ function metricKeyForColumn(col: ColumnDef): string { - switch (col.source) { - case "team_row": - return col.teamRowField; - case "bullet": - return col.metricKey; - case "metric": - return col.entryKey; - } + return col.source === "team_row" ? col.teamRowField : col.metricKey; } /** @@ -311,87 +282,13 @@ export function MembersHeatmap({ // their position vs department peers, not vs the displayed roster. const cohorts: DeptCohorts = deptCohorts ?? EMPTY_DEPT_COHORTS; - // Full column set (#1729): the curated core columns first, then a column - // for every legacy bullet and unified-path (git/ai) entry seen anywhere in - // the roster. Deduped by metric key; unified entries additionally dedupe - // against earlier columns by dot-suffix key and display label (the - // `git.prs_merged` ≈ `prs_merged` overlap). Members missing a metric - // render "—" in that cell. - const columns = useMemo(() => { - const cols: ColumnDef[] = [...CORE_COLUMNS]; - const seenKeys = new Set(cols.map(metricKeyForColumn)); - const seenLabels = new Set(cols.map((c) => c.label.toLowerCase())); - for (const m of members) { - const bullets = bulletsByPerson?.get(m.person_id.toLowerCase()) ?? []; - for (const b of bullets) { - if (seenKeys.has(b.metric_key)) continue; - seenKeys.add(b.metric_key); - seenLabels.add(b.label.toLowerCase()); - cols.push({ - key: b.metric_key, - label: b.label, - short: b.label, - unit: b.unit ?? "", - higher_is_better: - byMetricKey(bulletCatalogKey(b))?.higher_is_better ?? true, - mobile: false, - source: "bullet", - metricKey: b.metric_key, - }); - } - } - for (const m of members) { - const entries = - metricEntriesByPerson?.get(m.person_id.toLowerCase()) ?? []; - for (const e of entries) { - const suffix = e.key.split(".").pop() ?? e.key; - if ( - seenKeys.has(e.key) || - seenKeys.has(suffix) || - seenLabels.has(e.label.toLowerCase()) - ) { - continue; - } - seenKeys.add(e.key); - seenLabels.add(e.label.toLowerCase()); - cols.push({ - key: e.key, - label: e.label, - short: e.label, - unit: e.unit ?? "", - higher_is_better: e.higherIsBetter, - mobile: false, - source: "metric", - entryKey: e.key, - format: e.format, - }); - } - } - return cols; - }, [members, bulletsByPerson, metricEntriesByPerson, byMetricKey]); - const rows = useMemo(() => { const built = members.map((m) => { const personIdKey = m.person_id.toLowerCase(); const bullets = bulletsByPerson?.get(personIdKey); const prevBullets = previousBulletsByPerson?.get(personIdKey); const prevMember = previousMembers?.get(personIdKey); - const entries = metricEntriesByPerson?.get(personIdKey) ?? []; - const cells = columns.map((col): CellShape => { - // Unified-path columns carry their own cohort: the entry's status is - // already resolved vs the person's own org unit by the peer view. - if (col.source === "metric") { - const entry = entries.find((e) => e.key === col.entryKey); - return { - col, - value: entry?.value ?? null, - previous: null, - status: entry?.status ?? "neutral", - median: entry?.stats?.p50 ?? null, - unit: entry?.unit ?? col.unit, - format: col.format, - }; - } + const cells = COLUMNS.map((col) => { const value = valueForColumn(col, m, bullets); const previous = prevMember ? valueForColumn(col, prevMember, prevBullets) @@ -473,7 +370,7 @@ export function MembersHeatmap({ }; }); return built; - }, [members, bulletsByPerson, previousBulletsByPerson, previousMembers, metricEntriesByPerson, columns, cohorts, byMetricKey]); + }, [members, bulletsByPerson, previousBulletsByPerson, previousMembers, cohorts, byMetricKey]); const sortedRows = useMemo(() => { const copy = [...rows]; @@ -486,8 +383,8 @@ export function MembersHeatmap({ a.member.name.localeCompare(b.member.name), ); } else { - const colIdx = columns.findIndex((c) => c.key === sortKey); - const col = columns[colIdx]; + const colIdx = COLUMNS.findIndex((c) => c.key === sortKey); + const col = COLUMNS[colIdx]; if (col) { copy.sort((a, b) => { const av = a.cells[colIdx]?.value ?? Number.POSITIVE_INFINITY; @@ -497,10 +394,10 @@ export function MembersHeatmap({ } } return copy; - }, [rows, sortKey, columns]); + }, [rows, sortKey]); const gridStyle = { - gridTemplateColumns: `minmax(140px, max-content) repeat(${columns.length}, minmax(56px, max-content))`, + gridTemplateColumns: `minmax(140px, max-content) repeat(${COLUMNS.length}, minmax(56px, 1fr))`, }; const triageRows: TriageRow[] = sortedRows.map((r) => ({ @@ -515,31 +412,24 @@ export function MembersHeatmap({ topCount: r.topCount, })); - // The sheet shows the member's FULL metric set (#1729). The grid columns - // now cover that set themselves, so the sheet is mostly a linear read of - // the row's cells; the bullet/entry passes below only catch stragglers the - // column dedup dropped (e.g. a label collision with a curated column). + // The sheet shows the member's FULL metric set, not just the 7 grid + // columns (#1729): grid cells first (they carry WoW/dept context the + // user just clicked through), then the remaining legacy bullets, then + // the unified-path (git/ai) entries. Bullets and entries that a column + // already covers are deduped — team_row columns overlap unified keys by + // dot-suffix (`git.prs_merged` ≈ `prs_merged`) and by display label. const sheetRows: MemberDetailRow[] = useMemo(() => { if (!sheetMember) return []; const row = rows.find((r) => r.member.person_id === sheetMember.person_id); if (!row) return []; - const columnKeys = new Set(columns.map(metricKeyForColumn)); - const columnLabels = new Set(columns.map((c) => c.label.toLowerCase())); + const columnKeys = new Set(COLUMNS.map(metricKeyForColumn)); + const columnLabels = new Set(COLUMNS.map((c) => c.label.toLowerCase())); const fromCells: MemberDetailRow[] = row.cells.map((c) => ({ key: c.col.key, label: c.col.label, - display: - c.value == null - ? "—" - : c.format - ? formatMetricValue(c.value, c.format, c.unit || null) - : `${Math.round(c.value)}${unitSuffix(c.unit)}`, + display: c.value == null ? "—" : `${Math.round(c.value)}${c.unit ?? ""}`, medianDisplay: - c.median == null - ? null - : c.format - ? formatMetricValue(c.median, c.format, c.unit || null) - : `${Math.round(c.median)}${unitSuffix(c.unit)}`, + c.median != null ? `${Math.round(c.median)}${c.unit ?? ""}` : null, status: c.status, })); const fromBullets: MemberDetailRow[] = row.bullets @@ -563,7 +453,6 @@ export function MembersHeatmap({ ) .filter( (e) => - !columnKeys.has(e.key) && !columnKeys.has(e.key.split(".").pop() ?? e.key) && !columnLabels.has(e.label.toLowerCase()), ) @@ -577,7 +466,7 @@ export function MembersHeatmap({ status: e.status, })); return [...fromCells, ...fromBullets, ...fromMetrics]; - }, [rows, columns, sheetMember, cohorts.bullet, byMetricKey, metricEntriesByPerson]); + }, [rows, sheetMember, cohorts.bullet, byMetricKey, metricEntriesByPerson]); const handleMemberClick = (m: TeamMember) => { setSheetMember(m); @@ -615,7 +504,7 @@ export function MembersHeatmap({
- {columns.map((c) => ( + {COLUMNS.map((c) => ( = WOW_THRESHOLD; const wowUp = wowPct != null && wowPct > 0; @@ -697,9 +584,7 @@ function HeatmapCell({ const display = value == null ? "—" - : format - ? formatMetricValue(value, format, unit || null) - : `${Math.round(value)}${unitSuffix(unit)}`; + : `${Math.round(value)}${unit ?? ""}`; return (

- {median == null - ? "No peer data" - : format - ? `Cohort median: ${formatMetricValue(median, format, unit || null)}` - : `Dept median: ${Math.round(median * 10) / 10}${unitSuffix(unit)}`} + {median != null + ? `Dept median: ${Math.round(median * 10) / 10}${unit ?? ""}` + : "No peer data"}

{PEER_LABEL[focused]} From c263d0782b43f9fa04aba950da74e3f17cc6c47a Mon Sep 17 00:00:00 2001 From: Roman Mitasov Date: Wed, 15 Jul 2026 13:06:22 +0300 Subject: [PATCH 2/2] fix(team-view): link the member popup to the IC page, open the sheet from Expand details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the Members × metrics popup, "Open in IC view" opened the details sheet while "Expand details" toggled an inline bullet strip under the row. Rework per review of #198: "Open in IC view" is now a router link to the member's personal IC page, and "Expand details" opens the details sheet (which already shows the full metric set after #193). The inline ExpandedBullets strip and its expansion state go away. Co-Authored-By: Claude Fable 5 Signed-off-by: Roman Mitasov --- .../widgets/v2/members-heatmap/index.test.tsx | 39 ++++++- .../widgets/v2/members-heatmap/index.tsx | 106 +++--------------- 2 files changed, 51 insertions(+), 94 deletions(-) diff --git a/src/components/widgets/v2/members-heatmap/index.test.tsx b/src/components/widgets/v2/members-heatmap/index.test.tsx index 7cd49f3..a0686ef 100644 --- a/src/components/widgets/v2/members-heatmap/index.test.tsx +++ b/src/components/widgets/v2/members-heatmap/index.test.tsx @@ -26,6 +26,38 @@ vi.mock("@/api/catalog-client", async () => { return { ...actual, fetchCatalog: vi.fn() }; }); +// The member popup renders a router `` to the IC page; these tests +// don't exercise navigation, so stub Link to a plain anchor (with the +// `$person` param interpolated so the href is assertable) rather than +// standing up a full router context. +vi.mock("@tanstack/react-router", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + Link: ({ + to, + params, + children, + ...rest + }: { + to?: string; + params?: Record; + children?: React.ReactNode; + }) => ( + + {children} + + ), + }; +}); + import * as catalogClient from "@/api/catalog-client"; import { buildCatalogResponse, @@ -327,8 +359,13 @@ describe("", () => { ); await user.click(await screen.findByRole("button", { name: "Alice" })); + // "Open in IC view" navigates to the member's page; the sheet opens via + // "Expand details". + expect( + await screen.findByRole("link", { name: "Open in IC view" }), + ).toHaveAttribute("href", "/ic/alice%40example.com/personal"); await user.click( - await screen.findByRole("button", { name: "Open in IC view" }), + screen.getByRole("button", { name: "Expand details" }), ); const sheet = within(await screen.findByRole("dialog", { name: "Alice" })); diff --git a/src/components/widgets/v2/members-heatmap/index.tsx b/src/components/widgets/v2/members-heatmap/index.tsx index be996d3..9e3f268 100644 --- a/src/components/widgets/v2/members-heatmap/index.tsx +++ b/src/components/widgets/v2/members-heatmap/index.tsx @@ -1,4 +1,5 @@ import { useMemo, useState } from "react"; +import { Link } from "@tanstack/react-router"; import { ArrowDown, ArrowUp } from "lucide-react"; import { useCatalog } from "@/api/use-catalog"; @@ -275,7 +276,6 @@ export function MembersHeatmap({ const { byMetricKey } = useCatalog(); const [sortKey, setSortKey] = useState("issues"); const [sheetMember, setSheetMember] = useState(null); - const [expandedId, setExpandedId] = useState(null); // Cohort = each member's OWN department distribution (per-(org_unit_id, // metric_key) quartiles fetched by the screen). A member's cell colour = @@ -517,14 +517,6 @@ export function MembersHeatmap({ key={row.member.person_id} row={row} focusMode={focusMode} - bulletStats={cohorts.bullet} - byMetricKey={byMetricKey} - expanded={expandedId === row.member.person_id} - onToggleExpand={() => - setExpandedId((cur) => - cur === row.member.person_id ? null : row.member.person_id, - ) - } onOpenSheet={() => handleMemberClick(row.member)} /> ))} @@ -644,22 +636,13 @@ function HeatmapCell({ function MemberRow({ row, focusMode, - bulletStats, - byMetricKey, - expanded, - onToggleExpand, onOpenSheet, }: { row: RowShape; focusMode: FocusMode; - bulletStats: DeptStatsMap; - byMetricKey: CatalogByKey; - expanded: boolean; - onToggleExpand: () => void; onOpenSheet: () => void; }) { - const { member, cells, belowCount, topCount, worstMetricLabel, bullets, orgUnitId } = - row; + const { member, cells, belowCount, topCount, worstMetricLabel } = row; const issueText = belowCount > 0 ? `${belowCount} issue${belowCount === 1 ? "" : "s"}` @@ -674,7 +657,6 @@ function MemberRow({ @@ -691,11 +673,19 @@ function MemberRow({ {belowCount} below department peers · {topCount} in top

- -
@@ -725,80 +715,10 @@ function MemberRow({ focusMode={focusMode} /> ))} - {expanded ? ( - - ) : null} ); } -function ExpandedBullets({ - bullets, - columnCount, - focusMode, - bulletStats, - orgUnitId, - byMetricKey, -}: { - bullets: BulletMetric[]; - columnCount: number; - focusMode: FocusMode; - bulletStats: DeptStatsMap; - orgUnitId: string | null; - byMetricKey: CatalogByKey; -}) { - const RANK: Record = { - bottom: 0, - in_pack: 1, - top: 2, - neutral: 3, - }; - const annotated = bullets.map((b) => ({ - bullet: b, - status: deptBulletStatus(b, bulletStats, orgUnitId, byMetricKey), - })); - annotated.sort((a, b) => RANK[a.status] - RANK[b.status]); - return ( -
-

- All metrics -

-
- {annotated.map(({ bullet: b, status }) => { - const focused = applyFocus(status, focusMode); - return ( - - - {b.label} - - - {b.value} - {b.unit ? ` ${b.unit}` : ""} - - - ); - })} -
-
- ); -} - function ColumnHeader({ col, active,