Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughDefers JoinWaitlist redirect until authenticated user is available and centralizes login error handling. Adds NoMoreJailModal and EarlyUserModal with sessionStorage/flag gating, sets Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Setup/Views/JoinWaitlist.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T11:18:10.633Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/app/invite/page.tsx:44-53
Timestamp: 2025-09-25T11:18:10.633Z
Learning: The setup page (/setup) contains the waitlist functionality in the welcome step using the JoinWaitlist component, so redirecting users to /setup for joining the waitlist is correct.
Applied to files:
src/components/Setup/Views/JoinWaitlist.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
… unnecessary delay in login flow
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/layout.tsx (1)
100-100: Remove debug console.log.This console.log statement should be removed before merging to production.
Apply this diff:
- console.log(user, 'user') -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(mobile-ui)/layout.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/home/page.tsx (1)
142-187: Remove duplicate useEffect block.The effect for showing the balance warning modal is duplicated (lines 142-164 and 167-187). This causes the same logic to run twice on every render, which is inefficient and could lead to unexpected behavior.
Apply this diff to remove the duplicate:
- // effect for showing balance warning modal - useEffect(() => { - if (isFetchingBalance || balance === undefined || !user) return - - if (typeof window !== 'undefined') { - const hasSeenBalanceWarning = getFromLocalStorage(`${user!.user.userId}-hasSeenBalanceWarning`) - const balanceInUsd = Number(formatUnits(balance, PEANUT_WALLET_TOKEN_DECIMALS)) - - // show if: - // 1. balance is above the threshold - // 2. user hasn't seen this warning in the current session - // 3. no other modals are currently active - if ( - balanceInUsd > BALANCE_WARNING_THRESHOLD && - !hasSeenBalanceWarning && - !showIOSPWAInstallModal && - !showAddMoneyPromptModal - ) { - setShowBalanceWarningModal(true) - } - } - }, [balance, isFetchingBalance, showIOSPWAInstallModal, showAddMoneyPromptModal, user]) - // effect for showing add money prompt modal useEffect(() => {
🧹 Nitpick comments (1)
src/app/(mobile-ui)/home/page.tsx (1)
193-193: Consider guarding sessionStorage access.While the sessionStorage access here is inside a useEffect (which runs client-side), it's good practice to add a runtime check for safety, especially since Next.js can pre-render code paths during build.
Consider applying this diff:
- const showNoMoreJailModal = sessionStorage.getItem('showNoMoreJailModal') + const showNoMoreJailModal = typeof window !== 'undefined' ? sessionStorage.getItem('showNoMoreJailModal') : null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(mobile-ui)/home/page.tsx(4 hunks)src/components/Global/NoMoreJailModal/index.tsx(1 hunks)src/components/Invites/JoinWaitlistPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/components/Invites/JoinWaitlistPage.tsx:41-55
Timestamp: 2025-09-29T18:34:33.596Z
Learning: In the JoinWaitlistPage component, after successfully accepting an invite via invitesApi.acceptInvite(), calling fetchUser() is sufficient to update the user state and automatically display the app. No manual navigation to /home or other pages is required since the user is already on the home page and the app will be displayed once user.hasAppAccess is updated.
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/app/invite/page.tsx:44-53
Timestamp: 2025-09-25T11:18:10.633Z
Learning: The setup page (/setup) contains the waitlist functionality in the welcome step using the JoinWaitlist component, so redirecting users to /setup for joining the waitlist is correct.
📚 Learning: 2025-09-29T18:34:33.596Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/components/Invites/JoinWaitlistPage.tsx:41-55
Timestamp: 2025-09-29T18:34:33.596Z
Learning: In the JoinWaitlistPage component, after successfully accepting an invite via invitesApi.acceptInvite(), calling fetchUser() is sufficient to update the user state and automatically display the app. No manual navigation to /home or other pages is required since the user is already on the home page and the app will be displayed once user.hasAppAccess is updated.
Applied to files:
src/components/Invites/JoinWaitlistPage.tsx
🧬 Code graph analysis (1)
src/components/Global/NoMoreJailModal/index.tsx (1)
src/components/0_Bruddle/Button.tsx (1)
Button(76-267)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
src/components/Invites/JoinWaitlistPage.tsx (1)
46-46: LGTM!Setting the sessionStorage flag here is appropriate. Since this runs in an async handler after a successful invite acceptance, there are no SSR concerns, and the timing before
fetchUser()ensures the modal will be ready to display when the home page renders.src/app/(mobile-ui)/home/page.tsx (2)
207-209: Good modal precedence logic.The condition correctly prevents the Add Money Prompt modal from showing when the No More Jail modal should take precedence. This prevents modal stacking and ensures a better user experience.
274-274: Modal integration looks good.The NoMoreJailModal component is correctly integrated alongside other modals. The component manages its own visibility state based on sessionStorage, which fits the existing modal coordination pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(mobile-ui)/home/page.tsx (1)
195-211: Synchronize Add Money prompt with modal precedenceLine 209’s
!user?.showEarlyUserModalguard never prevents the Add Money prompt if the wallet balance resolves before the user does. On a zero-balance account, the effect runs whileuseris still undefined, opens the Add Money modal, and later the Early User modal opens as well—exactly what the precedence comment warns against. BecauseuserandisPostSignupActionModalVisibleare missing from the dependency list, the effect doesn’t re-run when those flags change, so the stale decision sticks. Please wait until the user payload is available and include the relevant dependencies so the guard actually works.- useEffect(() => { - if (typeof window !== 'undefined' && !isFetchingBalance) { + useEffect(() => { + if (typeof window === 'undefined' || isFetchingBalance || !user) { + return + } const hasSeenAddMoneyPromptThisSession = sessionStorage.getItem('hasSeenAddMoneyPromptThisSession') const showNoMoreJailModal = sessionStorage.getItem('showNoMoreJailModal') @@ - !isPostSignupActionModalVisible && - showNoMoreJailModal !== 'true' && - !user?.showEarlyUserModal + !isPostSignupActionModalVisible && + showNoMoreJailModal !== 'true' && + !user.showEarlyUserModal ) { setShowAddMoneyPromptModal(true) sessionStorage.setItem('hasSeenAddMoneyPromptThisSession', 'true') } - } - }, [balance, isFetchingBalance, showIOSPWAInstallModal, showBalanceWarningModal]) + }, [ + balance, + isFetchingBalance, + showIOSPWAInstallModal, + showBalanceWarningModal, + isPostSignupActionModalVisible, + user, + ])
🧹 Nitpick comments (1)
tailwind.config.js (1)
177-182: Consider optimizing the keyframes for efficiency.The keyframe states at 60% and 100% are identical, meaning the animation spends 40% of its duration with no visual change. You could simplify this to make the animation more efficient:
pulsateDeep: { '0%': { transform: 'scale(1)', opacity: '1' }, '30%': { transform: 'scale(0.7)', opacity: '1' }, - '60%': { transform: 'scale(1)', opacity: '1' }, - '100%': { transform: 'scale(1)', opacity: '1' }, + '100%': { transform: 'scale(1)', opacity: '1' }, },Alternatively, adjust the percentages to distribute the animation more evenly:
pulsateDeep: { '0%': { transform: 'scale(1)', opacity: '1' }, - '30%': { transform: 'scale(0.7)', opacity: '1' }, - '60%': { transform: 'scale(1)', opacity: '1' }, + '50%': { transform: 'scale(0.7)', opacity: '1' }, '100%': { transform: 'scale(1)', opacity: '1' }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/(mobile-ui)/home/page.tsx(4 hunks)src/components/Global/EarlyUserModal/index.tsx(1 hunks)src/components/Global/Icons/Icon.tsx(3 hunks)src/components/Global/Icons/lock.tsx(1 hunks)src/components/Home/InvitesIcon.tsx(1 hunks)src/interfaces/interfaces.ts(1 hunks)tailwind.config.js(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/components/Invites/JoinWaitlistPage.tsx:41-55
Timestamp: 2025-09-29T18:34:33.596Z
Learning: In the JoinWaitlistPage component, after successfully accepting an invite via invitesApi.acceptInvite(), calling fetchUser() is sufficient to update the user state and automatically display the app. No manual navigation to /home or other pages is required since the user is already on the home page and the app will be displayed once user.hasAppAccess is updated.
🧬 Code graph analysis (2)
src/components/Global/Icons/Icon.tsx (1)
src/components/Global/Icons/lock.tsx (1)
LockIcon(3-12)
src/components/Global/EarlyUserModal/index.tsx (3)
src/context/authContext.tsx (1)
useAuth(191-197)src/app/actions/users.ts (1)
updateUserById(12-35)src/utils/general.utils.ts (1)
generateInvitesShareText(1323-1325)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
tailwind.config.js (1)
193-193: LGTM!The animation configuration is correct and appropriate for a slow pulsating effect.
src/components/Home/InvitesIcon.tsx (1)
5-5: Excellent simplification to CSS-based animation!Replacing the Framer Motion animation with a pure CSS animation is a good improvement. CSS animations are more performant, reduce JavaScript overhead, and simplify the component. The new
animate-pulsate-slowclass integrates well with the Tailwind configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Global/NoMoreJailModal/index.tsx (2)
10-10: Fix state setter naming convention.The state setter
setisOpenshould besetIsOpen(capital I) to follow React naming conventions.Apply this diff:
- const [isOpen, setisOpen] = useState(false) + const [isOpen, setIsOpen] = useState(false)And update the usages:
const onClose = () => { - setisOpen(false) + setIsOpen(false) sessionStorage.removeItem('showNoMoreJailModal') } useEffect(() => { const showNoMoreJailModal = sessionStorage.getItem('showNoMoreJailModal') if (showNoMoreJailModal === 'true') { - setisOpen(true) + setIsOpen(true) } }, [])
63-63: Use standard Tailwind height class or arbitrary value.
h-42is not a standard Tailwind CSS class. Use either a standard height class (e.g.,h-40,h-44,h-48) or an arbitrary value with bracket notation (e.g.,h-[168px]).Apply one of these diffs:
- <div className="relative h-42 w-[90%] md:h-52"> + <div className="relative h-40 w-[90%] md:h-52">Or with arbitrary value:
- <div className="relative h-42 w-[90%] md:h-52"> + <div className="relative h-[168px] w-[90%] md:h-52">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Global/NoMoreJailModal/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1251
File: src/components/Invites/JoinWaitlistPage.tsx:41-55
Timestamp: 2025-09-29T18:34:33.596Z
Learning: In the JoinWaitlistPage component, after successfully accepting an invite via invitesApi.acceptInvite(), calling fetchUser() is sufficient to update the user state and automatically display the app. No manual navigation to /home or other pages is required since the user is already on the home page and the app will be displayed once user.hasAppAccess is updated.
🧬 Code graph analysis (1)
src/components/Global/NoMoreJailModal/index.tsx (1)
src/components/0_Bruddle/Button.tsx (1)
Button(76-267)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
src/components/Global/NoMoreJailModal/index.tsx (3)
12-15: LGTM!The
onClosehandler correctly closes the modal and cleans up the sessionStorage flag.
17-22: SSR issue resolved, reactivity enhancement optional.The sessionStorage access has been correctly moved into
useEffect, resolving the SSR/hydration issue from the previous review.The previous review also suggested adding a storage event listener to make the modal reactive to changes after mount. However, given that
showNoMoreJailModalis set once during invite acceptance and this modal is shown only once per session, the current single-check approach is sufficient.
24-68: LGTM! Well-structured modal implementation.The modal implementation is clean and well-organized:
- Proper Modal wrapper with appropriate props
- Clear content hierarchy with header, description, and action button
- Correct usage of Next.js Image component for assets
- Intentional styling with the animated peanut positioned above the modal content
The component integrates well with the broader modal system and aligns with the PR objectives of removing the jail modal and adding new user-facing modals.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Global/EarlyUserModal/index.tsx (1)
14-23: Standardize modal flag naming.The modal checks
user.showEarlyUserModalto display but persistshasSeenEarlyUserModalon close. This inconsistent naming pattern (showvs.hasSeen) makes the code harder to follow and maintain.Consider one of these approaches:
- Use a consistent prefix like
showEarlyUserModal/setEarlyUserModalSeenor- Align with a single pattern across all modals (e.g.,
isXModalVisiblefor display state,hasSeenXModalfor persistence)Based on learnings from past review comments about standardizing modal flag naming conventions.
src/app/(mobile-ui)/home/page.tsx (1)
205-213: Consider centralizing modal precedence logic.The modal precedence rules are becoming complex and fragmented:
- Some modals control their own visibility internally (
NoMoreJailModal,EarlyUserModal)- Others are controlled by state in this component (
AddMoneyPromptModal,IOSInstallPWAModal)- The Add Money Prompt must check multiple conditions including sessionStorage and user flags to determine if other modals are active
This fragmentation makes the system harder to maintain and reason about.
Consider creating a centralized modal manager or hook that:
- Maintains visibility state for all modals
- Enforces precedence rules in one place
- Provides a consistent API for showing/hiding modals
- Reduces coupling between modal components and this page
Example structure:
const { activeModal, showModal, hideModal } = useModalPrecedence({ modals: ['iosInstall', 'balanceWarning', 'postSignup', 'noMoreJail', 'earlyUser', 'addMoney'], precedence: [...], // ordered list conditions: {...} // visibility conditions for each })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/(mobile-ui)/home/page.tsx(8 hunks)src/components/Global/EarlyUserModal/index.tsx(1 hunks)src/components/Profile/index.tsx(3 hunks)src/utils/general.utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/Profile/index.tsx (1)
src/utils/general.utils.ts (2)
generateInviteCodeLink(1327-1331)generateInvitesShareText(1323-1325)
src/components/Global/EarlyUserModal/index.tsx (3)
src/context/authContext.tsx (1)
useAuth(191-197)src/utils/general.utils.ts (2)
generateInviteCodeLink(1327-1331)generateInvitesShareText(1323-1325)src/app/actions/users.ts (1)
updateUserById(12-35)
src/app/(mobile-ui)/home/page.tsx (1)
src/components/Global/PostSignupActionManager/index.tsx (1)
PostSignupActionManager(11-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
src/components/Profile/index.tsx (1)
34-34: LGTM - DRY principle applied.Good refactor extracting invite code generation into a shared utility. This addresses the previous review comment about code duplication across multiple places in the app.
Note: The empty username issue flagged in
generateInviteCodeLinkalso affects this call site.src/app/(mobile-ui)/home/page.tsx (2)
200-202: Comment updated - past review addressed.The comment has been updated to accurately reflect the new modal precedence rules, including the early user modal and no more jail modal.
71-71: Naming standardized - past review partially addressed.The renaming from
isPostSignupActionModalVisibletoshowPostSignupActionModalimproves consistency with other modal state variables in this file (e.g.,showAddMoneyPromptModal,showBalanceWarningModal).Note: The broader naming inconsistency issue across the entire modal system (flagged in past review) still exists, particularly with the
showEarlyUserModalvs.hasSeenEarlyUserModalpattern in the new EarlyUserModal.
2 participants
This PR -