Conversation
Two sections, a title, a description and three buttons were the wrong sheet for a pick that is tap-and-done. The picker is one 3×3 hand now: the initial always in slot 1, seven dealt by dealHand — at least one earned badge avatar when the user holds a badge with art, the current pick kept, the rest from the basics — and the die in slot 9. Rolling deals again and never touches the pick, so "use my initial" is a tile, not a button. The badge-earned toast passes its badge along (?badge=CODE) so the first hand holds the new art.
The avatar, the own name and the share pill said the same thing three times on one's own profile. They are one 72px pill now, three hit areas inside one border and never a button nested in a button: the avatar opens the picker, the handle copies peanut.me/handle, the share icon runs the existing share flow. Public profiles and the edit screen keep the stacked header.
|
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: 7218.03 → 7222.48 (+4.45) 🆕 New findings (30)
…and 10 more. ✅ Resolved (27)
…and 7 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
/chip review |
🖼 Visual diff — 9 screens moved13 of 66 shots changed · 53 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. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The avatar hand and self-profile pill match the intended flow, but a fast close/reopen can temporarily lose the selected tile and the new copy segment does not expose its action to assistive technology.
Findings
-
MINOR · src/components/Avatar/AvatarPicker.tsx:90 · Deal from the pending pick when reopening
If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it,pendingstill identifies the visibly selected avatar whilesavedis the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal frompickhere (and cover the close/reopen-during-save case) so the stated current-pick guarantee also holds during an in-flight save. -
MINOR · src/components/Profile/components/ProfileHeader.tsx:106 · Name the profile-link copy action
The pill's avatar and share segments expose action names, but this new button's accessible name is only the rendered URL. A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localizedaria-labelsuch asCopy profile link(optionally including the handle) and assert that action name in the header test.
Checked clean
- Confirmed the detached worktree head, trusted author, base ref, supplied base SHA, and merge base all match the request.
- Reviewed hand construction for no-badge, earned-avatar, current-pick, preferred-badge, reroll, save-failure, and serialized-save paths.
- Reviewed badge-toast deep-link creation and profile query-state cleanup, including the first-hand preference contract.
- Reviewed the self-profile pill's avatar, copy, share, verification, truncation, analytics, focus, and public-profile fallback behavior against the project handoff and design rules.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, native export, design-system lint, analysis, preview, and visual snapshots.
- The exact diff passes
git diff --check; focused local Jest was unavailable because the detached worktree has no installed dependencies. - Security and slop passes found no credential, authorization, injection, unsafe data-flow, or additional maintainability defect worth reporting.
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: 1039407448a6 · Context: repo, design, project · Took 15m
| useEffect(() => { | ||
| if (open) setOffer(offerBasics(saved)) | ||
| // deal once per open; the pick joins the row by being picked from it | ||
| if (open) setHand(dealHand(saved, unlocked, { prefer })) |
There was a problem hiding this comment.
MINOR: Deal from the pending pick when reopening
If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it, pending still identifies the visibly selected avatar while saved is the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal from pick here (and cover the close/reopen-during-save case) so the stated current-pick guarantee also holds during an in-flight save.
There was a problem hiding this comment.
Fixed in 46f4255. The reopen effect now deals from pick (which prefers pending) instead of saved, so a close/reopen during an in-flight save keeps the visibly selected avatar in the hand. Covered by a new test that fails against the old line: deals from the pending pick when reopened during an in-flight save.
| {pillVisible && ( | ||
| <span className="flex h-full shrink-0 items-center pl-3">{ownAvatar('small')}</span> | ||
| )} | ||
| <button |
There was a problem hiding this comment.
MINOR: Name the profile-link copy action
The pill's avatar and share segments expose action names, but this new button's accessible name is only the rendered URL. A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localized aria-label such as Copy profile link (optionally including the handle) and assert that action name in the header test.
There was a problem hiding this comment.
Fixed in 46f4255. Added an sr-only action label to the copy segment (global.copyToClipboard.copyProfileLink, en/es-419/pt-BR — es-AR carries no delta for that namespace), matching the pattern the sibling share segment already uses in this file. Accessible name is now Copy profile link peanut.me/<handle> Verified, so the action, the url and the verified state are all exposed rather than the aria-label replacing them. Asserted in names the copy action, the url and the verified state in the accessible name, and the suite-wide copyButton() matcher now requires the action name too.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Both previously raised minor findings remain at this exact head: a fast close/reopen can temporarily lose the selected tile, and the profile-link copy segment still does not expose its action to assistive technology.
Findings
-
MINOR · src/components/Avatar/AvatarPicker.tsx:90 · Deal from the pending pick when reopening
This remains present at the exact head. If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it,pendingstill identifies the visibly selected avatar whilesavedis the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal frompickhere and cover the close/reopen-during-save case. -
MINOR · src/components/Profile/components/ProfileHeader.tsx:106 · Name the profile-link copy action
This remains present at the exact head. The pill's avatar and share segments expose action names, but this button's accessible name is only the rendered URL. A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localizedaria-labelsuch asCopy profile linkand assert that action name in the header test.
Checked clean
- Confirmed the detached worktree head, trusted author, base ref, supplied base SHA, and merge base all match the request.
- Rechecked P1 against the pending/saved state model, serialized save loop, reopen effect, reroll path, and existing AvatarPicker tests; no guard or test covers the close/reopen race.
- Rechecked P2 against the three pill segments and ProfileHeader tests; avatar and share have action names while the copy segment still exposes only the URL.
- Reviewed the complete avatar-hand, badge deep-link, query cleanup, self-profile pill, public-profile fallback, localization, correctness, security, adversarial, and slop surfaces; no additional actionable finding survived.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, native export, design-system lint, analysis, preview, and visual snapshots.
- The exact diff passes
git diff --check; focused local Jest remains unavailable because the detached worktree has no installed dependencies.
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: 1039407448a6 · Context: repo, design, project · Took 7m
| useEffect(() => { | ||
| if (open) setOffer(offerBasics(saved)) | ||
| // deal once per open; the pick joins the row by being picked from it | ||
| if (open) setHand(dealHand(saved, unlocked, { prefer })) |
There was a problem hiding this comment.
MINOR: Deal from the pending pick when reopening
This remains present at the exact head. If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it, pending still identifies the visibly selected avatar while saved is the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal from pick here and cover the close/reopen-during-save case.
| {pillVisible && ( | ||
| <span className="flex h-full shrink-0 items-center pl-3">{ownAvatar('small')}</span> | ||
| )} | ||
| <button |
There was a problem hiding this comment.
MINOR: Name the profile-link copy action
This remains present at the exact head. The pill's avatar and share segments expose action names, but this button's accessible name is only the rendered URL. A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localized aria-label such as Copy profile link and assert that action name in the header test.
The "Earned" chip sat on the top of the sticker in every locale and covered its whole top edge in pt-BR, so the tile keeps an XL band above the art. Arrow keys drifted a column on the way up and down, because eight tiles in three columns are not a multiple of the column count; up and down now stay in their column and wrap within it. A pick this bundle's manifest does not know is no longer dealt as a second, checked "initial" tile. The pill asks ShareButton for no shadow instead of covering its shadow class from the outside, and the verified tick carries its name for screen readers. The picker tests deal a fixed hand rather than pinning Math.random to the deal's pool order.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Both previously raised minor findings remain at this exact head: a fast close/reopen can temporarily lose the selected tile, and the profile-link copy segment still does not expose its action to assistive technology.
Findings
-
MINOR · src/components/Avatar/AvatarPicker.tsx:90 · Deal from the pending pick when reopening
If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it,pendingstill identifies the visibly selected avatar whilesavedis the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal frompickhere and cover the close/reopen-during-save case. The supplied reply does not dispute this reachable state or move the fix to follow-up work, so it is neither a refutation nor a deferral. -
MINOR · src/components/Profile/components/ProfileHeader.tsx:107 · Name the profile-link copy action
The pill's avatar and share segments expose action names, but this button's accessible name is only the rendered URL (plusVerifiedwhen applicable). A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localizedaria-labelsuch asCopy profile linkand assert that action name in the header test. The supplied reply offers no evidence that the action is already exposed and names no follow-up, so it is neither a refutation nor a deferral.
Checked clean
- Confirmed the detached worktree head, trusted author, dev base ref, supplied base SHA, and merge base all match the request.
- Rechecked P1 against the pending/saved state model, serialized save loop, reopen effect, reroll path, and AvatarPicker tests; no guard or test covers the close/reopen race.
- Rechecked P2 against the three pill segments and ProfileHeader tests; avatar and share have action names while the copy segment still exposes only the URL and verification state.
- P3 and P4 duplicate the same two surviving defects, so they are not emitted as duplicate findings.
- Reviewed the latest review-fix commit: manifest-gated picks, three-column keyboard navigation, earned-chip spacing, verified-state naming, and nested share-shadow removal introduce no additional actionable defect.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, native export, design-system lint, analysis, preview, and visual snapshots.
- The exact diff passes
git diff --check; focused local Jest is unavailable because the detached worktree has no installed dependencies.
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: 676da2fcacd9 · Context: repo · Took 12m
| useEffect(() => { | ||
| if (open) setOffer(offerBasics(saved)) | ||
| // deal once per open; the pick joins the row by being picked from it | ||
| if (open) setHand(dealHand(saved, unlocked, { prefer })) |
There was a problem hiding this comment.
MINOR: Deal from the pending pick when reopening
If a user selects an avatar, closes the drawer before the serialized save/refetch finishes, and immediately reopens it, pending still identifies the visibly selected avatar while saved is the old server value. This line deals from that stale value, so the fresh random hand can omit the pending avatar and render with no radio checked even though the pill already shows the new pick. Deal from pick here and cover the close/reopen-during-save case. The supplied reply does not dispute this reachable state or move the fix to follow-up work, so it is neither a refutation nor a deferral.
| {pillVisible && ( | ||
| <span className="flex h-full shrink-0 items-center pl-3">{ownAvatar('small')}</span> | ||
| )} | ||
| <button |
There was a problem hiding this comment.
MINOR: Name the profile-link copy action
The pill's avatar and share segments expose action names, but this button's accessible name is only the rendered URL (plus Verified when applicable). A non-visual user therefore hears a URL button without learning that activation copies it rather than opening or sharing it. Add a localized aria-label such as Copy profile link and assert that action name in the header test. The supplied reply offers no evidence that the action is already exposed and names no follow-up, so it is neither a refutation nor a deferral.
Chip on #2989: - AvatarPicker dealt the reopened hand from `saved`, the server value. A close/reopen while a save is still draining could therefore drop the avatar the pill already shows and leave no tile checked. Deal from `pick`, which prefers the pending value. - The pill's copy segment exposed only the url as its accessible name, so it read as a link rather than as an action. Add an sr-only action label, as the sibling share segment already does; the url and the verified state stay in the name.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The two unique prior defects and their duplicate entries are fixed at this exact head; no additional actionable finding survived the full review.
Checked clean
- Confirmed the detached worktree head, trusted author, dev base ref, supplied base SHA, and merge base all match the request.
- Rechecked P1, P3, and P5 against the pending/saved state model and close/reopen regression test: the reopen effect now deals from the pending-aware pick.
- Rechecked P2, P4, and P6 against the rendered profile pill and accessibility regression test: the copy segment now exposes the localized action, URL, and verified state.
- Reviewed the complete avatar hand, badge deep link, picker query cleanup, serialized save behavior, self-profile pill, public-profile fallback, localization, correctness, security, adversarial, and slop surfaces; no additional actionable finding survived.
- Exact-head unit, typecheck, eslint, format, native export, design-system lint, analysis, preview, provenance, and aggregate CI checks completed successfully; the advisory visual-snapshot job was still in progress after aggregate CI passed.
- The exact diff passes git diff --check; focused local Jest was unavailable because the detached worktree has no installed dependencies.
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: 46f425519ea3 · Context: repo · Took 10m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The two unique prior defects and their duplicate entries are fixed at this exact head; no additional actionable finding survived the full review.
Checked clean
- Confirmed the detached worktree head, trusted author, dev base ref, supplied base SHA, and merge base all match the request.
- Rechecked P1, P3, and P5 against the pending/saved state model and close/reopen regression test: the reopen effect now deals from the pending-aware pick.
- Rechecked P2, P4, and P6 against the rendered profile pill and accessibility regression test: the copy segment now exposes the localized action, URL, and verified state.
- Reviewed the complete avatar hand, badge deep link, picker query cleanup, serialized save behavior, self-profile pill, public-profile fallback, localization, correctness, security, adversarial, and slop surfaces; no additional actionable finding survived.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, native export, design-system lint, analysis, preview, provenance, aggregate CI, and visual snapshots.
- The exact diff passes git diff --check; focused local Jest was unavailable because the detached worktree has no installed dependencies.
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: 46f425519ea3 · Context: repo · Took 11m
innolope-dev
left a comment
There was a problem hiding this comment.
This can't be used from the design and UX perspective. More on discord
|
Superseded by #3014, so closing this one. @abalinda — thank you, this was not wasted. The deal logic is yours: Final spec: https://claude.ai/code/artifact/b0d808c5-0924-4e1d-9c86-140a4993eb7c The four points and how each is answered are in #3014's description under Slava's points, answered. Worth a look if you want to see where your code ended up. |
Summary
Follow-up to #2929, from the "Avatar Hand" design (https://claude.ai/code/artifact/8e3ad40a-544f-48e7-ab5b-88e227b5320b). Three things the shipped v2 got wrong, fixed:
peanut.me/<handle>("Link copied" toast), the share icon runs the existingShareButtonflow. Public profiles and the edit screen keep the stacked header.dealHand(at least one earned badge avatar when the user holds a badge with art, the current pick kept, the rest from the basics), slot 9 is the die. A tap saves at once (the serialized-save logic from feat: badge-linked profile avatars (TASK-22142) #2929 is unchanged); rolling deals again and never changes the pick. "Use my initial" is now tile 1, not a button. Earned avatars carry a yellow border and an "Earned" chip./profile?avatarPicker=true&badge=CODE, so the first hand is guaranteed to hold the new badge's art. Closing the drawer clears both params.No API change: the pool the server validates a pick against is the same 20 basics plus what the user's badges unlock.
Task
TASK-22142 — https://app.notion.com/p/peanutprotocol/Build-badge-linked-avatar-system-v2-3cf83811757981efa8afecf6667d2662
Risks / breaking changes
REFERRAL_CTA_CLICKEDnow fires on the share segment only — copy taps are not counted (impressions unchanged).ShareButtongains an optionalshadowSize(default'4', unchanged for every existing caller); the pill passesnullso its ghost share segment draws no shadow of its own instead of covering the shadow class from the call site./code-review high): the "Earned" chip no longer overlaps the sticker (XL band above the art), arrow keys stay in their column in the 3-wide hand, a pick this bundle's manifest does not know is not dealt, the verified tick has an sr-only name. Not changed on purpose: the die inside the radiogroup (moving it out needsdisplay: contents, which has its own screen-reader history), the alternating spin direction, and Chip's two minor threads.main; against an API without it the pill shows the initial and a pick fails with the friendly toast (same as feat: badge-linked profile avatars (TASK-22142) #2929). Staging needs api#1519 (dev sync).avatar.{initial,earned,rollDie}added, seven keys dropped, mirrored in es-419 / pt-BR (translations are mine — native check welcome).Design — flagged, not changed (design.md "building or migrating a screen")
The artifact was written in raw px/hex; each value is snapped to the DS scale:
text-heading-card+text-body-l text-foreground-secondaryrounded-full bg-white border-black btn-shadow-primary-4rounded-round bg-background-default border-border-default shadow-4h-18) is not a DS height — flaggedtext-success-1text-green-500(whatVerifiedUserLabeluses)success-1is legacy palette (ratchet at 0); no semantic success foreground token exists — owed#FFF6CC+ 16px yellow bordered "Earned" pill, 8.5px capsborder-action-secondary(precedent: BadgeEarnToast) +StatusBadge status="custom"(Label/M)secondary-4is legacy); 8.5px is below the 12px floor; "don't: custom colored pills with raw classes"p-4+ 64px sticker = 96pxborder border-dashed border-border-default,<Icon name="dice" size={24}>dice→ lucideDicesrotate-360/rotate-0+motion-safe:duration-slow ease-springstylewould raise theinlineStyleratchet (direction alternates per tap — accepted)AVATAR_CAST(slug → name), English like today's slug labelsscripts/ds-lint-counts.mjs --checkis green (two counts went down).QA
/profile?__fixture=profile(the pill),/profile?avatarPicker=true&__fixture=avatar-picker(the hand, beetle selected, three Bug Whisperer tiles marked "Earned"),/profile?avatarPicker=true&badge=BUG_WHISPERER&__fixture=avatar-picker(deep link),/home?__fixture=home-avatar(unchanged). ds-shots diffs them per push.dealHand(length, initial first, pick kept, guarantee, prefer, seeded determinism),AvatarPicker(hand + earned chips, no-badge hand, deep-link prefer, save/snap-back/serialized saves, die, initial),ProfileHeader(visibility, impression, handle vs name, copy + toast, share capture, picker beside both buttons),BadgeEarnToast(deep link with badge). Full suite 465/465, typecheck, prettier, ds-lint ratchet all green locally.Screenshots
375 wide, fixtures (no backend), this head. Assets live on the
pr-assets-2989branch, deleted after merge. ds-shots visual diff on the head commit: see the sticky comment on this PR.peanut.me/handle+ verified check (copies the link), share icon (share flow).?__fixture=profile?avatarPicker=true&__fixture=avatar-picker