Skip to content

[TASK-18633] fix: setup flow bugs - install copy, notification stuck state, PWA loop#1715

Closed
jjramirezn wants to merge 1 commit intodevfrom
fix/setup-flow-bugs
Closed

[TASK-18633] fix: setup flow bugs - install copy, notification stuck state, PWA loop#1715
jjramirezn wants to merge 1 commit intodevfrom
fix/setup-flow-bugs

Conversation

@jjramirezn
Copy link
Contributor

  1. Install screen: Replace tech-speak fallback with user-friendly copy
  2. Waitlist notification: Add 'Not now' skip so users without invite codes don't get stuck on 'Enable Notifications' forever
  3. PWA install loop: Remove target=_blank /setup link that causes redirect loop between browser and PWA; show instruction to open from Home Screen with fallback 'Continue here instead' button

1. Install screen: Replace tech-speak fallback with user-friendly copy
2. Waitlist notification: Add 'Not now' skip so users without invite codes
   don't get stuck on 'Enable Notifications' forever
3. PWA install loop: Remove target=_blank /setup link that causes redirect
   loop between browser and PWA; show instruction to open from Home Screen
   with fallback 'Continue here instead' button
@vercel
Copy link

vercel bot commented Mar 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
peanut-wallet Ready Ready Preview, Comment Mar 6, 2026 3:45pm

Request Review

@notion-workspace
Copy link

@jjramirezn
Copy link
Contributor Author

@coderabbitai review in depth.

Evaluate:

  • risk of introducing new bugs and breaking existing functionality
  • performance impact and latency
  • code quality and DRY

Also, write a very concise testing guide for QA

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

Walkthrough

This PR modifies two UI components: JoinWaitlistPage adds a notification skip state to allow users to proceed without enabling notifications, and InstallPWA revises post-installation messaging and flow for non-Brave browsers to guide users to open the app from Home Screen instead of in a new tab.

Changes

Cohort / File(s) Summary
Notification Skip Flow
src/components/Invites/JoinWaitlistPage.tsx
Introduces notificationSkipped state to allow users to bypass notification permission prompts. Updates conditional rendering to display the enable notifications prompt only when permissions aren't granted and user hasn't skipped, with a "Not now" button to set the skip flag. Main content renders when notifications are granted or skip is true.
PWA Installation Flow
src/components/Setup/Views/InstallPWA.tsx
Revises post-install flow for non-Brave browsers: replaces "Open Peanut app" button (which attempted to open PWA in new tab) with "Continue here instead" button that calls handleNext(). Adds instructional text about opening from Home Screen. Updates fallback messaging and layout spacing for manual installation instructions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: setup flow bugs including install copy improvements, notification skip functionality, and PWA loop fixes.
Description check ✅ Passed The description is directly related to the changeset, providing specific details about each of the three bug fixes being implemented.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/setup-flow-bugs

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me gather the changed files first.


🏁 Script executed:

#!/bin/bash
# Get the list of changed files in this PR
gh pr diff 1715 --repo peanutprotocol/peanut-ui --name-only

Length of output: 85


🏁 Script executed:

#!/bin/bash
# Get the full diff of the PR
gh pr diff 1715 --repo peanutprotocol/peanut-ui

Length of output: 4788


🏁 Script executed:

#!/bin/bash
cat src/components/Invites/JoinWaitlistPage.tsx

Length of output: 7971


🏁 Script executed:

#!/bin/bash
cat src/components/Setup/Views/InstallPWA.tsx

Length of output: 11344


@jjramirezn Here is the in-depth review.


🔍 In-Depth Review — PR #1715

1. Risk of Introducing New Bugs / Breaking Existing Functionality

🟡 Medium — Image animation not updated when notification is skipped (JoinWaitlistPage.tsx)

The layout image prop at the top of the component is:

<InvitesPageLayout image={isPermissionGranted ? peanutAnim.src : chillPeanutAnim.src}>

The PR's new notificationSkipped flag is not reflected here. When a user taps "Not now", isPermissionGranted is still false, so the chillPeanutAnim is displayed while the "You're still in Peanut jail" content renders. This is a visual inconsistency — the intended lively animation only triggers if permission is granted, not when the user skips.

Fix:

<InvitesPageLayout image={(isPermissionGranted || notificationSkipped) ? peanutAnim.src : chillPeanutAnim.src}>

🟡 Low — loading state removed from "Continue here instead" button (InstallPWA.tsx)

The original "Open Peanut app" button had loading={isSetupFlowLoading}:

// OLD
<Button ... loading={isSetupFlowLoading}>Open Peanut app</Button>

// NEW
<Button onClick={() => handleNext()} ...>Continue here instead</Button>

The equivalent "Continue" button in Scenario 1 (standalone mode, line ~140) retains loading={isSetupFlowLoading}. The new button is missing it, which means if handleNext() triggers an async flow, the user can double-tap and fire it multiple times.

Fix:

<Button onClick={() => handleNext()} className="w-full" shadowSize="4" variant="purple" loading={isSetupFlowLoading}>
    Continue here instead
</Button>

🔵 Minor UX — notificationSkipped resets on page refresh

notificationSkipped lives only in React state. On page refresh or component remount, the notification prompt reappears for users who clicked "Not now." Depending on product intent this may be acceptable (one-time flow), but it could frustrate users who refresh. If persistence is desired, sessionStorage would be the right scope for this ephemeral flag.


✅ The core bug fixes are correct

  • The DOM-manipulation hack (createElement('a') / link.target = '_blank') that caused the PWA redirect loop is cleanly removed. The handleNext() path is the right choice.
  • The "Not now" button correctly unblocks the notification-stuck state without any side effects on the notification permission itself.
  • Scenario 4 copy improvement doesn't touch logic — no regression risk.

2. Performance Impact and Latency

No meaningful impact. The removal of the DOM manipulation (create element → append → click → remove) is actually a micro-improvement — it was an unnecessarily expensive way to navigate that also fired a browser navigation event. Replacing it with handleNext() is cleaner. The new boolean state toggle is negligible.


3. Code Quality and DRY

Concern Severity
isSetupFlowLoading missing on new button Low
Image prop not updated for skip state Medium
"Continue here instead" and Scenario 4 "Continue" share near-identical JSX (same handleNext(), same shadow/variant) Minor / cosmetic
<button> (lowercase) used for "Not now" — consistent with existing "Log in with a different account" button below it ✅ Intentional & consistent

The overall structure is clean and the PR is appropriately scoped.


📋 QA Testing Guide

Fix 1 — Install screen copy (Scenario 4 fallback)

  1. Open setup on a browser/device that does not trigger beforeinstallprompt (e.g., Firefox Android, or a device where the PWA is already installed and can't be re-prompted).
  2. ✅ Verify new copy: "To install the app, please add it to your Home Screen from your browser menu."
  3. ✅ Tap Continue → setup flow advances.

Fix 2 — "Not now" skip on waitlist notification screen

  1. Log in with a waitlisted account (no invite code, notifications not yet granted).
  2. ✅ Verify "Enable notifications" screen shows with both Yes, notify me and Not now.
  3. Tap Not now → ✅ verify "You're still in Peanut jail" screen appears immediately.
  4. ✅ Verify the animated image updates (this is the bug flagged above — currently it won't).
  5. Tap Yes, notify me → grant permission → ✅ jail screen shows.
  6. Tap Yes, notify me → deny permission → ✅ jail screen shows (afterPermissionAttempt handles it).
  7. Refresh the page after clicking "Not now" → ✅ notification prompt reappears (expected; state resets).

Fix 3 — PWA install loop prevention (Android Chrome)

  1. On Android Chrome, go through setup to the PWA install step.
  2. Tap Add Peanut to Home Screen and confirm installation.
  3. ✅ Verify "Peanut has been installed! Open it from your Home Screen to continue." message appears.
  4. ✅ Verify Continue here instead button is present and advances the flow without opening a new tab / triggering a PWA↔browser loop.
  5. Open the installed PWA from the Home Screen.
  6. ✅ Verify it opens in standalone mode and the setup flow shows a plain Continue button (Scenario 1).
  7. On Brave Android: install → ✅ verify brave-specific instructions are shown (no "Continue here instead" button, parent setShowBraveSuccessMessage triggered).

🧠 Learnings used
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 852
File: src/hooks/useTransactionHistory.ts:177-239
Timestamp: 2025-05-14T21:16:06.369Z
Learning: When reviewing code, pay attention to and resolve CodeRabbit comments as they can contain useful refactoring suggestions and insights.

Learnt from: CR
Repo: peanutprotocol/peanut-ui PR: 0
File: .cursorrules:0-0
Timestamp: 2026-02-26T21:54:25.278Z
Learning: Applies to docs/PR.md : Maintain a concise docs/PR.md during PR work with: 1) Summary of changes, 2) Risks (what might break), 3) QA guidelines (what to test)

Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 594
File: src/components/Request/Pay/Views/Initial.view.tsx:188-213
Timestamp: 2024-12-31T15:08:15.640Z
Learning: Always resolve coderabbit comments when reviewing code, since they can contain useful suggestions.

Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1072
File: src/app/(setup)/setup/page.tsx:173-175
Timestamp: 2025-08-07T12:53:50.946Z
Learning: In the peanut-ui setup flow at `src/app/(setup)/setup/page.tsx`, when handling unsupported scenarios, both device not supported and browser not supported cases should show the same "Unsupported browser" message using the `UnsupportedBrowserModal` component, rather than having distinct messaging for each scenario.

Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1089
File: src/components/LandingPage/dropLink.tsx:35-42
Timestamp: 2025-08-12T17:44:04.268Z
Learning: In the Peanut UI project, opening the `/setup` route in a new tab from landing page CTAs is intentional design behavior to keep users on the marketing page while they start the setup process.

Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/components/Invites/JoinWaitlistPage.tsx (1)

1-176: ⚠️ Potential issue | 🟡 Minor

Prettier check is currently failing for this file.

CI already reports formatting drift; please run:
pnpm prettier --write src/components/Invites/JoinWaitlistPage.tsx
before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Invites/JoinWaitlistPage.tsx` around lines 1 - 176, Prettier
formatting is failing for JoinWaitlistPage; run the formatter and commit the
changes: run pnpm prettier --write on
src/components/Invites/JoinWaitlistPage.tsx, then review the JoinWaitlistPage
component (and helper functions validateInviteCode, handleAcceptInvite,
handleLogout) to ensure no trailing commas/extra spaces or import ordering
issues remain, stage the formatted file, and push the commit so CI passes.
src/components/Setup/Views/InstallPWA.tsx (1)

1-288: ⚠️ Potential issue | 🟡 Minor

Prettier check is currently failing for this file.

CI already reports formatting drift; please run:
pnpm prettier --write src/components/Setup/Views/InstallPWA.tsx
before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Setup/Views/InstallPWA.tsx` around lines 1 - 288, Prettier
formatting is failing for InstallPWA; run the formatter and commit the changes:
run `pnpm prettier --write src/components/Setup/Views/InstallPWA.tsx`, save the
result, verify file compiles, and push—this will reformat the InstallPWA
component (including AndroidPWASpecificInstallFlow, DesktopInstructions,
handleInstall, and exported default InstallPWA) to satisfy CI; no functional
code changes required beyond the formatting pass.
🧹 Nitpick comments (1)
src/components/Invites/JoinWaitlistPage.tsx (1)

31-31: Move notificationSkipped to URL state instead of local component state.

This is user-facing flow state controlling which UI section renders. Local state resets on page refresh, losing the user's choice to skip notifications. Use useQueryStates with a boolean parser for this flag (e.g., parseAsBoolean from nuqs).

See established patterns in src/app/(mobile-ui)/add-money/[country]/bank/page.tsx and src/components/AddMoney/components/MantecaAddMoney.tsx for reference.

Also applies to: 103-105, 112-112

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Invites/JoinWaitlistPage.tsx` at line 31, Replace the local
React state notificationSkipped in JoinWaitlistPage with URL-backed state using
useQueryStates and the parseAsBoolean parser so the flag persists across
refreshes; specifically remove
useState(notificationSkipped)/setNotificationSkipped and instead call
useQueryStates({ notificationSkipped: [false, { parse: parseAsBoolean }] })
(importing parseAsBoolean from 'nuqs' and useQueryStates from
'next-use-query-state' or project equivalent), then update all references and
setters (notificationSkipped and setNotificationSkipped) to use the value and
setter returned by useQueryStates; apply the same change at the other mentioned
spots (around the blocks noted at lines ~103-105 and ~112) so the UI rendering
logic relies on the URL boolean flag rather than component local state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/components/Invites/JoinWaitlistPage.tsx`:
- Around line 1-176: Prettier formatting is failing for JoinWaitlistPage; run
the formatter and commit the changes: run pnpm prettier --write on
src/components/Invites/JoinWaitlistPage.tsx, then review the JoinWaitlistPage
component (and helper functions validateInviteCode, handleAcceptInvite,
handleLogout) to ensure no trailing commas/extra spaces or import ordering
issues remain, stage the formatted file, and push the commit so CI passes.

In `@src/components/Setup/Views/InstallPWA.tsx`:
- Around line 1-288: Prettier formatting is failing for InstallPWA; run the
formatter and commit the changes: run `pnpm prettier --write
src/components/Setup/Views/InstallPWA.tsx`, save the result, verify file
compiles, and push—this will reformat the InstallPWA component (including
AndroidPWASpecificInstallFlow, DesktopInstructions, handleInstall, and exported
default InstallPWA) to satisfy CI; no functional code changes required beyond
the formatting pass.

---

Nitpick comments:
In `@src/components/Invites/JoinWaitlistPage.tsx`:
- Line 31: Replace the local React state notificationSkipped in JoinWaitlistPage
with URL-backed state using useQueryStates and the parseAsBoolean parser so the
flag persists across refreshes; specifically remove
useState(notificationSkipped)/setNotificationSkipped and instead call
useQueryStates({ notificationSkipped: [false, { parse: parseAsBoolean }] })
(importing parseAsBoolean from 'nuqs' and useQueryStates from
'next-use-query-state' or project equivalent), then update all references and
setters (notificationSkipped and setNotificationSkipped) to use the value and
setter returned by useQueryStates; apply the same change at the other mentioned
spots (around the blocks noted at lines ~103-105 and ~112) so the UI rendering
logic relies on the URL boolean flag rather than component local state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9d98bdd-0da8-486f-8b0e-c5048a00b70a

📥 Commits

Reviewing files that changed from the base of the PR and between f5756da and 6cf5d5b.

📒 Files selected for processing (2)
  • src/components/Invites/JoinWaitlistPage.tsx
  • src/components/Setup/Views/InstallPWA.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant