Conversation
…er (TASK-22142) The shipped picker stacked a title, three section headings and an Initials group over five-column rows, and its successor was blocked for the yellow bordered, shadowed tiles. This is the hand from the final mockup: eight dealt tiles plus the die, slot 1 the user's initial as a real letter pick, at least one earned card when any badge is held, no two tiles sharing an art file, and every character named under its sticker. A selected tile carries a 2px edge and no shadow; an earned tile carries the Earned badge and no coloured border. Rolling re-deals and never touches the pick. The earn toast can point the first hand at a badge. Adds the dice and menu icons to the registry.
…opy and share (TASK-22142) The avatar inside the pill made the pill twice the height of any button in the system and pushed the whole header off the design system. The avatar goes back above, as a bordered round button that opens the picker, and the pill keeps its shipped 40px chrome with two hit areas: the handle copies the profile link and says so, the icon shares as before. The verified check shows once, in the name row when there is one, else in the pill.
…inks the picker to its badge (TASK-22142) Nobody read the avatar chip as the door to the whole menu. A 40px round menu button, the nav circle recipe, replaces it with the same label and route. The toast's "Choose avatar" now carries the badge code so the first hand shows that badge's card.
…he copy-link label; drop the header and group keys (TASK-22142) Twenty basics get a name and a line in en, es-419 and pt-BR, with three Argentine overrides in es-AR. The keys that only the old header and groups rendered are gone.
…-22142) The hand-drawn 36px die put a radius and six-pixel pips outside the spacing and radius scales and tripped the DS ratchet. The roll tile now shows the single five-pip die icon at 24, the one size the icon scale allows here, and keeps its dashed edge, white fill, label and one-turn spin.
… is gone (TASK-22142)
…nk clears on close (TASK-22142) A button that is not a radio cannot be a direct child of a radiogroup; the group now wraps the eight tiles only and the die is its sibling in the grid. The badge parameter from the earn toast used to outlive the first open, so every later hand kept preferring that badge; closing the drawer clears it.
…d control (TASK-22142) The framed avatar carried the hard shadow without the press that goes with it, and the split pill had lost the press the shipped pill shows. The avatar is the stroke Button now, so its press is the Button's own; the pill frame presses as one surface on either segment, the shipped pill's values. The share glyph is the pill's trailing 16px icon again, not a 32px box, with the 44px hit area kept.
…ASK-22142) A Link around a Button is two tab stops and nested interactive content. The link now carries the stroke button recipe itself, same circle, icon, label, hit area and press, with one focus stop and the honest role for a control that navigates.
…, names that survive 320px (TASK-22142) A deep link from the earn toast opened the picker before the user had loaded, so the hand dealt with no badges and an empty initial; the deal now waits for the user and happens once per open. The focus ring was clipped by the scroll box on the first and last rows. Sixteen-character names truncated to eleven on a 320px screen; names wrap to two lines like their line, with a fixed box so every tile keeps the same height.
|
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: 7252.27 → 7255.59 (+3.32) 🆕 New findings (30)
…and 10 more. ✅ Resolved (29)
…and 9 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 15 screens moved24 of 68 shots changed · 44 identical · baseline
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 review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Exact-head CI is green and the picker, profile, and home rebuild is broadly coherent. Two minor interaction edge cases remain: a mixed badge batch can deep-link the wrong badge, and the share segment's expanded hit box overlaps the copy segment.
Findings
-
MINOR · src/components/Badges/BadgeEarnToast.tsx:97 · Deep-link the badge that actually unlocked avatars
For a coalesced batch whose newest badge has no art but an older badge does (for example PRODUCT_HUNT followed by SHHHHH),avatarCountis non-zero because SHHHHH has avatars, but this routes withbadge=PRODUCT_HUNT.dealHandfinds no preferred PRODUCT_HUNT key and falls back to any avatar from every badge the user holds, so it can deal an older badge's art and show none of the newly unlocked SHHHHH art. Pick the newest pending code for whichbadgeAvatarKeys([code])is non-empty, route with that code, and cover the mixed art/no-art batch. -
MINOR · src/components/Profile/components/ProfileHeader.tsx:182 · Keep the share hit box out of the copy segment
The 16px share button'safter:-inset-3.5extends its clickable pseudo-element 14px in every direction. With only the 4pxml-1gap between siblings, the later share button overlays roughly 10px of the copy button, so tapping near the right edge of the handle can open sharing instead of copying even though these are meant to be independent hit areas. Reserve a real non-overlapping 44px share segment, or expand only vertically while allocating the horizontal target, and cover clicks at the segment boundary.
Checked clean
- Confirmed the detached worktree HEAD, trusted author, base ref, supplied base SHA, and merge base exactly match the requested review target.
- Reviewed the hand-dealing algorithm, manifest-backed unlock checks, initial-letter behavior, deep-link lifecycle, save serialization, failure fallback, keyboard navigation, and focused tests.
- Reviewed the self-profile identity column, copy/share analytics and failure handling, counterparty branches, home menu navigation, icon registrations, fixtures, and locale changes.
- Checked the repository button CSS and the canonical design guidance for icon sizing, pressed states, and 44px touch targets.
- Exact-head CI completed successfully, including unit, typecheck, eslint, format, native-export, design-system lint, screenshots, and aggregate ci-success checks. The detached worktree has no installed Jest binary for an additional local focused run.
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: 340d8ac8f2e5 · Context: repo, design · Took 19m
…off the handle (TASK-22142) Both from Chip's review of 340d8ac. Most badges ship no avatar art, so a coalesced earn toast can pair a newest badge with none and an older one with three. The link named the newest, and `prefer` on a code with no art falls back to any avatar already held — so the user tapped through to a hand holding none of what was just unlocked. Name the newest code that has art instead. The share glyph reaches 44px through a 14px `after:` inset, but sat 4px from the handle, so the last ~10px of the url opened sharing instead of copying. 16px of margin puts the two hit areas side by side with nothing shared, and stays on the spacing scale.
dev's #3013 split the badge-earned toast into two sequential toasts and dropped the badge from the picker link; dev's #2953 lowered the ds-lint baseline. Kept both: the avatar toast stays a delayed second toast, and its link keeps the badge code — the newest one that actually has art. The tighter `retypedCardLiteral` baseline no longer had room for the picker tile, which retyped Card's surface because a `role="radio"` must be a `<button>` and `Card` is a `div`. Card now exports that surface and both use it.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
P1 and P2 are fixed at this exact head, and all exact-head checks are green. One new minor issue remains: the new copy segment is only 40px tall while the adjacent share segment correctly expands to the 44px touch-target floor.
Findings
- MINOR · src/components/Profile/components/ProfileHeader.tsx:154 · Extend the copy segment to the 44px touch target
The pill frame ish-10, and this new button is onlyh-full; unlike the adjacent share button, it has no pseudo-element extending its hit area. A touch in the 2px strip immediately above or below the handle falls inside the standard 44px target envelope around the visible 40px pill but does nothing. Make the copy button relative and extend it vertically by 2px without growing it horizontally, which preserves P2's non-overlap fix.
Checked clean
- Confirmed the detached worktree head, trusted author, dev base ref, supplied base SHA, and merge base all match the request.
- P1 is fixed: the toast filters the newest-first badge codes to those with avatar art, deep-links the first survivor, and covers the supplied PRODUCT_HUNT plus SHHHHH case.
- P2 is fixed: the share button now has a 16px left margin against a 14px horizontal hit-area expansion, leaving the copy and share targets disjoint; the regression test pins that arithmetic.
- Traced the hand deal, art-path deduplication, preferred-badge draw, current-pick retention, cold auth resolution, query cleanup, keyboard roving, and serialized save/fallback paths; no additional correctness defect survived.
- Reviewed the self-profile avatar and copy/share split, counterparty and public-profile branches, home menu replacement, localization overlays, analytics success gating, and changed design-system primitives.
- Checked the policy-branch API sibling: letter picks are accepted and badge picks are revalidated against held badges; the avatar pools used by this UI remain compatible.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, native export, design-system lint, analysis, visual snapshots, deploy preview, provenance, and aggregate CI.
- The exact diff passes git diff --check. Focused local Jest could not run because the detached worktree has no installed dependencies.
- Security, privacy, and money: the URL badge parameter only changes client-side deal preference, and persisted avatar picks remain server-authorized; no new credential, amount, or privilege surface was introduced.
- Slop and history: reviewed all commits and changed hotspots; no actionable duplication, dead runtime code, speculative abstraction, or architecture drift survived beyond the touch-target finding above.
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: 0bf49bd05e60 · Context: repo, ci, design, api-sibling · Took 25m
…SK-22142) From Chip's review of 0bf49bd. The pill is 40px and the handle was `h-full`, so the 2px strip above and below it did nothing while the share glyph beside it already answered there. It grows through the same `after:` pseudo-element — vertically only, so the two hit areas stay disjoint.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the pinned head: the three prior touch/deep-link findings are fixed, and the rebuilt avatar picker, profile identity block, and home navigation have no remaining actionable defects.
Checked clean
- P1 fixed: coalesced badge batches now select the newest code that actually ships avatar art, so [PRODUCT_HUNT, SHHHHH] deep-links SHHHHH.
- P2 fixed: the share control's 16px left margin exceeds its 14px pseudo-element growth, keeping its hit area out of the copy segment.
- P3 fixed: the 40px copy segment now grows by 2px vertically on each edge to reach 44px without widening into the share target.
- Avatar hand composition, art-path deduplication, preferred-badge dealing, current-pick retention, cold auth resolution, URL cleanup, radio navigation, and serialized save/refetch failure paths.
- Profile self/counterparty branching, copy/share attribution, clipboard failure behavior, analytics firing, truncation, pressed states, and disjoint 44px touch targets.
- Home menu navigation, single interactive element semantics, haptics, rewards visibility, icon registration, shared card surface extraction, and localized cast/copy labels.
- The sibling API policy branch accepts letter. keys for every user and validates badge avatars against held badge codes.
- Exact-head required CI is green, including unit, typecheck, eslint, format, ds-lint, native-export, author, and baseline gates; advisory ds-shots was still in progress.
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: 70fc5833ca6d · Context: repo, other_repo · Took 16m
|
@innolope-dev — you blocked #2989 on 5 September, so this is the one to look at instead. #2989 is closed; this replaces it. Your four points are quoted and answered one by one in the description, under Slava's points, answered. Short version: the home avatar is your option A, a labelled 40px menu button with a 44px hit area; the pill is back to its shipped One thing I did not decide for you, in One open question the 430px capture raises: a user holding a single badge with three unlocks can be dealt two or three of them in the same hand. Different art, different names, so the dedupe is doing its job — but they share a line, the badge's name, and at 320px they read as the same sticker. That is your third objection reached by a different route. Capping earned tiles at one per badge is a few lines in Eight DS rows ship flagged, not changed in the same section — the dashed roll border and the 16px inline share glyph are the two most likely to matter to you. @kushagrasarathe is the requested reviewer; no need to approve, an opinion on the two above is what would help. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the pinned head: the three prior touch/deep-link findings are fixed, and the rebuilt avatar picker, profile identity block, and home navigation have no remaining actionable defects.
Checked clean
- P1 fixed: coalesced badge batches now select the newest code that actually ships avatar art, so [PRODUCT_HUNT, SHHHHH] deep-links SHHHHH.
- P2 fixed: the share control's 16px left margin exceeds its 14px pseudo-element growth, keeping its hit area out of the copy segment.
- P3 fixed: the 40px copy segment now grows by 2px vertically on each edge to reach 44px without widening into the share target.
- Avatar hand composition, art-path deduplication, preferred-badge dealing, current-pick retention, cold auth resolution, URL cleanup, radio navigation, and serialized save/refetch failure paths.
- Profile self/counterparty branching, copy/share attribution, clipboard failure behavior, analytics firing, truncation, pressed states, and disjoint 44px touch targets.
- Home menu navigation, single interactive element semantics, haptics, rewards visibility, icon registration, shared card surface extraction, and localized cast/copy labels.
- The sibling API policy branch accepts letter. keys for every user and validates badge avatars against held badge codes.
- All exact-head CI completed successfully, including unit, typecheck, eslint, format, ds-lint, native-export, ds-shots, author, baseline, and aggregate gates.
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: 70fc5833ca6d · Context: repo, other_repo · Took 7m
|
Ruling from Konrad (7 Sep): several avatars from the same badge may appear in one hand. That is by design, so the "one card per badge" question in the description is closed with no cap; the art differs and the line under each tile names the badge. @innolope-dev this PR is yours to review and merge; Kushagra can weigh in on the DS rows if you want a second pair of eyes. |
Rebased replacement: #3047. It preserves the eight-tile hand and home hamburger, with simplification and regression fixes. Repository rules block force-pushes, so this branch retains its original history.
Summary
The avatar picker, the profile identity block and the home top nav, rebuilt to the final mockup. This supersedes #2989, which Slava blocked on 5 September; his four points are quoted and answered below.
The picker is a hand. The sheet has no header — it is eight dealt tiles and a die, three across. Slot 1 is always your own initial and saving it writes a real
letter.<x>pick. The rest come fromdealHand: at least one earned avatar when you hold any badge, your current pick kept in place, the rest from the twenty basics, deduped by art path so no two tiles draw the same picture. The die deals a new hand and never changes the pick. Every tile carries its character's name and line, so the sheet reads as a cast rather than a grid of stickers. The 26-letter Initials group from #2998 is gone — slot 1 replaces it.Profile identity is a column. The 64px round avatar button sits above the share pill, and the pill stays at its shipped 40px. The pill is two hit areas: the handle copies
peanut.me/<handle>with a toast and aPROFILE_LINK_COPIEDevent, the trailing glyph opens the share sheet.Home loses the avatar chip. A 40px round menu button linking to
/profiletakes its place. The sticker is identity; identity lives on the profile and in the picker, and up in the nav a lone sticker read as decoration.Copy: 20 cast names and lines plus the earned, roll and copy-link labels in
en,es-419andpt-BR, with three Argentine overrides ines-AR. Seven keys the rebuild stopped rendering are deleted.Task
TASK-22142— Build badge-linked avatar system v2. Supersedes #2989.Slava's points, answered
His option A. The chip and its chevron are gone. In their place is a bordered 40px circle with a menu icon,
aria-label="Open your profile", a 44px hit area and the stroke press — a control, not a picture.src/features/home/views/HomeTopNav.tsx.The avatar came out of the pill. It is now a 64px round button 8px above, and the pill is back to the
h-10it ships with today.src/components/Profile/components/ProfileHeader.tsx.The hand is deduped by art path, not by key, so two badge codes that share a picture can never both be dealt. Letters live outside the deck, so slot 1 can never repeat a tile. All 94 files under
public/avatars/were checked byte-distinct, so path dedupe is sufficient in fact and not only in theory. A 20-seed test asserts seven distinct art paths on every deal.src/components/Avatar/avatar.utils.ts.No tile carries a shadow. The selected tile is
border-2and nothing else. The only two shadowed controls added are the avatar button and the pill — both of them, so there is no one-element case — and both areshadow-4with the DS pressed state.Also gone with the rebuild: the shipped picker's cropped buttons, and "use initials" that did nothing. Slot 1 is the initial and it saves.
Risks / breaking changes
peanut-api-tsacceptsletter.<a-z>avatar keys ondevsince api#1530 (5 Sep), but not onmainyet. Until that API release ships, a letter pick on production is rejected and falls back to the device-local mirror — the behaviour that already ships from #2998 — with no error toast and no lost pick, and it promotes itself to the durable server copy on the next accepted write. Badge and basic picks are unaffected. The next dev → main release must carry both repos.This branch carries a merge of
dev. Two siblings landed while it was open: #3013 split the badge-earned toast into two sequential toasts and dropped the badge from the picker link — the merge keeps dev's two-toast shape and the badge code — and #2953 lowered theds-lintbaseline, which theCARD_SURFACEextraction above answers.Blast radius is otherwise three surfaces: the profile header self branch, the avatar picker drawer, the home top nav. The counterparty and public-profile branches of
ProfileHeaderare untouched and pinned by tests./profile/editand the public profile still render the 96px avatar with no button.Removed exports (
offerBasics,letterAvatarKeys) have zero remaining references.useHomeFlowno longer computes an avatar key for a chip that is gone.QA
No backend or provider state is needed — the picker, the pill and the nav all render from the user object.
/profile— the avatar sits above the pill; tap it to open the picker.?badge=is gone from the URL after you close it.PROFILE_LINK_COPIED; tap the glyph → the share sheet andREFERRAL_CTA_CLICKED./home— a menu button top-left, no sticker, no chevron; it goes to/profile.Automated: 41 suites / 464 tests over
src/components/Avatar,src/components/Profile,src/features/home,src/components/Badges,src/i18n,src/dev/fixtures. Full suite green (6080 passing).tsc --noEmitclean.ds-lint-counts --checkgreen — no metric increased,iconOffScalefell 70 → 68.Screenshots
ds-shotsfixture captures of the commit under review — no backend, the fixture answers every call. The full run is in the visual diff comment: 10 screens moved, 18 of 68 shots changed, baselineecd7c0b→ head0bf49bd.Shown at 320px, the narrowest phone we support and the width where the predecessor's copy was cut off.
dev)The whole hand, 430px — eight tiles, the die, the selected tile's
border-2, the Earned tags:Assets live on the
pr-assets-3014branch and are deleted when this PR closes (pr-assets-cleanup.yml).Several avatars from one badge in a hand: by design
That hand deals two Bug Whisperer avatars, Shell and Beetle, and an earlier deal of the same fixture drew all three. Each is a different art file with its own name; the dedupe is by art path and is doing its job. Konrad ruled on 7 September that a hand may carry several avatars from the same badge, so there is no cap and nothing to change here.
Design notes / accepted trade-offs
Fixed from Chip's review of
340d8ac8f: the earn toast now deep-links the newest badge that actually has art (38 of 54 codes ship none, so a batch pairing an artless newest with an older unlock sent the user to a hand holding nothing they just earned); and the share glyph's 44px hit box no longer reaches back over the handle, so a tap on the end of the url copies rather than shares. Both have tests.Fixed before that, from the DS, behaviour and adversarial passes: the die became the on-scale 24px icon; the
radiogroupwraps the eight tiles only (display:contents) so the die is not a non-radio child; the?badge=param clears on close; the avatar button and the pill both press; the share glyph is inline rather than an icon-only button; a cold-loaded deep link re-deals once the user resolves; the grid haspy-1so the 3px focus ring is not clipped by the scroll box; tile names wrap to two lines so 320px keeps them whole; the home menu button is oneLinkwith the button recipe rather than aButtoninside aLink; the home flow's orphanedavatarKeyis gone.Flagged, not changed — each one needs a ruling, none of them is a build error:
border-[1.5px] border-dashed border-border-defaultdivide-dashed).--border-width-s: 1.5pxexists inglobals.cssbut is deliberately kept out of@theme, so there is no utility to write it with.after:inset.line-clamp-2 h-8— two linestext-green-500foreground-success. It is the classUserHeaderandVerifiedUserLabelalready use. Which green is the verified check?StatusBadge status="custom"customhas no board row (a pre-existing open conflict in design.md). Renders the mockup's exact badge colours; no coloured border, no yellow.rotate-360/rotate-0by turn parityrotate-<number>off the stock steps. Valid Tailwind v4, and it alternates turn direction — an inline style would push theinlineStyleratchet.className="contents"display:contentsdropped the a11y node in Safari before 15.5. On iOS 15.0-15.4 the tiles still work; they stop being announced as one group.NavHeader<Link><Button>HomeTopNavno longer nests them;NavHeaderis not this PR's file.Deliberate drift from the mockup, so nobody puts it back: tile side padding 6px →
px-1; the tile line 11/14px →text-body-xs12/16; the pill's right inset 8px →pr-4, the value it ships with; the pill renderspeanut.me/handlein one bold run where the mockup greys the domain — that is dev parity, not new.Cardnow exports its surface asCARD_SURFACE. The picker tile needs that chrome on a<button>(arole="radio"cannot be thedivCardrenders), and it used to retype the three classes — the exact duplication theretypedCardLiteralratchet counts, whichdevtightened under us while this PR was open. One string, two users, count back wheredevput it.Follow-ups, not in this PR — code: export the
NavHeadercircle recipe instead of duplicating half of it inHomeTopNav; point thehome-avatardev fixture at/profileor delete it (it routes to/home, which renders no avatar now); deleteavatarPool, which has no production caller; fix theUserHeaderanddev/ds/auditcomments that still name the home sticker.Follow-ups, not in this PR — mono docs (content and code never travel together, so none of this is here):
design/design.md:124still says user avatars are alwaysAvatarWithBadgewith a circle and a 1px border, which #2929 already made false and this PR compounds;design/design.md:461's figma↔code map has no row forUserAvatar.tsxor the picker; the open-conflicts table wants a row for the dashed container border, and one for a pressed frame that holds two independent hit areas;product/support-answers/rewards-points-badges.mdnever says a badge unlocks avatar art, so support under-answers it. No legal impact — the one new event is a PostHog capture on a local clipboard write, and PostHog and "taps and interactions" are already named in the privacy policy (§2, §4, §6).