Conversation
🦋 Changeset detectedLatest commit: 985b78c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRestructures payment UI: removes legacy pay-mode components, adds a new pay folder (Pay, Repay, Calculator, Overdue/UpcomingPayments, PaymentSheet updates), centralizes card-mode mutation options, updates navigation (tab icons/titles and haptics), adds i18n keys, and updates Maestro subflows and changesets. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Home
participant Calculator
participant PaymentSheet
participant Server
User->>Home: Open Payments screen
Home->>Server: Fetch market data & maturities
Server-->>Home: Market data + maturities
Home->>Home: Aggregate & sort maturities
User->>Home: Tap "Installments calculator"
Home->>Calculator: Navigate to /calculator
User->>Calculator: Enter amount
Calculator->>Server: Fetch installment rates
Server-->>Calculator: Rates
Calculator-->>User: Show installment options (BEST APR)
User->>Home: Tap payment item
Home->>PaymentSheet: Open details
PaymentSheet->>Server: Submit repay/rollover mutation
Server-->>PaymentSheet: Updated card details
PaymentSheet->>Home: Refresh & close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive overhaul of the application's payment management features. It focuses on enhancing the user experience by providing a more intuitive and feature-rich payments screen, including a dedicated calculator for installment planning. The changes also streamline underlying logic for card mode management and refresh the application's navigation aesthetics. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
home
branch
4 times, most recently
from
84b9ea9 to
943fa36
Compare
dieguezguille
force-pushed
the
home
branch
2 times, most recently
from
6e34c64 to
89412ce
Compare
dieguezguille
force-pushed
the
payments
branch
from
c90c1ce to
5977ffd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
home
branch
5 times, most recently
from
e574d09 to
1b889c2
Compare
dieguezguille
force-pushed
the
payments
branch
from
5977ffd to
161dcd6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
161dcd6 to
29e6706
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
29e6706 to
d14b4ad
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
d14b4ad to
c991ea7
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| if (maturity && !open) { | ||
| setDisplayMaturity(maturity); | ||
| setOpen(true); | ||
| } |
There was a problem hiding this comment.
🚩 RolloverIntroSheet uses setState-during-render instead of useEffect
At src/components/pay/RolloverIntroSheet.tsx:43-46, the component calls setDisplayMaturity and setOpen directly during the render phase. This is an officially supported React pattern (similar to getDerivedStateFromProps), and it works correctly — the condition maturity && !open prevents infinite loops. However, the sibling component src/components/pay/PaymentSheet.tsx:60-65 handles the identical scenario using useEffect. This inconsistency may confuse future maintainers. The render-phase pattern is also slightly more fragile: if any code path sets open = false without simultaneously clearing the parent's maturity prop, the sheet would immediately reopen on the next render.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const hasPayments = allMaturities.length > 0; | ||
| const firstMaturity = allMaturities[0]; |
There was a problem hiding this comment.
🚩 Empty state shown during initial data load on Pay screen
In src/components/pay/Pay.tsx:106-107, hasPayments is false when allMaturities is empty, which includes the initial loading state (before markets is fetched). This means the Empty component (src/components/pay/Empty.tsx) is displayed briefly while the previewer data loads, before payments appear. The RefreshControl spinner overlays this. Consider gating on markets being defined to distinguish 'loading' from 'no payments' — e.g., rendering a skeleton or nothing while loading.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary by CodeRabbit
New Features
UI/UX Improvements
Refactor
Documentation
Greptile Summary
This PR revamps the Payments screen with per-payment detail cards, early-repayment discount/late-fee info sheets, and a redesigned payment list with clearer dates and amounts. The core payment logic (
Repay.tsx,RepayAmountSelector.tsx) is solid, and thecardModeMutationOptionsrefactor (server.ts) cleanly centralizes optimistic-update logic.However, three issues require attention:
RolloverIntroSheet.tsx:setStateis called directly during render (lines 43–46) instead of insideuseEffect, inconsistent withPaymentSheet.tsxand risky under concurrent rendering.isLatestPluginguard for returning users: The plugin-version check was moved intoRolloverIntroSheetbut is skipped whenrolloverIntroShown=true. Returning users navigate to/roll-debtwithout the guard, creating a gap if/roll-debtlacks its own check."Payment due {{date}}, {{amount}}"and"Overdue payment {{date}}, {{amount}}"are used in code but not translated ines.json, affecting screen-reader users.Confidence Score: 2/5
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD TAB[Payments Tab] --> PAY[Pay.tsx] PAY --> EMPTY{Has payments?} EMPTY -- No --> EMPTYVIEW[Empty.tsx] EMPTY -- Yes --> HEADER[TotalOutstandingCard] HEADER --> FIRST[FirstMaturityCard earliest maturity] FIRST --> OVERDUE[OverduePayments excludeMaturity=first] OVERDUE --> UPCOMING[UpcomingPayments excludeMaturity=first] FIRST -- Pay --> REPAY[Repay.tsx] FIRST -- Rollover introShown=false --> ROLLINTRO[RolloverIntroSheet] FIRST -- Rollover introShown=true --> ROLLDEBT[roll-debt screen] ROLLINTRO -- isLatestPlugin true --> ROLLDEBT ROLLINTRO -- isLatestPlugin false --> TOAST[Upgrade toast] OVERDUE -- onSelect --> SHEET[PaymentSheet maturity param] UPCOMING -- onSelect --> SHEET SHEET -- Pay --> REPAY SHEET -- Rollover introShown=false --> PAY_ROLLINTRO[onRolloverIntro callback] PAY_ROLLINTRO --> ROLLINTRO SHEET -- Rollover introShown=true --> ROLLDEBT CALC[Calculator.tsx] -.->|entry from InstallmentsSheet| CALC PAY --> INFOSHEET[InfoSheet total/discount/fees]Comments Outside Diff (3)
src/components/pay/RolloverIntroSheet.tsx, line 43-46 (link)setStateis called directly during render (lines 43–46) instead of insideuseEffect. This is a React anti-pattern that is inconsistent with how the identical pattern is correctly handled inPaymentSheet.tsx(lines 46–51).Calling
setStateduring render can cause subtle bugs under React 18's concurrent rendering — the render can be interrupted and retried, potentially triggering the state update more than once.src/components/pay/PaymentSheet.tsx, line 106-114 (link)isLatestPluginguard was removed from the pre-navigation check for returning users. In the new code,isLatestPluginis only checked insideRolloverIntroSheet(which is only shown when!rolloverIntroShown). This means returning users (whererolloverIntroShown === true) bypass the plugin guard entirely and navigate directly to/roll-debtwithout the version check.The same gap exists in
Pay.tsx'sonRolloverhandler (line 161–167). If/roll-debtdoes not independently checkisLatestPlugin, users with outdated account plugins may encounter runtime errors instead of receiving the "Upgrade account to rollover" message.Consider re-adding the
isLatestPlugincheck to thenavigateToRollovercallback for all users, not just first-time users:src/components/pay/UpcomingPayments.tsx, line 115 (link)Accessibility aria-label key
"Payment due {{date}}, {{amount}}"(used here) is missing fromes.json. The same issue exists for"Overdue payment {{date}}, {{amount}}"inOverduePayments.tsx(line 120). Spanish-speaking users relying on screen readers will hear the raw English key string instead of a localized description.Add the following entries to
es.json:Last reviewed commit: 08feec0