Uh oh!
There was an error while loading. Please reload this page.
feat(ui): converge card/table + badge surfaces onto primitives (#520 PR9) - #554
Merged
Conversation
…es (#520 PR9) Card (packages/ui/src/primitives/card.tsx) — thin surface container (data-slot="card" + radius-surface), following maka's ChoiceCard philosophy: each call site keeps its own layout/visual CSS. settingsRows, settingsMetricCard, and maka-error-card now route through Card; their CSS drops the border-radius line (Card owns it), layout is byte-identical. Table (packages/ui/src/primitives/table.tsx) — shadcn-style family (Table/TableHeader/TableBody/TableRow/TableHead/TableCell) with data-slot. settingsStatsTable retires entirely; the table chrome (border + radius + caption font-size) and the cell chrome (tabular-nums + hairline row separators + caption-tone color + semibold head) move into the primitive. maka-error-card stays on Card (not Alert): it is a large crash surface with shadow-modal + stack <pre>, not a small inline callout. card-table-converge-contract locks the migration. Updated four existing contracts whose selectors pinned the retired classes/structure: radius-converge (drop .settingsRows tier — Card owns it now), tabular-nums (drop .settingsStatsTable th/td — Table owns it now), settings-usage (regex now matches the Table family + scoped heads/cells), web-search-boundary (regex now matches <SettingsRows className=…>). Token values verified equivalent: --font-weight-semibold=600 (font-semibold), --space-1/2, --radius-surface=8px. Screenshot manifest passes (32/32, 0 fail).
#520 PR9 commit 2: collapse the four coexisting badge surfaces onto two canonical primitives, split by UI role. - pill Badge (primitives/badge.tsx): emphasis markers. Retire the PrimitiveBadge alias + the legacy ui.tsx Badge (raw emerald/amber variants). health/permission center, artifact-pane, plan-reminder, and permission-dialog route through <Badge>. - squared Chip (primitives/chip.tsx): dense status rows. Retire the .settingsBadge + .settingsConnectionBadge CSS chips. settings connection status / default / category markers route through <Chip>; variants mirror StatusTone, so settings callers pass the tone directly instead of going through statusBadgeVariant (which stays for the health/permission pill Badge sites). - contracts: badge-converge (pill track), chip-converge (squared track, locks radius-control not pill), settings-form-a11y (lock point moved from the CSS class to the Chip primitive), radius-converge (drop the stale ui.tsx badgeVariants entry). - visual zero-change: Chip cva reproduces the retired CSS oklch alphas (success/12, info/14, warning/18, destructive/15) and the neutral foreground-5 base. screenshot pixel diff vs main: settings-bots light/dark 1280 AE=0.
…ontract, comment fix #520 PR9 review fixes: - P2: the three .settingsBadge migration sites (provider-connection-detail x2, provider-add-form x1) now use Chip size="sm" to reproduce the retired .settingsBadge geometry (18px / font-normal / 0-6px padding). Without size="sm" they defaulted to the larger status-row size (20px / semibold / 2-8px padding) and drifted. The neutral background did not drift: bg-secondary aliases --color-secondary = var(--foreground-5), so only height/weight/padding moved. - P3-2: chip-converge-contract now locks user-visible tokens (neutral bg-secondary + foreground-secondary text, sm/default size geometry, status-tone alphas /12 /14 /18 /15) so a cva class change that keeps the import is still caught. Dropped the "no old span" migration-narrative loop; kept import + role-split + token assertions. - P3-3: bot.css / connection.css retire comments now say Chip variant="neutral" (not Badge variant="secondary"), matching the actual migration and not steering future migrants back to the pill Badge. Verification: 2050/2050 contract pass, typecheck clean, screenshot settings-bots light/dark 1280 + light 990 all AE=0 (pixel-identical to main after the size=sm fix; the 990 variant's earlier 13380 px diff is gone).
#520 PR9 review P3-1: the Table primitive had a single consumer (usage-settings-page SimpleStatsTable) and no second HTML <table> consumer in sight — the model "table" is a div-based list (.modelTable), not an HTML table, so it will never migrate onto Table. Per Occam's razor, lift the primitive only when a second real consumer appears. - Delete packages/ui/src/primitives/table.tsx and its barrel export. - SimpleStatsTable in usage-settings-page now renders a native HTML <table>/<thead>/<tbody>/<tr>/<th>/<td> with the same Tailwind classes the primitive applied (w-full border-collapse rounded-[var(--radius-surface)] border border-border text-caption + cell border-b/px/py/align/ tabular-nums). Styles stay inline so the stats surface is self-contained until a second HTML <table> consumer justifies lifting it back. - Rename card-table-converge-contract → card-converge-contract: drop the Table data-slot / table-sites assertions, keep Card. Note the usage stats table a11y semantics (aria-label + scope) now live in settings-usage-contract, whose assertions are updated to match the native <table>/<th scope> shape. - settings-form-a11y and tabular-nums are untouched: they assert on the .modelTable CSS class (a div surface), not on the Table primitive. Verification: 2049/2049 contract pass (one fewer test — the deleted "table sites import Table" case), typecheck clean. Screenshot pixel diff vs main: settings-general light/dark 1280 AE=0; settings-data 1280 shows ~4500 px (RMSE 0.001) in the `data` section, which is main-branch composer/sidebar drift (#510/#511), not this change — the usage stats table lives in the `usage` section and its styles are identical to the retired primitive.
…impleStatsTable Per PR9 review P3: the retire comment still referenced the deleted packages/ui/src/primitives/table.tsx, which would mislead a future maintainer into restoring a public Table primitive. Now points at the local native SimpleStatsTable in usage-settings-page.tsx, matching the actual state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#520 PR9: converge the hand-written settings card surfaces onto a
Cardprimitive, and collapse the four coexisting badge surfaces onto two role-split primitives — pillBadge(emphasis markers) and squaredChip(dense settings status rows).A review-driven correction: an initial
Tableprimitive was retired before merge — with a single HTML<table>consumer and no second one in sight (the model "table" is a div-based list, not an HTML table), it was premature. The usage stats table now stays a local native<table>inusage-settings-page.tsx.Why
Refs #520. The renderer had four parallel badge implementations and three hand-written settings card CSS surfaces, each drifting from the shared token system. The convergence goal is one primitive per UI role, governed by contracts instead of ad-hoc CSS.
A key finding drove the badge split: Base UI (
@base-ui/react1.5.0/1.6.0) ships no Badge/Chip/Tag component, and shadcn/ui shipsBadgebut noChip. So both primitives are self-built (cva +data-slot+ Base UIuseRender), same pattern as the existingCard/Alertprimitives. Settings status rows need compact squared chips (radius-control) for information density — thesettings-form-a11ycontract explicitly locks "not pills". Forcing them onto the pillBadgewould regress that intent and turn the destructive tone into a solid red block. The correct convergence is 4 surfaces → 2 primitives by role, not 4 → 1.The
Tableprimitive was added in an earlier commit but removed per review: the only HTML<table>consumer is the usage stats table, and the model "table" (.modelTable) is a div-based list with radio + chips, not an HTML table, so it will never migrate onto a Table primitive. By Occam's razor the primitive is lifted only when a second real consumer appears; the stats table keeps its styles inline.Scope
Changed:
packages/ui/src/primitives/card.tsx— thinCard(data-slot="card", radius-surface); caller keeps layout/visual CSS.packages/ui/src/primitives/chip.tsx— squaredChip(data-slot="chip", radius-control), 5 status-tone variants mirroringStatusTone,sm/defaultsizes. cva reproduces the retired CSS oklch alphas (success/12, info/14, warning/18, destructive/15) and the neutral foreground-5 base;smreproduces.settingsBadge(18px / font-normal / 0-6px padding) anddefaultreproduces.settingsConnectionBadge(20px / semibold / 2-8px padding).packages/ui/src/primitives/badge.tsxis now the canonical pillBadge; thePrimitiveBadgealias and the legacyui.tsxBadge(raw emerald/amber variants) are gone.Card:settingsRows+ 3 consumer pages,settingsMetricCard,maka-error-card. OntoBadge: health-center, permission-center, artifact-pane, plan-reminder, permission-dialog. OntoChip: provider-connection-detail (size="sm"for the generic label), provider-add-form (size="sm"), web-search, memory-settings, account-settings, provider-oauth (8 sites).<table>/<thead>/<tbody>/<tr>/<th scope>/<td>inusage-settings-page.tsx(SimpleStatsTable), with the same Tailwind classes the retired Table primitive applied — no public Table primitive..settingsStatsTable,.settingsBadge,.settingsConnectionBadge+ data-tone variants.card-converge-contract,badge-converge-contract,chip-converge-contract; updatedradius-converge(drop stale.settingsRowstier +ui.tsx badgeVariantsentry),tabular-nums(drop.settingsStatsTable),settings-usage(regex matches native<table>/<th scope>a11y),web-search-boundary(regex matches<SettingsRows>),settings-form-a11y(lock point moved from CSS class toChipprimitive).Not included:
composer-model-chip/prompt-chip/plan-card-chipCSS classes are not migrated toChipin this PR; they are left for a future sweep now thatChipexists as the convergence target.Tableprimitive is intentionally not shipped (see Why). Lift when a second HTML<table>consumer appears.Verification
npm run typecheck— clean.npm run -w @maka/desktop test— 2049/2049 pass (one fewer than the initial Table-inclusive state: the deleted "table sites import Table" case).npm run -w @maka/ui build+npm run -w @maka/desktop build:main— clean.settings-bots(8),settings-memory(8),settings-general(8),settings-data(8) captured (no crash).compare):settings-botsandsettings-generallight/dark 1280 AE=0 (pixel-identical).settings-data1280 shows ~4500 px diff (RMSE 0.001) in thedatasection, which is the main-branch composer/sidebar commits (feat(sidebar): add project-grouped session view mode #510/feat(composer): workspace picker, git branch switching, and composer defaults persistence #511), not this PR — the usage stats table lives in theusagesection (not in thesettings-datascenario) and its inline styles match the retired primitive exactly.User-facing impact
None intended. Card/badge/chip visuals are preserved by reproducing the retired CSS token values inside the primitives (and inside
SimpleStatsTablefor the usage stats table). ThesettingsConnectionBadgewarning tone no longer misuses--info(it now uses--warning, which aliases--infoso the color is unchanged but the intent is correct). The.settingsBadge-derived generic labels keep 18px / font-normal viaChip size="sm".Reviewer notes
Badge+ squaredChip) is the load-bearing decision. The alternative — collapsing all 4 surfaces onto pillBadge— was rejected because it regresses thesettings-form-a11y"compact squared, not pills" lock and turns the destructive status into a solid red block. Seechip-converge-contractfor the role-split + token lock.Tableprimitive was removed per review (P3-1): single consumer, and the model "table" is a div surface so no second consumer is coming.SimpleStatsTablekeeps the same styles inline.Chipvariants mirrorStatusTonedirectly, so settings callers passtonestraight through.statusBadgeVariantis retained because health/permission center still use it with the pillBadge.Cardis intentionally thin (onlydata-slot+ radius-surface); layout/visual CSS stays at call sites, matching theChoiceCardphilosophy.Checklist