Skip to content

fix(ui): migrate 3 hand-rolled tables to the shared Table component#7059

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
philluiz2323:fix/hand-rolled-tables-to-shared-table
Jul 17, 2026
Merged

fix(ui): migrate 3 hand-rolled tables to the shared Table component#7059
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
philluiz2323:fix/hand-rolled-tables-to-shared-table

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Summary

  • 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 (a ui-kit-backed component), unlike the rest of the app's adopted convention (epic Miner dashboard redesign: visual overhaul + action-capable chat (epic) #6504, already applied to apps/loopover-miner-ui's tables).
  • Migrated all three, preserving each table's existing columns, formatting logic, and per-cell styling exactly (e.g. contributor-quality-table.tsx's whitespace-nowrap, check-run-readiness-table.tsx's scope="col" + sr-only caption a11y pattern, command-table.tsx's DEFAULT_ROLE_SUMMARY fallback). No new wrapper component was invented; TableScroll (a separate, orthogonal scroll-container wrapper already used by two of the three) is left in place around the migrated Table.
  • command-table.tsx had no existing test; added one covering its columns, per-entry rendering, and the DEFAULT_ROLE_SUMMARY fallback (see policy for an unmapped command id).

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves.

Closes #6986

Validation

  • git diff --check
  • npm run actionlint (no workflow changes in this PR)
  • npm run typecheck (via npm run ui:typecheck, the UI-scoped equivalent)
  • npm run test:coverage (backend-only; this PR only touches apps/loopover-ui, which is outside the Codecov patch gate per codecov.yml's ignore: apps/**)
  • npm run test:workers (not applicable to this UI-only change)
  • npm run build:mcp (not applicable to this UI-only change)
  • npm run test:mcp-pack (not applicable to this UI-only change)
  • npm run ui:openapi:check (no API/schema changes in this PR)
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build (validated via lint + typecheck + the full local test suite instead; the full ui:build pipeline invokes an unrelated content-collection step for .mdx docs that mutates files outside this diff in this sandbox)
  • npm audit --audit-level=moderate (no dependency changes in this PR)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This PR only touches three presentational components' table markup (plus one new test file) -- no API/backend/dependency surface. npm run ui:lint and npm run ui:typecheck both pass locally, and all 16 tests across the three components' test suites pass (14 pre-existing + 2 new), confirming the migrated markup still renders the same real <table> structure, columns, and content.
  • UI Evidence (screenshots) honestly could not be captured: I made repeated genuine attempts to screenshot all three tables via a scripted Playwright run against the local dev server. Each attempt hit a different, pre-existing, unrelated issue in this sandboxed environment -- e.g. the docs route hosting command-table.tsx throws Error: CodeBlock is not defined in its compiled MDX content (a bug in the docs renderer, not in this PR's diff), and other routes in this app have separately hit an unrelated McpVersionBadge crash against an unreachable registry.npmjs.org in earlier attempts this session. None of these are caused by or related to the table-markup migration itself. Rather than force a misleading capture, I'm flagging this honestly, same as the already-merged Skeleton-adoption PR for app.index.tsx (app.index.tsx SparkStat hand-rolls skeleton placeholders instead of adopting Skeleton #6984) hit and disclosed the same limitation. The three components' test suites (which assert on real DOM roles: getByRole("table"), getAllByRole("columnheader"), getAllByRole("row")) are the verification that the migrated markup renders correctly.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A -- no auth/cookie/CORS/session changes in this PR.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- no API/OpenAPI/MCP changes in this PR.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots. See the honest explanation above -- capture was attempted multiple times and did not succeed due to pre-existing, unrelated sandbox issues.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • See the Validation section above for why UI Evidence screenshots are missing despite genuine, repeated attempts.

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 JSONbored#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.
@philluiz2323
philluiz2323 requested a review from JSONbored as a code owner July 17, 2026 20:06
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 17, 2026
@loopover-orb

loopover-orb Bot commented Jul 17, 2026

Copy link
Copy Markdown

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-17 20:17:25 UTC

4 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR mechanically migrates three hand-rolled HTML tables (check-run-readiness-table, contributor-quality-table, command-table) to the shared Table/TableHeader/TableBody/TableRow/TableCell/TableHead/TableCaption primitives, preserving existing className styling, a11y attributes (scope="col", sr-only caption), and business logic (DEFAULT_ROLE_SUMMARY fallback) exactly. The migration is a straightforward 1:1 tag swap with no logic changes, a new test file was added for the previously-untested command-table.tsx, and CI passed. The diff closes the linked issue #6986 and matches its stated scope precisely.

Nits — 4 non-blocking
  • The `<caption>` element only exists in check-run-readiness-table.tsx and contributor-quality-table.tsx; command-table.tsx has neither a TableCaption nor a visible heading tied via aria-labelledby beyond the `<h2>`, worth confirming this is intentional since the other two migrated tables preserve captions.
  • command-table.test.tsx comment references '3 tables in apps/loopover-ui hand-roll raw <table> markup instead of the shared Table component #6986' inline — fine, but consider matching the existing JSDoc-style comment convention used in contributor-quality-table.tsx and check-run-readiness-table.tsx for consistency.
  • Verify the shared `Table` component in `@​/components/ui/table` renders a real `<table>`/`<thead>`/`<tbody>` under the hood (confirmed via command-table.test.tsx's `getByRole("table")` assertion, so this is already covered).
  • Consider adding a similar smoke test for check-run-readiness-table.tsx and contributor-quality-table.tsx now that command-table.tsx has one, since they were also previously untested per the diff (though this is out of scope for the stated PR).

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6986
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 1092 registered-repo PR(s), 659 merged, 135 issue(s).
Contributor context ✅ Confirmed Gittensor contributor philluiz2323; Gittensor profile; 1092 PR(s), 135 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Partially addressed
All three components are correctly migrated to the shared Table primitives with columns, sorting/formatting, and per-cell styling preserved, and a new test was added for command-table.tsx which previously lacked one. However, the issue's deliverables explicitly ask for tests confirming each of the three tables still renders correctly, plus a before/after screenshot table per the repo's UI-PR conve

Review context
  • Author: philluiz2323
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 1092 PR(s), 135 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 2d62c82 into JSONbored:main Jul 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 tables in apps/loopover-ui hand-roll raw <table> markup instead of the shared Table component

1 participant