Fix received transaction details to match iOS parity #486
Conversation
|
Warning Rate limit exceeded@praveenperera has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request restructures transaction details UI presentation across two Kotlin files. Changes include relocating confirmation and block number displays earlier in the layout flow for confirmed transactions, adding a loading indicator for confirmations, reorganizing the "Sent to" section to render only for sent transactions, and adjusting spacer and divider positioning accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 0
🧹 Nitpick comments (1)
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsWidget.kt (1)
88-93: Consider using NumberFormat for consistency with received transactions.The confirmations count uses
.toString()here, whileReceivedTransactionDetails.kt(line 61) usesNumberFormat.getNumberInstance().format()to add thousand separators. For large confirmation counts (e.g., 10,000+), the formatted version improves readability.🔎 Proposed change for consistent number formatting
+import java.text.NumberFormat + if (numberOfConfirmations != null) { Text( - numberOfConfirmations.toString(), + NumberFormat.getNumberInstance().format(numberOfConfirmations), color = sub, fontSize = 14.sp, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/ReceivedTransactionDetails.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsWidget.kt
🧰 Additional context used
📓 Path-based instructions (1)
android/app/src/main/java/org/bitcoinppl/cove/**/*.kt
⚙️ CodeRabbit configuration file
android/app/src/main/java/org/bitcoinppl/cove/**/*.kt:⚠️ CRITICAL FFI/Threading Policy - READ FIRST:
NEVER suggest moving Rust FFI calls to background threads (withContext(Dispatchers.IO))
Rust FFI calls are SYNCHRONOUS and FAST - they complete in microseconds
The Rust core uses Tokio runtime internally and handles all async operations
Database operations (redb) are optimized and do NOT block the UI thread
ONLY suggest Dispatchers.IO with profiling evidence showing >16ms UI blocking
If you see a Rust FFI call on the main thread, DO NOT FLAG IT - this is correct
Ignore generated bindings code in android/app/src/main/java/org/bitcoinppl/cove_core/**
Verify correct functionality and logic consistency.
Check for idiomatic Kotlin usage and performance best practices.
FFI Resource Management:
- Always verify that UniFFI objects implementing AutoCloseable call .close() before being nulled
- Example: Mnemonic must call .close() to trigger zeroization of sensitive data
- Use DisposableEffect for cleanup in Compose, never just null references
Compose Best Practices:
- LaunchedEffect should be keyed to actual dependencies, not Unit
- Set loading states at the beginning of LaunchedEffect blocks
Files:
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/ReceivedTransactionDetails.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsWidget.kt
⏰ 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). (7)
- GitHub Check: Greptile Review
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: compile-android
- GitHub Check: compile-ios
- GitHub Check: ktlint
- GitHub Check: clippy
🔇 Additional comments (5)
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsWidget.kt (2)
61-77: LGTM! Clean restructuring of the sent transaction path.The dedicated "Sent to" section for sent transactions is well-structured and maintains the existing functionality while improving code organization.
154-160: LGTM! Proper separation of concerns.Delegating received transaction details to a dedicated component maintains clean code organization and aligns with the PR objective to restructure the received transaction UI.
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/ReceivedTransactionDetails.kt (3)
50-89: Excellent implementation of iOS parity for confirmed transactions!The new structure perfectly matches the PR objectives:
- Confirmations and Block Number are displayed as separate rows at the top for confirmed transactions
- The loading indicator (lines 67-72) provides good UX feedback while confirmations are being fetched
NumberFormatusage ensures large confirmation counts are readable with thousand separators- The conditional at line 51 ensures pending transactions skip this block entirely
The implementation is clean, well-structured, and maintains visual consistency.
91-135: LGTM! Well-implemented address display with copy functionality.The "Received At" section correctly appears for all received transactions (both confirmed and pending), and the copy-to-clipboard functionality with visual feedback is intuitive and user-friendly.
138-143: LGTM! Proper LaunchedEffect usage.The effect is correctly keyed to
isCopied(following the coding guidelines to key on actual dependencies), and the 5-second auto-reset provides good user feedback without lingering too long.
Greptile SummaryRestructured received transaction details to match iOS layout by moving confirmations and block number above the address section, and removed the duplicate "Received from" label (now "Received At"). Key Changes:
Issue Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TransactionDetailsWidget
participant ReceivedTransactionDetails
participant TransactionDetails
User->>TransactionDetailsWidget: View transaction details
alt Transaction is Sent
TransactionDetailsWidget->>TransactionDetails: isSent()
TransactionDetails-->>TransactionDetailsWidget: true
TransactionDetailsWidget->>TransactionDetails: addressSpacedOut()
TransactionDetailsWidget->>TransactionDetails: blockNumberFmt()
TransactionDetailsWidget->>TransactionDetailsWidget: Display "Sent to" section
TransactionDetailsWidget->>TransactionDetailsWidget: Display network fee details
TransactionDetailsWidget->>TransactionDetailsWidget: Display total spent
else Transaction is Received
TransactionDetailsWidget->>TransactionDetails: isSent()
TransactionDetails-->>TransactionDetailsWidget: false
TransactionDetailsWidget->>ReceivedTransactionDetails: Render received details
alt Transaction is Confirmed
ReceivedTransactionDetails->>TransactionDetails: isConfirmed()
TransactionDetails-->>ReceivedTransactionDetails: true
ReceivedTransactionDetails->>ReceivedTransactionDetails: Display confirmations
ReceivedTransactionDetails->>TransactionDetails: blockNumberFmt()
ReceivedTransactionDetails->>ReceivedTransactionDetails: Display block number
end
ReceivedTransactionDetails->>TransactionDetails: addressSpacedOut()
ReceivedTransactionDetails->>ReceivedTransactionDetails: Display "Received At" with copy button
User->>ReceivedTransactionDetails: Click copy button
ReceivedTransactionDetails->>TransactionDetails: address().string()
ReceivedTransactionDetails->>ReceivedTransactionDetails: Copy to clipboard
ReceivedTransactionDetails-->>User: Show "Copied" feedback
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
...rg/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionDetails/ReceivedTransactionDetails.kt
Show resolved
Hide resolved
Remove duplicate "Received from" section that was incorrectly appearing for received transactions. Restructure ReceivedTransactionDetails to match iOS layout: show Confirmations and Block Number as separate rows at the top (for confirmed transactions), then "Received At" with address and copy button below. Previously Android was showing the address twice and using a combined "block | confirmations" row at the bottom, which didn't match iOS.
praveenperera
force-pushed
the
androi-parity-txn-details
branch
from
b38198e to
c1816ca
Compare
Summary
Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.