Design system overhaul: unified tokens, component authority, standardized layout#1570
Design system overhaul: unified tokens, component authority, standardized layout#1570
Conversation
…ized layout Establish a single design token system and make React UI components the canonical styling authority, replacing the dual CSS-class/inline-style approach. No product behavior changes. ## Phase 1 — Design Foundation - Add 25+ semantic CSS custom properties to :root (status colors, form controls, interactive states, overlay, shadows, layout widths) with matching light-theme overrides - Expand @theme block from 24 → 80+ entries mapping all tokens to Tailwind v4 utilities (spacing, typography, status, form, layout) - Migrate 11 ui/ primitives to use tokens instead of hardcoded rgba/hex: input, textarea, label, select, badge, button, card, dialog, sheet, dropdown-menu, toggle-group - Introduce --accent-fg token for guaranteed text contrast on accent backgrounds across both themes - Add --page-max (1536px), --page-narrow (900px), --page-min (320px) layout tokens; standardize all page/nav/footer max-widths - Deprecate CSS classes (.btn, .tag, .card, .form-input, .form-label) with @deprecated comments — React components are now canonical ## Phase 2 — IA & Navigation - Extract shared nav config (src/lib/nav-items.ts) consumed by Header and Footer, eliminating 3-place navigation duplication - Add beforeLoad guard on /$owner/$slug catch-all route to validate owner handles and prevent confusing 404s on typos - Add UserBadge hover tooltip with lazy-fetched stats (published skills, stars, downloads) via new getHoverStats Convex query - Wrap app in TooltipProvider (src/components/AppProviders.tsx) ## Phase 3 — UI Sweep - Migrate 54 .btn usages → <Button> across 13 route/component files - Migrate 24 .tag usages → <Badge> across 12 files - Eliminate ~89 inline style={{}} occurrences → Tailwind utilities - Replace .card divs → <Card> component in detail/dashboard/search pages - Fix text-white → text-accent-fg in 5 locations (SkillsToolbar, InstallSwitcher, toggle-group, .star-toggle, .btn-primary) - Standardize all page max-widths to var(--page-max) token (was 6 different values: 900–1450px) - Add min-width: var(--page-min) to .app-shell - Convert raw styled <a>/<button> elements to <Button asChild> pattern ## New files - src/lib/nav-items.ts — shared navigation config - src/lib/icon-sizes.ts — ICON_SM/MD/LG constants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryLarge but well-scoped design-system overhaul: unified CSS custom properties, Tailwind v4 Confidence Score: 5/5Safe to merge — no blocking correctness, security, or data-integrity issues found. All findings are P2 style/performance suggestions. The design-system changes are mechanical token and component migrations with no product-behavior changes, and the new Convex query is functionally correct. convex/users.ts and src/components/UserBadge.tsx for the reactive-subscription and unbounded-collect concerns. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/components/UserBadge.tsx
Line: 104-107
Comment:
**`useQuery` creates persistent reactive subscription for tooltip stats**
Per the project's Convex performance rules, `ConvexHttpClient.query()` (one-shot fetch) should be used for data that doesn't need real-time updates. After the first hover, `shouldFetch` flips to `true` and stays `true` for the component's lifetime, leaving a live subscription to the `skills` table. On a browse page with many `UserBadge` components this accumulates one reactive subscription per hovered user — all re-run whenever any of that user's skills change.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: convex/users.ts
Line: 474-477
Comment:
**Unbounded `.collect()` on the `skills` table**
The query fetches every skill for the user and filters in JS. For a prolific user this scans all their skill documents in a single transaction, approaching Convex's per-transaction read budget. Adding a reasonable cap (e.g. `.take(500)`) would guard against edge cases and keep the query predictably cheap.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Design system overhaul: unified tokens, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e7e4e83bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Sign-in button: add size="sm" (min-h 34px) to align with the 38px theme toggle group in the navbar - UserBadge tooltip: replace reactive useQuery with one-shot ConvexHttpClient.query() so hover stats don't leave persistent subscriptions on the skills table - getHoverStats: replace unbounded .collect() with .take(500) to cap per-transaction reads for prolific users Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d880acccc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Replace all logo/favicon assets with the new lobster logo. All variants use a solid white background with rounded corners (12% radius). Updated assets: clawd-logo.png (512), clawd-mark.png (256), logo192.png, logo512.png, favicon.ico (multi-res), logo.svg. Added explicit favicon/icon/manifest link tags to __root.tsx. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ceebcc5bca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The CSS minifier converts #808080 to the named color 'gray', which Tailwind v4 rejects as an illegal theme token value. Use #818181 instead — visually identical but not a named CSS color. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84a48f28f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace <Container> wrapper with .section class so plugin detail uses the same max-width token (--page-max) and padding as skill detail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each primary nav tab gets a monochrome lucide icon (Wrench, Plug, Ghost) at 50% opacity for scanability. Icons brighten to 85% on hover and 100% when active. Secondary tabs unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests that import src/convex/client.ts crash at module load time
because getRequiredRuntimeEnv("VITE_CONVEX_URL") throws when the
env var is missing. Set a non-resolvable dummy URL for CI since
unit tests don't make real Convex calls.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 6 test files that mock convex/react now include a stub ConvexReactClient class so src/convex/client.ts can import without crashing at module load time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74b834b4dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2c856e68a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| expect(convexQueryMock).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| });}); |
There was a problem hiding this comment.
The file now ends with });});, which is a syntax error, so Vitest cannot parse this test module and bun run test will fail before executing the suite. This blocks CI and hides regressions in unrelated tests until the extra }); is removed.
Useful? React with 👍 / 👎.
|
|
||
| describe("skill route loader", () => { | ||
| it("allows numeric owner handles in beforeLoad", () => { | ||
| expect(() => runBeforeLoad({ owner: "123abc", slug: "weather" })).not.toThrow(); |
There was a problem hiding this comment.
runBeforeLoad is async, but these checks use expect(() => runBeforeLoad(...)).not.toThrow(), which only verifies synchronous throws. If beforeLoad rejects (e.g., notFound()), the promise rejection is not asserted here, so these tests can pass even when the guard behavior regresses.
Useful? React with 👍 / 👎.
|
|
||
| export const Route = createFileRoute("/$owner/$slug")({ | ||
| beforeLoad: ({ params }) => { | ||
| const isHandle = /^[a-zA-Z0-9_][a-zA-Z0-9_-]*$/.test(params.owner); |
There was a problem hiding this comment.
The new guard only allows owner handles matching [a-zA-Z0-9_-], but handle normalization elsewhere is permissive (trim/lowercase) and does not enforce this character set. Any existing owner handle containing other valid persisted characters (for example .) will now be rejected in beforeLoad with notFound() before canonical resolution, turning valid skill URLs into 404s.
Useful? React with 👍 / 👎.
1 participant
Summary
Comprehensive design system overhaul establishing a single token system and making React UI components the canonical styling authority. No product behavior changes — purely structural/visual consistency improvements across 48 files.
Phase 1 — Design Foundation
:root(status colors, form controls, interactive states, overlay, shadows, layout widths) with matching light-theme overrides@themeblock expanded from 24 → 80+ entries, mapping all tokens to Tailwind v4 utilities (spacing, typography, status, form, layout)ui/primitives migrated to tokens:input,textarea,label,select,badge,button,card,dialog,sheet,dropdown-menu,toggle-group— eliminating all hardcodedrgba()/hex values--accent-fgtoken introduced for guaranteed text contrast on accent backgrounds across both themes (fixes invisible button text in dark mode)--page-max(1536px),--page-narrow(900px),--page-min(320px) — all page/nav/footer max-widths standardized (was 6 different values: 900–1450px).btn,.tag,.card,.form-input,.form-label) with@deprecatedcomments — React components are canonicalPhase 2 — IA & Navigation
src/lib/nav-items.ts) eliminates 3-place navigation duplication (Header desktop, Header mobile, Footer)/$owner/$slugcatch-all validates owner handles, preventing confusing 404s on typos like/settignsUserBadge— lazy-fetches stats (published skills, stars, downloads) via newgetHoverStatsConvex queryTooltipProvideradded toAppProviders.tsxPhase 3 — UI Sweep
.btn→<Button>across 13 files (management, dashboard, index, about, plugins, souls, search, u/$handle, SkillHeader, SkillMetadataSidebar, SkillDiffCard, Header, SkillsResults).tag→<Badge>across 12 filesstyle={{}}→ Tailwind utilities across 17 files.carddivs →<Card>in detail/dashboard/search pagestext-white→text-accent-fgin 5 locations (was invisible on white accent in dark mode)<a>/<button>→<Button asChild>pattern (e.g., plugins "Download zip")min-width: var(--page-min)added to.app-shellNew files
src/lib/nav-items.ts— shared navigation config (NAV_ITEMS,FOOTER_NAV_SECTIONS,filterNavItems())src/lib/icon-sizes.ts—ICON_SM/ICON_MD/ICON_LGconstantsWhat's intentionally unchanged
.browse-page,.navbar,.home-hero, etc.) — complex responsive layouts kept in CSSgetHoverStatsquery)Test plan
/settignsin URL bar — should get clean 404, not broken pagenpx tsc --noEmit— zero type errorsgrep -r "rgba\|#[0-9a-f]\{3,6\}" src/components/ui/— should return minimal/zero matches