smol fix and thought writeup

[Hugo0]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
peanut-wallet	Ready	Preview	Comment	Oct 16, 2025 6:44pm

[coderabbitai]

Walkthrough

This PR adds a comprehensive legacy web push cleanup plan documentation detailing migration from VAPID/web-push to OneSignal, and modifies TransactionCard component styling to conditionally apply line-through text decoration based on the hasPerk condition for currency display amounts.

Changes

Cohort / File(s)	Change Summary
Documentation docs/LEGACY_WEBPUSH_CLEANUP.md	New document outlining migration plan phases (preparation, code changes, dependency cleanup, testing, deployment) with risk assessment, benefits, success criteria, and timeline
Component Styling src/components/TransactionDetails/TransactionCard.tsx	Adds conditional line-through style to currencyDisplayAmount span when hasPerk is true, mirroring existing displayAmount styling behavior

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The diff contains heterogeneous changes with varying complexity: documentation additions (trivial) and component styling logic modifications (medium). Understanding the styling conditional and its visual implications requires separate reasoning.

Possibly related PRs

• fix: polish+sizing+breakpoints #877: Modifies TransactionCard.tsx rendering logic for amount/display styling.
• fix: more and more bugssssss #1030: Updates TransactionCard.tsx with changes to amount span styling conditionals and prop propagation.

Suggested labels

enhancement

Suggested reviewers

• jjramirezn
• kushagrasarathe

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	There is no pull request description, so reviewers lack context for the addition of the migration plan document and the UI styling change. A minimal summary is needed to explain the purpose and scope of these edits.	Please add a brief description that outlines the Legacy Web Push Cleanup Plan’s objectives and the conditional styling fix in TransactionCard to ensure reviewers understand the intent and impact of this PR.
Title Check	❓ Inconclusive	The title "smol fix and thought writeup" is vague and does not clearly convey the primary changes, namely the addition of a legacy web push cleanup plan document and the conditional line-through styling update in TransactionCard. It uses casual slang and fails to summarize the main content of the PR.	Please update the title to succinctly describe the key changes, for example “Add Legacy Web Push Cleanup Plan and apply line-through style in TransactionCard when hasPerk,” so that reviewers can immediately understand the PR’s scope.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/pts_fixes

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/components/TransactionDetails/TransactionCard.tsx (1)

171-173: LGTM! Consistent perk styling applied.

The conditional line-through styling correctly mirrors the displayAmount behavior (lines 165-169), ensuring both USD and currency amounts show as crossed out when a perk is claimed.

Optional: Consider consistent styling approach.

For consistency, you could align the styling patterns. Currently:

• displayAmount (lines 165-169): Uses ternary operator with two separate <span> elements
• currencyDisplayAmount (lines 171-173): Uses template literal with conditional class

Both work fine, but using the same pattern throughout would slightly improve readability.

docs/LEGACY_WEBPUSH_CLEANUP.md (1)

1-220: Excellent technical planning document!

This cleanup plan is comprehensive, well-structured, and provides clear actionable steps for removing the legacy VAPID push notification system. The document includes:

• Clear context and rationale
• Detailed file-level changes with code examples
• Phased migration approach
• Risk assessment with mitigation strategies
• Success criteria and timeline

Optional: Consider adding explicit rollback procedures.

While you mention "Have rollback plan ready" in the Risk Assessment (line 185), consider adding a dedicated "Rollback Plan" section with specific steps. This would be helpful if issues arise in production:

• Git commands to revert the changes
• Environment variable restoration steps
• Expected behavior after rollback
• Who to notify if rollback is needed

Note: The LanguageTool grammar hints flagging QB_NEW_EN are false positives related to Markdown list formatting and can be safely ignored.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40fadb9 and db36e89.

📒 Files selected for processing (2)

• docs/LEGACY_WEBPUSH_CLEANUP.md (1 hunks)
• src/components/TransactionDetails/TransactionCard.tsx (1 hunks)

🧰 Additional context used

🪛 LanguageTool

docs/LEGACY_WEBPUSH_CLEANUP.md

[grammar] ~16-~16: There might be a mistake here.
Context: ...initialization and permission management - Used in: Home page, various prompts th...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...page, various prompts throughout the app - Features: Permission modals, reminder ...

(QB_NEW_EN)

---

[grammar] ~22-~22: There might be a mistake here.
Context: ...ins subscribeUser, unsubscribeUser, sendNotification - Context: `src/context/pushProvider.tsx...

(QB_NEW_EN)

---

[grammar] ~23-~23: There might be a mistake here.
Context: ...es VAPID-based push subscription context - Usage: Only used in `src/components/Gl...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: ...d of usePush 2. Remove PushProvider from context tree 3. Delete `src/context/pus...

(QB_NEW_EN)

---

[grammar] ~150-~150: There might be a mistake here.
Context: ...e web-push and @types/web-push from package.json 2. Run pnpm install to clean up `pnpm-loc...

(QB_NEW_EN)

---

[grammar] ~160-~160: There might be a mistake here.
Context: ...Signal notifications still work 5. Test on staging environment ### Phase 5: Deplo...

(QB_NEW_EN)

---

[grammar] ~171-~171: There might be a mistake here.
Context: ... Risk Assessment Risk Level: 🟢 Low Reasons: - Legacy code is isolated an...

(QB_NEW_EN)

---

[grammar] ~183-~183: There might be a mistake here.
Context: ...st thoroughly on staging first - Deploy during low-traffic period - Monitor Sentry for...

(QB_NEW_EN)

---

[grammar] ~185-~185: There might be a mistake here.
Context: ...ic period - Monitor Sentry for errors - Have rollback plan ready ## Benefits 1. **...

(QB_NEW_EN)

---

[grammar] ~189-~189: There might be a mistake here.
Context: ...KB from client bundle (web-push library) 2. Code Quality: Remove 200+ lines of unu...

(QB_NEW_EN)

---

[grammar] ~190-~190: There might be a mistake here.
Context: ...lity**: Remove 200+ lines of unused code 3. Maintenance: One less system to mainta...

(QB_NEW_EN)

---

[grammar] ~191-~191: There might be a mistake here.
Context: ...intenance**: One less system to maintain 4. Clarity: Clear separation - OneSignal ...

(QB_NEW_EN)

---

[grammar] ~197-~197: There might be a mistake here.
Context: ...ia - [ ] All VAPID-related code removed - [ ] All tests passing - [ ] "Get notifie...

(QB_NEW_EN)

---

[grammar] ~198-~198: There might be a mistake here.
Context: ...ted code removed - [ ] All tests passing - [ ] "Get notified" feature works with On...

(QB_NEW_EN)

---

[grammar] ~199-~199: There might be a mistake here.
Context: ...t notified" feature works with OneSignal - [ ] No VAPID-related errors in productio...

(QB_NEW_EN)

---

[grammar] ~200-~200: There might be a mistake here.
Context: ... ] No VAPID-related errors in production - [ ] Bundle size reduced - [ ] Documentat...

(QB_NEW_EN)

---

[grammar] ~201-~201: There might be a mistake here.
Context: ... in production - [ ] Bundle size reduced - [ ] Documentation updated ## Timeline ...

(QB_NEW_EN)

---

[grammar] ~210-~210: There might be a mistake here.
Context: ...ded schedule**: 1. Code changes: 1 hour 2. Testing: 1 hour 3. Staging deployment: 3...

(QB_NEW_EN)

---

[grammar] ~211-~211: There might be a mistake here.
Context: ... Code changes: 1 hour 2. Testing: 1 hour 3. Staging deployment: 30 min 4. Production...

(QB_NEW_EN)

---

[grammar] ~212-~212: There might be a mistake here.
Context: ...ng: 1 hour 3. Staging deployment: 30 min 4. Production deployment: 30 min 5. Monitor...

(QB_NEW_EN)

---

[grammar] ~213-~213: There might be a mistake here.
Context: ... 30 min 4. Production deployment: 30 min 5. Monitoring: 1 hour --- _Created: 2025-...

(QB_NEW_EN)

⏰ 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