fix(card): keep share-asset stickers off the username pill#2352
Merged
Conversation
The "I'M IN!" share asset could render a badge sticker on top of the peanut.me/<handle> pill, hiding the handle — the whole point of the shareable asset. Two compounding causes: - the final SEPARATION_PASSES cleanup dropped the pill keep-out, so pairwise separation could shove a sticker onto the pill on the last pass (up to ~65% of the handle covered in the worst case); - the pill rendered below the stickers (z-index 3 < 4), so any sticker that landed over it painted on top. Restore the keep-out in the final pass as a *soft* pull — a hard shove re-creates the corner deadlock that pass exists to break — and render the pill above the stickers (z-index 5) so the handle can never be covered. Add a regression test asserting no sticker covers >40% of the rendered pill rect across a seed/count sweep.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughUpdates the ChangesPill layering and overlap prevention
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
Contributor
Code-analysis diffPainscore total: 5872.71 → 5873.18 (+0.47) 🆕 New findings (9)
✅ Resolved (9)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Card/share-asset/shareAssetLayout.ts (1)
246-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM! Soft pull is correctly scaled from the existing hard-push logic and matches the documented deadlock-avoidance rationale.
Optional: the two
hitsPill/exit blocks (main loop vs final pass) are near-identical except for the0.5scale factor — could be extracted into a smallpillPull(p, half, pill, strength)helper to avoid future drift between the hard and soft variants.♻️ Example extraction
+function pillPull(p: { x: number; y: number }, half: number, pill: { x0: number; y0: number }, strength: number) { + if (!hitsPill(p.x, p.y, half, pill)) return + const exitLeft = p.x + half - (pill.x0 - PILL_PAD) + const exitUp = p.y + half - (pill.y0 - PILL_PAD) + if (exitLeft < exitUp) p.x -= exitLeft * strength + else p.y -= exitUp * strength +}Then call
pillPull(p, half, pill, 1)in the main loop andpillPull(p, half, pill, 0.5)in the final pass.Also applies to: 308-318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Card/share-asset/shareAssetLayout.ts` around lines 246 - 252, The `hitsPill`/exit adjustment logic is duplicated between the main loop and the final pass in `shareAssetLayout`, differing only by the pull strength, so extract that block into a small `pillPull(p, half, pill, strength)` helper and reuse it from both places. Keep the existing `hitsPill`, `PILL_PAD`, and exit-axis choice logic inside the helper, then call it with the appropriate strength values to prevent the hard/soft variants from drifting apart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/Card/share-asset/shareAssetLayout.ts`:
- Around line 246-252: The `hitsPill`/exit adjustment logic is duplicated
between the main loop and the final pass in `shareAssetLayout`, differing only
by the pull strength, so extract that block into a small `pillPull(p, half,
pill, strength)` helper and reuse it from both places. Keep the existing
`hitsPill`, `PILL_PAD`, and exit-axis choice logic inside the helper, then call
it with the appropriate strength values to prevent the hard/soft variants from
drifting apart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb555b18-efbc-4a20-8416-0fa04efea825
📒 Files selected for processing (3)
src/components/Card/share-asset/ShareAssetD3.tsxsrc/components/Card/share-asset/__tests__/shareAssetLayout.test.tssrc/components/Card/share-asset/shareAssetLayout.ts
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
The card share asset ("I'M IN!") could render a badge sticker on top of the
peanut.me/<handle>username pill, hiding the handle — which is the entire point of a shareable asset. Two compounding causes:SEPARATION_PASSEScleanup inplaceStampsdropped the pill keep-out (it only re-applied the card keep-outs), so pairwise sticker separation could shove a sticker onto the pill on the last pass. Measured up to ~65% of the pill covered across a username × badge-count sweep.z-index: 3, below the stickers atz-index: 4, so any sticker that landed over it painted on top.Fix
z-index: 5, above the stickers, so the handle can never be covered even if a corner drifts under it.Both are needed: the soft pull keeps the collage clean; the z-index is the hard guarantee.
Risks / blast radius
Small and self-contained — only the share-asset layout engine + its renderer. No API/contract changes, no shared-state or money paths.
placeStampsstays deterministic (same seed → same layout). Only visible effect elsewhere: a badge sticker may now sit slightly higher/left near the bottom-right, and the pill paints over any residual overlap.QA
<ShareAssetD3 />rendered from the running app across 20 representative + 5 extreme configs (1-char / 12-char / emoji handles, 1–12 badges): every handle legible, pill on top.