Skip to content

Commit aed5ee3

Browse files
philluiz2323philluiz2323
committed
fix(ui): migrate 3 hand-rolled tables to the shared Table component
check-run-readiness-table.tsx, command-table.tsx, and contributor-quality-table.tsx each hand-rolled raw <table>/<thead>/<tbody> markup instead of the shared Table/TableHeader/TableBody/TableRow/TableCell/TableCaption primitives from @/components/ui/table, unlike the rest of the app's adopted convention (epic #6504). Migrated all three, preserving each table's existing columns, formatting logic, and per-cell styling exactly -- no new wrapper component invented. command-table.tsx had no existing test; added one covering its columns, per-entry rendering, and the DEFAULT_ROLE_SUMMARY fallback.
1 parent b8621fc commit aed5ee3

4 files changed

Lines changed: 144 additions & 67 deletions

File tree

‎apps/loopover-ui/src/components/site/app-panels/contributor-quality-table.tsx‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import { BoundaryBadge, StatusPill } from "@/components/site/control-primitives";
22
import { TableScroll } from "@/components/site/data-table";
33
import { EmptyState } from "@/components/site/state-views";
4+
import {
5+
Table,
6+
TableBody,
7+
TableCaption,
8+
TableCell,
9+
TableHead,
10+
TableHeader,
11+
TableRow,
12+
} from "@/components/ui/table";
413
import {
514
QUALITY_BAND_TONE,
615
type MaintainerTopContributor,
@@ -32,42 +41,44 @@ export function ContributorQualityTable({
3241
/>
3342
) : (
3443
<TableScroll className="mt-4" label="Top contributors by quality band">
35-
<table className="w-full min-w-[420px] whitespace-nowrap text-left text-token-sm">
36-
<caption className="sr-only">
44+
<Table className="min-w-[420px] whitespace-nowrap text-left text-token-sm">
45+
<TableCaption className="sr-only">
3746
Contributors with their quality band and open pull request count.
38-
</caption>
39-
<thead>
40-
<tr className="border-b-hairline font-mono text-token-2xs uppercase tracking-wider text-muted-foreground">
41-
<th scope="col" className="py-2 pr-3 font-normal">
47+
</TableCaption>
48+
<TableHeader>
49+
<TableRow className="border-b-hairline font-mono text-token-2xs uppercase tracking-wider text-muted-foreground">
50+
<TableHead scope="col" className="py-2 pr-3 font-normal">
4251
Contributor
43-
</th>
44-
<th scope="col" className="py-2 pr-3 font-normal">
52+
</TableHead>
53+
<TableHead scope="col" className="py-2 pr-3 font-normal">
4554
Band
46-
</th>
47-
<th scope="col" className="py-2 font-normal">
55+
</TableHead>
56+
<TableHead scope="col" className="py-2 font-normal">
4857
Open PRs
49-
</th>
50-
</tr>
51-
</thead>
52-
<tbody>
58+
</TableHead>
59+
</TableRow>
60+
</TableHeader>
61+
<TableBody>
5362
{topContributors.map((contributor) => (
54-
<tr
63+
<TableRow
5564
key={contributor.login}
5665
className="border-b-hairline last:border-b-0 transition-colors hover:bg-muted/40"
5766
>
58-
<td className="py-2 pr-3 font-medium text-foreground">{contributor.login}</td>
59-
<td className="py-2 pr-3">
67+
<TableCell className="py-2 pr-3 font-medium text-foreground">
68+
{contributor.login}
69+
</TableCell>
70+
<TableCell className="py-2 pr-3">
6071
<StatusPill status={QUALITY_BAND_TONE[contributor.band] ?? "info"}>
6172
{contributor.band}
6273
</StatusPill>
63-
</td>
64-
<td className="py-2 font-mono text-token-xs text-muted-foreground">
74+
</TableCell>
75+
<TableCell className="py-2 font-mono text-token-xs text-muted-foreground">
6576
{contributor.openPrCount}
66-
</td>
67-
</tr>
77+
</TableCell>
78+
</TableRow>
6879
))}
69-
</tbody>
70-
</table>
80+
</TableBody>
81+
</Table>
7182
</TableScroll>
7283
)}
7384
</section>

‎apps/loopover-ui/src/components/site/check-run-readiness-table.tsx‎

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
import { StatusPill, type Status } from "@/components/site/control-primitives";
22
import { TableScroll } from "@/components/site/data-table";
3+
import {
4+
Table,
5+
TableBody,
6+
TableCaption,
7+
TableCell,
8+
TableHead,
9+
TableHeader,
10+
TableRow,
11+
} from "@/components/ui/table";
312
import {
413
COMPONENT_BAND_LABEL,
514
READINESS_BAND_LABEL,
@@ -62,43 +71,43 @@ export function CheckRunReadinessTable({
6271
className="rounded-token border-hairline"
6372
label="Context check readiness signals"
6473
>
65-
<table className="w-full text-left text-token-xs">
66-
<caption className="sr-only">
74+
<Table className="text-left text-token-xs">
75+
<TableCaption className="sr-only">
6776
Readiness signals with their band, evidence, and recommended action.
68-
</caption>
69-
<thead className="border-b-hairline font-mono uppercase tracking-wider text-muted-foreground">
70-
<tr>
71-
<th scope="col" className="px-3 py-2 font-normal">
77+
</TableCaption>
78+
<TableHeader className="border-b-hairline font-mono uppercase tracking-wider text-muted-foreground">
79+
<TableRow>
80+
<TableHead scope="col" className="px-3 py-2 font-normal">
7281
Signal
73-
</th>
74-
<th scope="col" className="px-3 py-2 font-normal">
82+
</TableHead>
83+
<TableHead scope="col" className="px-3 py-2 font-normal">
7584
Band
76-
</th>
77-
<th scope="col" className="px-3 py-2 font-normal">
85+
</TableHead>
86+
<TableHead scope="col" className="px-3 py-2 font-normal">
7887
Evidence
79-
</th>
80-
<th scope="col" className="hidden px-3 py-2 font-normal lg:table-cell">
88+
</TableHead>
89+
<TableHead scope="col" className="hidden px-3 py-2 font-normal lg:table-cell">
8190
Action
82-
</th>
83-
</tr>
84-
</thead>
85-
<tbody>
91+
</TableHead>
92+
</TableRow>
93+
</TableHeader>
94+
<TableBody>
8695
{view.components.map((row) => (
87-
<tr key={row.key} className="border-b-hairline last:border-b-0 align-top">
88-
<td className="px-3 py-2 font-medium text-foreground">{row.label}</td>
89-
<td className="px-3 py-2">
96+
<TableRow key={row.key} className="border-b-hairline last:border-b-0 align-top">
97+
<TableCell className="px-3 py-2 font-medium text-foreground">{row.label}</TableCell>
98+
<TableCell className="px-3 py-2">
9099
<StatusPill status={COMPONENT_BAND_TONE[row.band]}>
91100
{COMPONENT_BAND_LABEL[row.band]}
92101
</StatusPill>
93-
</td>
94-
<td className="px-3 py-2 text-muted-foreground">{row.evidence}</td>
95-
<td className="hidden px-3 py-2 text-muted-foreground lg:table-cell">
102+
</TableCell>
103+
<TableCell className="px-3 py-2 text-muted-foreground">{row.evidence}</TableCell>
104+
<TableCell className="hidden px-3 py-2 text-muted-foreground lg:table-cell">
96105
{row.action}
97-
</td>
98-
</tr>
106+
</TableCell>
107+
</TableRow>
99108
))}
100-
</tbody>
101-
</table>
109+
</TableBody>
110+
</Table>
102111
</TableScroll>
103112
</section>
104113
);

‎apps/loopover-ui/src/components/site/command-table.test.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { render, screen, within } from "@testing-library/react";
2+
import { describe, expect, it } from "vitest";
3+
4+
import { CommandTable } from "@/components/site/command-table";
5+
6+
// #6986: pinned after migrating the hand-rolled <table> markup onto the shared Table primitive --
7+
// confirms the real table structure, columns, and default-role lookup still render correctly.
8+
describe("CommandTable", () => {
9+
it("renders a real <table> with the syntax/effect/default-roles columns and one row per entry", () => {
10+
render(
11+
<CommandTable
12+
title="Commands"
13+
entries={[
14+
{ id: "review", title: "Review", description: "Runs a review pass." },
15+
{
16+
id: "unlisted-command",
17+
title: "Unlisted",
18+
description: "Not in the role summary map.",
19+
},
20+
]}
21+
/>,
22+
);
23+
24+
const table = screen.getByRole("table");
25+
const headers = within(table)
26+
.getAllByRole("columnheader")
27+
.map((cell) => cell.textContent);
28+
expect(headers).toEqual(["Syntax", "Effect", "Default roles"]);
29+
30+
const rows = within(table).getAllByRole("row");
31+
expect(rows).toHaveLength(3); // header row + 2 entries
32+
33+
expect(within(rows[1]!).getByText("@loopover review")).toBeTruthy();
34+
expect(within(rows[1]!).getByText("Runs a review pass.")).toBeTruthy();
35+
expect(within(rows[1]!).getByText("maintainer, collaborator, confirmed_miner")).toBeTruthy();
36+
37+
// Falls back to "see policy" when the entry id has no DEFAULT_ROLE_SUMMARY mapping.
38+
expect(within(rows[2]!).getByText("@loopover unlisted-command")).toBeTruthy();
39+
expect(within(rows[2]!).getByText("see policy")).toBeTruthy();
40+
});
41+
42+
it("renders the title heading", () => {
43+
render(<CommandTable title="Commands reference" entries={[]} />);
44+
expect(screen.getByRole("heading", { name: "Commands reference" })).toBeTruthy();
45+
});
46+
});

‎apps/loopover-ui/src/components/site/command-table.tsx‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
import {
2+
Table,
3+
TableBody,
4+
TableCell,
5+
TableHead,
6+
TableHeader,
7+
TableRow,
8+
} from "@/components/ui/table";
9+
110
const DEFAULT_ROLE_SUMMARY: Record<string, string> = {
211
help: "maintainer, collaborator, confirmed_miner (default policy)",
312
ask: "maintainer, collaborator, confirmed_miner",
@@ -38,28 +47,30 @@ export function CommandTable({
3847
<>
3948
<h2>{title}</h2>
4049
<div className="not-prose overflow-x-auto">
41-
<table className="w-full border-collapse text-token-sm">
42-
<thead>
43-
<tr className="border-hairline text-left text-token-xs text-muted-foreground">
44-
<th className="py-2 pr-4 font-medium">Syntax</th>
45-
<th className="py-2 pr-4 font-medium">Effect</th>
46-
<th className="py-2 font-medium">Default roles</th>
47-
</tr>
48-
</thead>
49-
<tbody className="divide-hairline">
50+
<Table className="border-collapse text-token-sm">
51+
<TableHeader>
52+
<TableRow className="border-hairline text-left text-token-xs text-muted-foreground">
53+
<TableHead className="py-2 pr-4 font-medium">Syntax</TableHead>
54+
<TableHead className="py-2 pr-4 font-medium">Effect</TableHead>
55+
<TableHead className="py-2 font-medium">Default roles</TableHead>
56+
</TableRow>
57+
</TableHeader>
58+
<TableBody className="divide-hairline">
5059
{entries.map((entry) => (
51-
<tr key={entry.id} className="align-top">
52-
<td className="py-2 pr-4 font-mono text-token-xs whitespace-nowrap">
60+
<TableRow key={entry.id} className="align-top">
61+
<TableCell className="py-2 pr-4 font-mono text-token-xs whitespace-nowrap">
5362
@loopover {entry.id}
54-
</td>
55-
<td className="py-2 pr-4 text-muted-foreground">{entry.description}</td>
56-
<td className="py-2 text-muted-foreground">
63+
</TableCell>
64+
<TableCell className="py-2 pr-4 text-muted-foreground">
65+
{entry.description}
66+
</TableCell>
67+
<TableCell className="py-2 text-muted-foreground">
5768
{DEFAULT_ROLE_SUMMARY[entry.id] ?? "see policy"}
58-
</td>
59-
</tr>
69+
</TableCell>
70+
</TableRow>
6071
))}
61-
</tbody>
62-
</table>
72+
</TableBody>
73+
</Table>
6374
</div>
6475
</>
6576
);

0 commit comments

Comments
 (0)