Repository navigation
feat(logo): add bitcoin icon to Easter egg, make morph system reusable - #327
Merged
Merged
Conversation
…sable Extract pixel shape definitions and morph state machine into reusable modules so the animation system can be used elsewhere (e.g. loading screens). - Add pixel-shapes.ts: shared shape definitions, parseShape(), grid constants - Add usePixelMorph.ts: custom hook for morph state machine (advance, revert, settle) - Refactor Logo.tsx to consume the hook and shared constants - Add bitcoin (₿) pixel art to the shape cycle - Support variable pixel counts across shapes via automatic padding - Cycle order: logo → space invader → bitcoin → heart → logo
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughExtracts pixel-morph logic and shape data from Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Logo as Logo Component
participant Hook as usePixelMorph
participant Shapes as pixel-shapes
participant Timer as RAF/Timeouts
User->>Logo: click / Space / Enter
Logo->>Hook: advance()
Hook->>Hook: update shapeIdx, settled, reverting
Hook->>Shapes: read SLOT_SHAPES[shapeIdx]
Hook-->>Logo: slots, settled, reverting (return state)
Hook->>Timer: schedule revert / settle via RAF & timeouts
Timer-->>Hook: timers fire (revert/settle)
Hook-->>Logo: update state -> re-render SVG pixels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Deploying wallet-bitcoin with
|
| Latest commit: |
df7fa24
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d8b069ba.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://feat-logo-easter-egg-icons.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
df7fa24
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://df674120.arkade-wallet.pages.dev |
| Branch Preview URL: | https://feat-logo-easter-egg-icons.arkade-wallet.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/icons/usePixelMorph.ts (2)
59-65:shapeIdxfrom the closure could be stale under rapid taps within the same render frame.If
advanceis called twice synchronously before React re-renders (unlikely but possible),shapeIdxin the closure is still the value from the previous render. The second call won't see the first call'ssetShapeIdx. This can also cause two rAF/timeout schedules where the first timeout handle is leaked when the second overwritesrevertRef.current.Using a functional updater for
setShapeIdxwouldn't fully solve it (thenext === 0branch decision also needs the current index). If you want to be fully safe, auseRefmirror ofshapeIdxwould work. Given the tap-rate constraint this is low risk, but noting for completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/usePixelMorph.ts` around lines 59 - 65, The closure over shapeIdx in advance can be stale during rapid synchronous calls and can leak timers via revertRef; fix by adding a useRef mirror (e.g., shapeIdxRef) that you update whenever you call setShapeIdx (use functional updater to compute next index and then set shapeIdxRef.current = next), and in advance always read from shapeIdxRef.current to compute next and decide the next === 0 branch; also clear any existing timeout in revertRef before setting a new one (and update revertRef.current when scheduling) so no handles are overwritten/leaked; reference symbols: shapeIdx, setShapeIdx, shapeIdxRef (new), revertRef, startRevert, REVERT_DELAY, SHAPES, advance.
40-57: Therevertingandsettledbranches are identical — consider deduplication.Both branches perform the same operations: clear the current state, set
settled(false), then via rAF advance to shape 1 and schedule revert. Collapsing them would reduce surface area for divergent bugs.Proposed refactor
const advance = useCallback(() => { if (revertRef.current) clearTimeout(revertRef.current) if (settleRef.current) clearTimeout(settleRef.current) - if (reverting) { - setReverting(false) - setSettled(false) - requestAnimationFrame(() => { - setShapeIdx(1) - revertRef.current = setTimeout(startRevert, REVERT_DELAY) - }) - return - } - - if (settled) { + if (reverting || settled) { + setReverting(false) setSettled(false) requestAnimationFrame(() => { setShapeIdx(1) revertRef.current = setTimeout(startRevert, REVERT_DELAY) }) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/usePixelMorph.ts` around lines 40 - 57, The two branches can be collapsed to avoid duplication: replace the separate if (reverting) { ... } and if (settled) { ... } blocks with a single if (reverting || settled) { if (reverting) setReverting(false); setSettled(false); requestAnimationFrame(() => { setShapeIdx(1); revertRef.current = setTimeout(startRevert, REVERT_DELAY); }); return; } so you preserve the setReverting(false) call only when reverting is true while reusing the shared logic (setSettled(false), requestAnimationFrame, setShapeIdx, revertRef.current, startRevert, REVERT_DELAY).src/icons/Logo.tsx (1)
22-27: Pre-existing:onKeyDownis a no-op — keyboard users can't trigger the Easter egg with Space.Not introduced in this PR, but worth noting: the empty
onKeyDown={() => {}}means therole="button"div doesn't respond to Space key presses (Enter may work viaonClickin some browsers). Since you're touching this component, consider wiring it up:Proposed fix
- onKeyDown={() => {}} + onKeyDown={(e) => { if (e.key === ' ' || e.key === 'Enter') { e.preventDefault(); advance(); } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/Logo.tsx` around lines 22 - 27, The div with role='button' currently has a no-op onKeyDown; update the onKeyDown handler in the Logo component so keyboard users can trigger the Easter egg: implement an onKeyDown that listens for 'Enter' and Space (handle both ' ' and legacy 'Spacebar'), call the existing advance() function when those keys are pressed, and preventDefault for Space to avoid page scroll; replace the empty onKeyDown={() => {}} with this accessible handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/icons/usePixelMorph.ts`:
- Around line 19-24: The cleanup in the useEffect only clears existing timeouts
(revertRef/settleRef) but doesn’t prevent requestAnimationFrame callbacks from
later creating new timeouts in advance, which can orphan timers after unmount;
add a mounted flag (e.g., isMountedRef) set true on mount and false in the
cleanup, and then guard any rAF callback inside advance (and any other callbacks
that create timeouts) with if (!isMountedRef.current) return so they never set
revertRef.current or settleRef.current after unmount.
---
Nitpick comments:
In `@src/icons/Logo.tsx`:
- Around line 22-27: The div with role='button' currently has a no-op onKeyDown;
update the onKeyDown handler in the Logo component so keyboard users can trigger
the Easter egg: implement an onKeyDown that listens for 'Enter' and Space
(handle both ' ' and legacy 'Spacebar'), call the existing advance() function
when those keys are pressed, and preventDefault for Space to avoid page scroll;
replace the empty onKeyDown={() => {}} with this accessible handler.
In `@src/icons/usePixelMorph.ts`:
- Around line 59-65: The closure over shapeIdx in advance can be stale during
rapid synchronous calls and can leak timers via revertRef; fix by adding a
useRef mirror (e.g., shapeIdxRef) that you update whenever you call setShapeIdx
(use functional updater to compute next index and then set shapeIdxRef.current =
next), and in advance always read from shapeIdxRef.current to compute next and
decide the next === 0 branch; also clear any existing timeout in revertRef
before setting a new one (and update revertRef.current when scheduling) so no
handles are overwritten/leaked; reference symbols: shapeIdx, setShapeIdx,
shapeIdxRef (new), revertRef, startRevert, REVERT_DELAY, SHAPES, advance.
- Around line 40-57: The two branches can be collapsed to avoid duplication:
replace the separate if (reverting) { ... } and if (settled) { ... } blocks with
a single if (reverting || settled) { if (reverting) setReverting(false);
setSettled(false); requestAnimationFrame(() => { setShapeIdx(1);
revertRef.current = setTimeout(startRevert, REVERT_DELAY); }); return; } so you
preserve the setReverting(false) call only when reverting is true while reusing
the shared logic (setSettled(false), requestAnimationFrame, setShapeIdx,
revertRef.current, startRevert, REVERT_DELAY).
- Add mountedRef to prevent orphaned timers after unmount - Deduplicate reverting/settled branches in advance() - Wire up keyboard handler for Space/Enter on logo button
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/icons/usePixelMorph.ts (1)
28-36: Consider guardingsettleRef's callback withmountedReffor consistency.The
settleReftimeout callback (lines 32–35) callssetSettled/setRevertingwithout amountedRef.currentcheck, unlike the rAF callback inadvance. In practice this is safe — the cleanup effect cancelssettleRefviaclearTimeoutbefore the component fully unmounts, and React 18 treats post-unmountsetStateas a no-op — but applying the same guard here makes the defensive pattern uniform across all async callbacks.♻️ Proposed consistency fix
settleRef.current = setTimeout(() => { + if (!mountedRef.current) return setSettled(true) setReverting(false) }, MORPH_MS + 50)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/usePixelMorph.ts` around lines 28 - 36, The timeout callback in startRevert uses settleRef to call setSettled and setReverting without checking mountedRef like the rAF path; update startRevert so the setTimeout handler first checks mountedRef.current (or bail if false) before calling setSettled/setReverting, while keeping the existing clearTimeout(settleRef.current) and setTimeout behavior intact to ensure the defensive pattern matches advance's rAF callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/icons/usePixelMorph.ts`:
- Around line 28-36: The timeout callback in startRevert uses settleRef to call
setSettled and setReverting without checking mountedRef like the rAF path;
update startRevert so the setTimeout handler first checks mountedRef.current (or
bail if false) before calling setSettled/setReverting, while keeping the
existing clearTimeout(settleRef.current) and setTimeout behavior intact to
ensure the defensive pattern matches advance's rAF callback.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/icons/Logo.tsxsrc/icons/usePixelMorph.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/icons/Logo.tsx
Member
|
plese |
bordalix
approved these changes
Feb 24, 2026
bordalix
left a comment
bordalix
left a comment
Collaborator
There was a problem hiding this comment.
NIT: change flow from:
space monster -> bitcoin -> love to
space monster -> loves -> bitcoin 😄
Contributor
Author
new pr for this submitting now lol |
This was referenced Feb 26, 2026
6 tasks
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
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
pixel-shapes.ts,usePixelMorph.ts) so the animation can be used elsewhere (loading screens, etc.)Changes
src/icons/pixel-shapes.ts— shared shape definitions,parseShape(), grid constants, auto-padding for variable pixel countssrc/icons/usePixelMorph.ts—usePixelMorph()hook encapsulating the morph state machinesrc/icons/Logo.tsx— simplified to consume the hook and shared constantsTest plan
LogoIconAnimatedin AlertBox still pulses correctlySummary by CodeRabbit
Refactor
Style