style: tokenize rawHex remainders + FAQs type migration (DS KR1 ratchet) - #2953
style: tokenize rawHex remainders + FAQs type migration (DS KR1 ratchet)#2953kushagrasarathe wants to merge 10 commits into
Conversation
…cots Yellow body -> var(--color-action-secondary); the offline banner's near-miss #F9F4F0 -> var(--color-background-page) (#FAF4F0) — a deliberate one-shade drift fix onto the token.
AvatarWithBadge inlineStyle/textColor land in CSS style props, so the hex literals become var(--color-background-icon-bubble-yellow/green) and var(--color-foreground-primary). UserCard's wallet avatar was off-token gold #FFD700 — snapped to the DS yellow (visible drift fix, flagged in the PR body for veto).
…ble) - rewards InvitesGraph backgroundColor -> var(--color-background-default); force-graph applies the prop as canvas.style.background, so a css var resolves fine - the two arbitrary offset shadows now spell var(--color-shadow-primary) instead of #000; kept the arbitrary form (0.12rem has no DS utility, and shadow-4 would add a dark: variant the capture asset never had) - Claimed.view icon circle: off-token #FFCC00 -> bg-background-icon-bubble-yellow (visible drift fix, flagged in the PR body)
Stock text sizes -> the type ramp by role: learn-more link text-base/md:text-lg -> text-body-m/md:text-body-l; question summary text-lg/md:text-xl (+font-extraBlack) -> text-heading-card/md:text-heading-xs (tokens carry their own weight, stacked weights are banned); the + glyph text-3xl -> text-heading-m; answers text-lg/md:text-xl semibold -> text-body-l (no 20px body step exists — answers settle at 18px regular). Section bg near-miss #F9F4F0 -> bg-background-page (drift fix). Used by MerchantLandingPage, Marketing/mdx FAQ, LandingPage/faq.
rawHex 27 -> 8 (files 20 -> 5), stockTextSize 8 -> 1, nonDsClassesInViews 16 -> 15. Debt only goes down.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 7096.21 → 7092.74 (-3.47) 🆕 New findings (9)
✅ Resolved (9)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 7 screens moved14 of 62 shots changed · 48 identical · baseline
new screens (3)
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean review: the design-system tokenization and FAQ type migration preserve the intended behavior, with the documented visual drift only.
Checked clean
- Confirmed the detached worktree head, exact base merge-base, trusted author, and PR metadata match the supplied review target.
- Reviewed all 16 changed files for token-value equivalence, CSS-variable consumers, FAQ typography changes, baseline ratcheting, and regression paths.
- Verified ForceGraph 1.51.0 applies backgroundColor through canvas.style.background, so the CSS variable resolves rather than being parsed as a canvas color.
- Exact-head CI passed ds-lint, ds-shots, format, eslint, typecheck, unit, analyze, native-export, and preview checks.
- Correctness, security, adversarial, and slop passes found no reachable defect or new security, privacy, or money surface.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: d1f08e016641 · Context: repo · Took 6m
Masked state only — the reveal needs a passkey step-up no fixture can answer, and the masked '****' uses the same span as the digits.
text-6xl leading-none font-extrabold -> text-heading-big-input (the token carries 700 + its own line-height; stacked weights are banned). Actual delta: 48px/800 -> 52px/700 — this repo's 6xl is overridden to 3rem/3.25rem. The loading skeleton is already h-[52px], so the old 48px text made the row content jump size on load; digits at 52px now match the skeleton exactly. Row geometry is held by the fixed h-[52px] wrapper either way. Baseline: stockTextSize 1 -> 0.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the pinned head. The token substitutions preserve intended CSS behavior, the deliberate typography and color drift changes are internally consistent, and no correctness, security, or maintainability defect survived adversarial review.
Checked clean
- Confirmed the detached worktree HEAD, merge base, trusted author, and dev base exactly match the supplied review coordinates.
- Traced every changed dynamic color through its consumer; AvatarWithBadge accepts CSS color values, SVG presentation attributes accept the tokens, and force-graph 1.51.0 assigns backgroundColor to canvas.style.background rather than a drawing context.
- Verified the replacement color, shadow, background, and typography tokens exist and that value changes match the PR's declared visual drift.
- Checked CardPinScreen's masked, loading, and revealed geometry plus replay-redaction boundary; the new fixture contains fake masked-state data only.
- Ran the design-system lint ratchet and diff whitespace check successfully at this head.
- Reviewed exact-head CI: code, type, lint, format, unit, visual, export, provenance, and aggregate gates passed; the separate review dispatcher failed only because its worker socket was temporarily unavailable, not because of repository code.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 988bce20fb28 · Context: repo · Took 5m (queued 6m)
…rison Kush wants both candidates side by side: heading-xl is 42px/800/48px line vs big-input's 52px/700/64px. Both are ramp tokens, counts unchanged. Loading skeleton stays h-[52px] — if heading-xl is the pick, deciding whether the skeleton follows the 48px line box is called out in the PR body rather than resized silently.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean review. The design-token migrations preserve compatible CSS/SVG/canvas behavior, the deliberate visual changes are scoped, and the PIN typography change has fixture coverage.
Checked clean
- Confirmed the detached worktree head and merge base match the supplied SHAs, and verified the trusted PR metadata targets dev.
- Reviewed all changed token substitutions across inline SVG attributes, React style props, Tailwind arbitrary shadows, avatar colors, and the invite graph canvas background.
- Checked the FAQ type-ramp migration and CardPinScreen geometry, including the new masked-state card PIN fixture and active-card response shape.
- Ran git diff --check and the design-system lint ratchet locally; both passed.
- Exact-head unit, typecheck, format, eslint, native-export, analyze, and aggregate ci-success checks passed; ds-shots was still in progress when reviewed.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 98e4d566de74 · Context: repo · Took 5m
1 participant
Summary
DS cleanup pass ruled by Kush after reviewing the exact rawHex remainders post-#2946 (KR1 metrics of
scripts/ds-lint-counts.mjs). Tokenizes every fixable raw hex, migrates the FAQs panel to the DS type ramp, rewords 4 comments the rawHex regex false-positives on, and ratchets the lint baseline down.Task: TASK-21446 (DS 02 — lint count baselines / KR1 ratchet)
Changes
Comment false positives (no behavior change)
qr-pay/page.tsx#920-> "PR 920",app/page.tsx#418-> "error 418",not-found.tsx#310-> "error 310",0_Bruddle/Card.tsxquoted#161616-> "near-black border-default"Inline SVG fills -> tokens
Global/OfflineScreen: mascot yellow ->var(--color-action-secondary); banner#F9F4F0->var(--color-background-page)(drift fix, see below)Global/BackendErrorScreen: mascot yellow ->var(--color-action-secondary)Dynamic color logic -> css vars (all consumers land in CSS style props, verified)
AddWithdraw/AddWithdrawCountriesListx2,Global/Contributors/ContributorCard(bg + textColor),Global/PeanutActionDetailsCard(#29CC6A->--color-background-icon-bubble-green,#FFC900->--color-background-icon-bubble-yellow),User/UserCard(#FFD700-> yellow token, see below)One-offs
rewards/page.tsx:InvitesGraph backgroundColor->var(--color-background-default)— force-graph applies the prop ascanvas.style.background, so a css var resolvesAddMoney/views/CryptoDeposit.view+Card/CardRejectionScreen: offset-shadow hex ->var(--color-shadow-primary)inside the arbitrary class (kept arbitrary: 0.12rem has no DS utility;shadow-4would add adark:variant the capture asset never had)Claim/Generic/Claimed.view:bg-[#FFCC00]->bg-background-icon-bubble-yellow(drift fix)Global/FAQs:bg-[#F9F4F0]->bg-background-page(drift fix)FAQ type migration (
Global/FAQs— used by MerchantLandingPage, Marketing/mdx FAQ, LandingPage/faq)text-base/md:text-lg->text-body-m/md:text-body-ltext-lg font-extraBlack md:text-xl->text-heading-card md:text-heading-xs(tokens carry weight; stacked weights are banned, so extraBlack 1000 becomes 700/800)+glyphtext-3xl->text-heading-mtext-lg leading-6 font-semibold md:text-xl->text-body-l(no 20px body step in the ramp — answers settle at 18px regular at all widths)CardPinScreendigits:text-6xl leading-none font-extrabold->text-heading-xl(Kush-approved follow-up; three-way comparison in the keep-or-revert section below). Plus acard-pinfixture so ds-shots covers the PIN screen (masked state) per PRMetrics (node scripts/ds-lint-counts.mjs)
Baseline rebased downward (
scripts/ds-lint-baseline.json); all other ratchet metrics unchanged,--checkgreen.Remaining rawHex (all ruled leave-alone):
(setup)/layout.tsxCapacitor StatusBar literal,Global/Icons/Icon.tsx:181var fallback,Global/Icons/{invite-heart,bulb}icon artwork,Global/ConsoleGreetingconsole.log styling.PIN size: keep or revert? (three-way)
CardPinScreendigits moved off the last stock text size. Two ramp candidates were tried — the branch currently ships heading-xl; say the word to switch back to big-input or revert to the original.text-6xl leading-none font-extraboldtext-6xlto 3rem/3.25rem — not the stock 60pxtext-heading-big-inputh-[52px]skeleton height exactlytext-heading-xl(current)h-[52px]skeleton/wrapper should follow the 48px line box (left untouched here, not resized silently)Row geometry is held by the fixed
h-[52px]wrapper in all three (the eye button does not move). Mock data only — captured on a local throwaway route (not committed) that patchesrainApi.getCardPinto return1234, over thecard-pinfixture (fake card, last4 4242). 375x667.The
pr-assets-2953branch is deleted after merge.Visual changes — review these
Five deliberate visible shifts (the rest is value-identical tokenization) — the PIN change above, plus:
#FFD700has no token; snapped to#FFC900. Veto point — say the word and it reverts.#FFCC00->#FFC900(subtle).#F9F4F0-> token#FAF4F0(imperceptible).ds-shots CI diff: the drift fixes SHOULD produce small diffs on fixture-covered screens; anything else diffing is a bug.
Risks / breaking changes
QA
pnpm prettier --check .clean,npm run typecheckclean,npm test438 suites / 5603 tests greennode scripts/ds-lint-counts.mjs --checkgreen (no metric increased)Design notes / accepted trade-offs
--color-background-icon-bubble-yellow(the semantic role token, same value asaction-secondary); SVG mascot artwork uses--color-action-secondary.getColorForUsername) is hand-rolled in UserCard, ContributorCard, PeanutActionDetailsCard and AddWithdrawCountriesList (candidate for one shared helper), andAddWithdrawCountriesListhas a constantcolor: bank ? 'black' : 'black'ternary. Follow-up material, not touched here.d1f08e0— no blocking findings, 0 inline comments, third opinion (claude-opus) 0 findings. (It fired late, on ready-for-review rather than open — bot was recovering from the earlier outage.)