-
Notifications
You must be signed in to change notification settings - Fork 8
Fix Android transaction list state issues and 24 word import #487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Use proper alert states for duplicate and successful wallet imports:
- Change from AppAlertState.General to AppAlertState.DuplicateWallet
for duplicate wallets (removes wallet ID from message)
- Change from AppAlertState.General to AppAlertState.ImportedSuccessfully
for successful imports
- Add navigation to wallet on OK press in ImportedSuccessfully handler
- Remove premature popRoute() calls that caused race conditions
This ensures consistent messaging ("Duplicate Wallet" / "This wallet has
already been imported!") and reliable navigation to the wallet after
pressing OK, matching iOS behavior.
Add popRoute() before setting alertState in QR scan screens so the scanner is dismissed before showing success/duplicate/error alerts, matching iOS behavior.
- Fix AvailableTransactions handler to update state when SCANNING or LOADED with same or more transactions (prevents flicker on refresh) - Show inline spinner during scan except for initial loading state - Add loading popup when tapping transaction details - Enhance empty state with icon and description - Fix unsigned transaction amount and pending signature opacity - Add divider after every transaction including last one - Add string resource for empty state description 🤖 Generated with [Claude Code](https://claude.com/claude-code)
WalkthroughThese changes enhance transaction state management in WalletManager (Android and iOS) to conditionally transition based on transaction list size, introduce tabbed pagination UI for 24-word seed imports with page indicators, redesign the empty transaction state with improved visual hierarchy, adjust UI parameters, and add a new string resource. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Greptile SummaryThis PR fixes transaction list state management issues on Android and adds paginated 24-word wallet import functionality to match iOS behavior. Key Changes:
iOS/Android Parity: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ImportScreen
participant WordInputGrid
participant WalletManager
participant TransactionView
Note over User,WordInputGrid: 24-Word Import Flow
User->>ImportScreen: Enter 24-word recovery phrase
ImportScreen->>ImportScreen: Initialize tabIndex = 0
User->>WordInputGrid: Focus on word field
WordInputGrid->>ImportScreen: onFocusChanged(fieldIndex)
ImportScreen->>ImportScreen: Calculate newTab = fieldIndex / 12
ImportScreen->>WordInputGrid: Update tabIndex (auto-switch page)
WordInputGrid->>WordInputGrid: Display words for current page
Note over WalletManager,TransactionView: Transaction State Management
WalletManager->>WalletManager: StartedInitialFullScan
WalletManager->>WalletManager: loadState = LOADING
TransactionView->>TransactionView: Show large spinner
WalletManager->>WalletManager: AvailableTransactions(txns)
alt loadState is LOADING
WalletManager->>WalletManager: loadState = SCANNING(txns)
else loadState is SCANNING
alt txns.size >= current.txns.size
WalletManager->>WalletManager: loadState = SCANNING(txns)
else
WalletManager->>WalletManager: Ignore update
end
else loadState is LOADED
alt txns.size >= current.txns.size
WalletManager->>WalletManager: loadState = SCANNING(txns)
else
WalletManager->>WalletManager: Keep LOADED state
end
end
TransactionView->>TransactionView: Show inline spinner if scanning
WalletManager->>WalletManager: ScanComplete(txns)
WalletManager->>WalletManager: loadState = LOADED(txns)
TransactionView->>TransactionView: Hide spinner, show transactions
User->>TransactionView: Click transaction
TransactionView->>TransactionView: app.alertState = Loading
TransactionView->>WalletManager: transactionDetails(txId)
WalletManager-->>TransactionView: Return details
TransactionView->>TransactionView: app.alertState = null
TransactionView->>TransactionView: Navigate to details
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/NewWalletSelectScreen.kt (1)
112-140: Close the Wallet object to ensure resource cleanup.The
Walletobject created at line 114 is never explicitly closed. Kotlin objects implement AutoCloseable by default, and the recommended way to automatically close resources is to use the inline extension function use(). Wrap the wallet usage in auseblock:Wallet.newFromXpub(xpub = content.trim()).use { wallet -> val id = wallet.id() android.util.Log.d("NewWalletSelectScreen", "Imported Wallet: $id") app.rust.selectWallet(id = id) } app.alertState = TaggedItem(AppAlertState.ImportedSuccessfully)
🧹 Nitpick comments (2)
android/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.kt (1)
270-296: Consider movingstartActivitycall outsideDispatchers.IO.While file I/O correctly runs on
Dispatchers.IO,startActivityis typically called from the main thread for consistency. The current approach works but could be cleaner.🔎 Suggested improvement
private suspend fun shareTransactionsFile( context: Context, manager: WalletManager, ) { val result = manager.rust.exportTransactionsCsv() - withContext(Dispatchers.IO) { + val uri = withContext(Dispatchers.IO) { val file = File(context.cacheDir, result.filename) file.writeText(result.content) - val uri = - FileProvider.getUriForFile( - context, - "${context.packageName}.fileprovider", - file, - ) + FileProvider.getUriForFile( + context, + "${context.packageName}.fileprovider", + file, + ) + } - val intent = - Intent(Intent.ACTION_SEND).apply { - type = "text/csv" - putExtra(Intent.EXTRA_STREAM, uri) - addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - } + val intent = Intent(Intent.ACTION_SEND).apply { + type = "text/csv" + putExtra(Intent.EXTRA_STREAM, uri) + addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) + } - context.startActivity(Intent.createChooser(intent, "Share Transactions")) - } + context.startActivity(Intent.createChooser(intent, "Share Transactions")) }android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.kt (1)
202-210: LGTM: Stable keys for Compose list items.Switching from
itemsIndexedtoitemswith stable keys derived from transaction IDs is a best practice for Compose. This ensures proper recomposition behavior and prevents incorrect reuse of composables when the list changes.The keys assume transaction IDs are unique across both confirmed and unconfirmed transactions. If there's a scenario where the same transaction can temporarily exist in both states (e.g., during a state transition), consider prefixing the key with the transaction type to guarantee uniqueness:
key = { when (it) { is Transaction.Confirmed -> "confirmed-${it.v1.id()}" is Transaction.Unconfirmed -> "unconfirmed-${it.v1.id()}" } }This is likely unnecessary but provides additional safety.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
android/app/src/main/java/org/bitcoinppl/cove/AppAlertState.ktandroid/app/src/main/java/org/bitcoinppl/cove/AppManager.ktandroid/app/src/main/java/org/bitcoinppl/cove/MainActivity.ktandroid/app/src/main/java/org/bitcoinppl/cove/WalletManager.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/NewWalletSelectScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/ColdWalletQrScanScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/QrCodeImportScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportCard.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.ktandroid/app/src/main/java/org/bitcoinppl/cove/views/ImageButton.ktandroid/app/src/main/java/org/bitcoinppl/cove/views/QrExportView.ktandroid/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.ktandroid/app/src/main/java/org/bitcoinppl/cove_core/cove.ktandroid/app/src/main/res/values/strings.xmlios/Cove/WalletManager.swift
🧰 Additional context used
📓 Path-based instructions (2)
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/views/ImageButton.ktandroid/app/src/main/java/org/bitcoinppl/cove/WalletManager.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/ColdWalletQrScanScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.ktandroid/app/src/main/java/org/bitcoinppl/cove/AppManager.ktandroid/app/src/main/java/org/bitcoinppl/cove/views/QrExportView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportCard.ktandroid/app/src/main/java/org/bitcoinppl/cove/AppAlertState.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/NewWalletSelectScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/QrCodeImportScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/MainActivity.kt
ios/Cove/**/*.swift
⚙️ CodeRabbit configuration file
ios/Cove/**/*.swift: - Review SwiftUI view code for proper layout, best practices
- Identify spelling mistakes in comments, string literals, and documentation
- Ignore generated bindings code in ios/CoveCore/Sources/CoveCore/generated/**
Files:
ios/Cove/WalletManager.swift
⏰ 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: compile-android
- GitHub Check: test (macos-latest)
- GitHub Check: clippy
- GitHub Check: test (ubuntu-latest)
- GitHub Check: compile-ios
- GitHub Check: ktlint
🔇 Additional comments (30)
android/app/src/main/java/org/bitcoinppl/cove/views/ImageButton.kt (1)
72-72: LGTM! Improved readability.Increasing the minimum font size from 7.sp to 12.sp ensures better readability and accessibility. The previous 7.sp minimum was too small to be legible.
android/app/src/main/java/org/bitcoinppl/cove/WalletManager.kt (1)
227-241: The code design is sound and correctly handles all transaction update scenarios. TheAvailableTransactionsmessage uses a size comparison because it represents progressive blockchain discovery, which should monotonically increase or remain stable. When transaction content changes without the count changing—such as transaction replacements—the separateUpdatedTransactionsmessage (lines 245-253) handles it unconditionally, without size restrictions. This two-message design is intentional and appropriate:AvailableTransactionsmanages discovery progress whileUpdatedTransactionsmanages content updates. No verification needed.Likely an incorrect or invalid review comment.
android/app/src/main/res/values/strings.xml (1)
49-49: LGTM!Clear, friendly messaging for the empty transaction state that aligns well with the app's user experience.
ios/Cove/WalletManager.swift (1)
162-172: LGTM! State transition logic prevents transaction count regression.The refactored logic correctly handles transaction state updates by only transitioning to
.scanningwhen the new transaction count is greater than or equal to the current count. This defensive pattern prevents UI flicker and state inconsistencies when transaction counts temporarily decrease during scanning.android/app/src/main/java/org/bitcoinppl/cove/views/QrExportView.kt (2)
105-120: LGTM! Typography and layout improvements enhance readability.The changes improve the visual hierarchy with larger text styles and better spacing. The
heightIn(min = 500.dp)ensures consistent minimum sizing for the export view.
165-187: LGTM! Excellent responsive sizing implementation.The refactor improves the QR display with:
BoxWithConstraintsenables responsive sizing based on available widthremember(qrString)efficiently memoizes bitmap generation, preventing unnecessary regenerationsize(qrSize)withContentScale.FillBoundsmaintains proper square aspect ratioThe combination ensures the QR code scales correctly while maintaining performance through proper use of Compose's memoization patterns.
android/app/src/main/java/org/bitcoinppl/cove_core/cove.kt (5)
2152-2157: LGTM - Async function signature changes are consistent.The shift from
RustBuffer.ByValuereturns toLong(future handle) forexportandexportToBbqrWithDensityaligns with UniFFI's async Rust future pattern. The removal ofuniffi_out_errparameter is correct since async functions propagate errors through the future completion mechanism.
3562-3570: LGTM - Checksum updates reflect updated native signatures.The updated checksum values for
labelmanager_export(24203) andlabelmanager_export_to_bbqr_with_density(50284) correctly correspond to the new async function signatures in the native library.
12788-12795: LGTM - Suspend function declarations enable proper async support.The interface methods are correctly declared as
suspendfunctions, enabling Kotlin coroutine integration for the async Rust FFI calls.
12928-12946: LGTM - Async implementation follows UniFFI conventions.The
export()implementation correctly usesuniffiRustCallAsyncwith appropriate:
- Future polling via
ffi_cove_rust_future_poll_rust_buffer- Completion via
ffi_cove_rust_future_complete_rust_buffer- Resource cleanup via
ffi_cove_rust_future_free_rust_buffer- String lifting via
FfiConverterString.lift- Error propagation via
LabelManagerException.ErrorHandler
12965-12983: LGTM - Consistent async implementation for QR export.The
exportToBbqrWithDensity()implementation correctly:
- Lowers the
densityparameter viaFfiConverterTypeQrDensity.lower()- Uses the same async future infrastructure as
export()- Lifts the result to
List<String>viaFfiConverterSequenceString.liftThe implementation is consistent with the other async method and follows UniFFI conventions.
android/app/src/main/java/org/bitcoinppl/cove/MainActivity.kt (2)
562-578: LGTM! Non-dismissable loading dialog implementation.The empty
onDismissRequest = {}correctly prevents users from dismissing the loading indicator during blocking operations. The UI is clean with a centered spinner and message.
580-594: LGTM! Navigation on successful import.The OK button correctly dismisses the alert and navigates to the imported wallet when available. The null-safe handling with
?.letensures graceful fallback if no wallet is selected.android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/QrCodeImportScreen.kt (2)
73-77: LGTM! Consistent route-pop-then-alert pattern.The refactored flow correctly pops the scanner route before showing alerts, ensuring proper navigation state. Using
AppAlertState.ImportedSuccessfullyandAppAlertState.DuplicateWalletaligns with the unified alert system.
78-90: LGTM! Error handling follows consistent pattern.Both the generic exception handler and the invalid QR code path correctly pop the route before displaying alerts, maintaining consistent UX across all outcomes.
android/app/src/main/java/org/bitcoinppl/cove/AppManager.kt (1)
580-582: LGTM! Clean refactor to dedicated Loading state.Replacing the verbose
AppAlertState.General(title="Working on it...", message="")with the dedicatedAppAlertState.Loadingis semantically clearer and aligns with the new alert state architecture.android/app/src/main/java/org/bitcoinppl/cove/AppAlertState.kt (2)
94-96: LGTM! Well-designed Loading state.Using
data objectis appropriate for a singleton state with no parameters. The placement near the comment "// loading popup with progress indicator" clearly documents its purpose.
136-136: LGTM! Consistent title/message implementations.The
title()andmessage()functions are correctly extended for the Loading state. Returning an empty message is appropriate since the loading dialog displays its own "Working on it..." text.Also applies to: 166-166
android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportScreen.kt (2)
118-126: LGTM! Well-implemented tab auto-switching.The
LaunchedEffect(focusedField)correctly auto-switches pages when focus moves to a different word group. The boundary checknewTab < numberOfGroupsprevents invalid tab indices.
308-334: LGTM! Clean page indicator implementation.The dot indicators correctly reflect the current page and support manual navigation via clicks. The conditional rendering for 24-word imports only is appropriate.
android/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.kt (1)
90-97: LGTM! Error handling for transaction export.The try/catch properly handles export failures and provides user feedback via snackbar with the localized error message.
android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/cold_wallet/ColdWalletQrScanScreen.kt (1)
83-88: LGTM! Consistent import feedback pattern.The success and duplicate wallet cases follow the same pattern as
QrCodeImportScreen.kt: pop the scanner route, then show the appropriate alert. This ensures consistent UX across cold wallet import flows.android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportCard.kt (2)
56-56: LGTM! Clean parameter addition.Adding
tabIndexparameter enables the pagination feature while maintaining backward compatibility with the existing word handling logic.
68-73: LGTM! Correct pagination logic.The pagination calculation correctly handles both 12-word (single page) and 24-word (two pages) imports:
pageStart = tabIndex * pageSizecorrectly offsets for each pagecoerceAtMost(wordCount)safely bounds the right column indices- 6 words per column matches the iOS layout
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.kt (6)
118-119: LGTM: Improved loading state logic.The refined condition correctly shows the inline spinner only during subsequent scans, while the initial loading state displays the larger centered spinner (line 146). This provides clearer visual feedback to users.
153-177: LGTM: Enhanced empty state UI.The redesigned empty state with the Bitcoin icon, primary label, and encouraging subtitle provides better visual feedback and user guidance compared to the previous plain text display.
378-394: LGTM: Proper loading state management.The loading state pattern is clean and consistent:
- Set
AppAlertState.Loadingbefore the operation- Perform the FFI call
- Clear the alert state on both success and error paths
The Rust FFI call on the main thread is correct per the coding guidelines—these calls are synchronous and fast, completing in microseconds.
481-497: LGTM: Consistent loading pattern.This implements the same clean loading state pattern as
ConfirmedTransactionWidget, ensuring consistent UX across both confirmed and unconfirmed transaction interactions.
667-667: LGTM: Enhanced visibility for pending state.The more pronounced orange color with 80% alpha improves visibility of the pending signature indicator, making it clearer to users that action is required.
676-676: LGTM: Improved readability for unsigned amounts.Displaying unsigned transaction amounts at full opacity (removing the alpha fade) improves readability and maintains consistency with the enhanced visibility theme across the pending transaction UI.
Paginate 24-word import to match iOS behavior (12 words per page) and raise ImageButton minFontSize from 7.sp to 12.sp to prevent excessive shrinking. - Add tabIndex state and page indicator dots for 24-word import - Auto-switch pages when focus moves between word groups - WordInputGrid now shows 6 words per column regardless of total
f23bee5 to
d5e8c57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportCard.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.ktandroid/app/src/main/java/org/bitcoinppl/cove/views/ImageButton.ktandroid/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportScreen.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/NewWalletFlow/hot_wallet/HotWalletImportCard.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.ktandroid/app/src/main/java/org/bitcoinppl/cove/views/ImageButton.ktandroid/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.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). (6)
- GitHub Check: test (macos-latest)
- GitHub Check: clippy
- GitHub Check: test (ubuntu-latest)
- GitHub Check: ktlint
- GitHub Check: compile-ios
- GitHub Check: compile-android
🔇 Additional comments (8)
android/app/src/main/java/org/bitcoinppl/cove/views/ImageButton.kt (1)
72-72: LGTM!Increasing
minFontSizefrom 7.sp to 12.sp improves readability and accessibility by preventing text from shrinking to an unreadable size.android/app/src/main/java/org/bitcoinppl/cove/flows/NewWalletFlow/hot_wallet/HotWalletImportCard.kt (1)
56-56: LGTM!The pagination logic correctly handles both 12-word and 24-word seed phrases. The index calculations properly map each page to 6 words per column (12 words per page), and
coerceAtMost(wordCount)safely bounds the right column indices.Also applies to: 68-73
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/TransactionsCardView.kt (6)
117-118: LGTM!The spinner logic correctly avoids showing the inline spinner during the initial first scan when no transactions exist (since a large centered spinner is shown instead). This prevents duplicate loading indicators.
152-178: LGTM!The redesigned empty state provides a clear visual hierarchy with a Bitcoin icon and helpful messaging.
203-224: Good improvement using stable keys.Using transaction IDs as keys instead of list indices prevents unnecessary recompositions when the transaction list changes and ensures proper state preservation.
378-395: LGTM!The loading state management correctly wraps the async operation, setting
Loadingat the start and clearing it in both success and error paths. Per coding guidelines, the Rust FFI call doesn't requireDispatchers.IO.
482-498: LGTM!Consistent loading state pattern with
ConfirmedTransactionWidget.
666-680: LGTM!The color adjustments improve visual consistency - the pending signature text now uses a matching orange tone with the warning indicator, and the amount text is more prominent.
android/app/src/main/java/org/bitcoinppl/cove/wallet/WalletSheets.kt
Outdated
Show resolved
Hide resolved
|
@greptile-apps re-review |
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.