Fix send flow UI issues on Android and add validation toasts #485
Conversation
|
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. WalkthroughIntroduces content-aware, multi-branch validation for the Send flow on Android and iOS with targeted inline feedback and focus handling; adds a new themed swipe-track background color and a small Android padding change for the amount input. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Send UI
participant Container as SendFlowContainer / ViewController
participant Validator as Validation logic
participant Snackbar as Inline Snackbar / FloaterPopup
participant Focus as FocusManager
rect rgb(230,242,255)
note right of Container: Triggered by Next / Swipe action
end
UI->>Container: NextAction()
Container->>Validator: validateAddress()
alt address invalid and has content
Validator-->>Container: addressInvalid
Container->>Snackbar: show "Address not valid. Please try again."
Container->>Focus: focus(ADDRESS)
Container-->>UI: stop
else address valid
Container->>Validator: validateAmount()
alt amount invalid and has content
Validator-->>Container: amountInvalid
Container->>Snackbar: show "Amount not valid. Please try again."
Container->>Focus: focus(AMOUNT)
Container-->>UI: stop
else amount valid
Container->>Container: dispatch(FinalizeAndGoToNextScreen)
Container-->>UI: proceed
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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: 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/SendFlow/EnterAmountView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowConfirmScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.ktandroid/app/src/main/java/org/bitcoinppl/cove/ui/theme/Color.ktios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.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/flows/SendFlow/SendFlowConfirmScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAmountView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.ktandroid/app/src/main/java/org/bitcoinppl/cove/ui/theme/Color.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/Flows/SendFlow/SendFlowSetAmountScreen.swift
🧬 Code graph analysis (1)
ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift (1)
ios/Cove/Flows/SendFlow/SendFlowManager.swift (2)
validateAddress(61-63)validateAmount(65-67)
⏰ 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: ktlint
- GitHub Check: clippy
- GitHub Check: test (macos-latest)
- GitHub Check: compile-android
- GitHub Check: compile-ios
🔇 Additional comments (4)
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowConfirmScreen.kt (1)
170-170: LGTM! Proper use of themed color for improved dark mode contrast.The change from
MaterialTheme.colorScheme.surfaceVarianttoMaterialTheme.coveColors.swipeTrackBgcorrectly applies the new themed color that matches iOS systemGray5, improving the swipe track visibility in dark mode as intended by the PR objectives.android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAmountView.kt (1)
124-124: LGTM! Padding prevents text overlap with dropdown.The horizontal padding addition correctly prevents the amount text from overlapping the "sats" dropdown when many digits are entered, matching the iOS layout as specified in the PR objectives.
android/app/src/main/java/org/bitcoinppl/cove/ui/theme/Color.kt (1)
36-38: LGTM! Proper theme color integration for cross-platform consistency.The new
swipeTrackBgcolor pair correctly implements the iOS systemGray5 equivalent (light: #E5E5EA, dark: #2C2C2E) and follows the established pattern for theme-aware colors in the codebase. The integration intoCoveColorSchemeand both light/dark color schemes is properly structured.Also applies to: 87-87, 94-94, 101-101
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.kt (1)
213-213: LGTM! Content-aware validation with proper user feedback.The new validation logic correctly implements the multi-branch, content-aware validation flow specified in the PR objectives:
- Validates address first, showing a snackbar only when the field contains content
- Then validates amount, showing a snackbar only when there's a non-zero amount
- Focuses the appropriate field on validation failure
- Advances to the next screen only when both inputs are valid
The implementation properly uses a coroutine scope for snackbar display and aligns with the iOS validation changes.
Also applies to: 233-266
Greptile SummaryImproved Send Flow validation UX by showing contextual toasts only when fields contain invalid content (not when empty), and fixed Android dark mode contrast for Swipe to Send button.
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UI as Send Flow UI
participant Validation as Validation Logic
participant Toast as Toast/Snackbar
User->>UI: Tap Next button
UI->>Validation: validateAddress()
alt Address invalid & has content
Validation-->>UI: false
UI->>Toast: Show "Address not valid" (Android: Snackbar, iOS: FloaterPopup)
Toast-->>User: Display for 1 second
UI->>UI: Focus address field
else Address invalid & empty
Validation-->>UI: false
UI->>UI: Focus address field (no toast)
else Address valid
Validation-->>UI: true
UI->>Validation: validateAmount()
alt Amount invalid & has content
Validation-->>UI: false
UI->>Toast: Show "Amount not valid"
Toast-->>User: Display for 1 second
UI->>UI: Focus amount field
else Amount invalid & empty
Validation-->>UI: false
UI->>UI: Focus amount field (no toast)
else Amount valid
Validation-->>UI: true
UI->>UI: Proceed to next screen
end
end
|
praveenperera
force-pushed
the
android-send-flow-tweak
branch
from
08b5870 to
edc0898
Compare
- Fix dark mode contrast for Swipe to Send by adding swipeTrackBg color matching iOS systemGray5 (#E5E5EA light, #2C2C2E dark) - Prevent amount text overlap with sats dropdown by adding horizontal padding - Show snackbar/floater popup on validation failure when field has content - Android: snackbar with validation message - iOS: FloaterPopup dismissing after 1 second
praveenperera
force-pushed
the
android-send-flow-tweak
branch
from
edc0898 to
9c11bd4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift (1)
67-70: Excellent refactoring to eliminate duplication.The extraction of validation logic into
performValidation()successfully addresses the code duplication concerns raised in previous reviews. The guard statement makes the success path clear and handles early returns cleanly.
🧹 Nitpick comments (2)
ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift (1)
77-110: Validation logic correctly implements PR requirements.The content-aware validation flow correctly:
- Shows FloaterPopup only when fields have content (address non-empty, amount non-zero)
- Dismisses FloaterPopup after 1 second as specified
- Focuses the appropriate field on validation failure
- Returns early to prevent progression when validation fails
The implementation matches the PR objectives and the Android validation behavior described in the context.
💡 Optional: Add documentation comment
Consider adding a brief doc comment to clarify the function's behavior:
/// Validates address and amount fields with content-aware feedback. /// - Returns: `true` if both validations pass, `false` otherwise. /// - Note: Shows FloaterPopup only when field has content; otherwise just sets focus. private func performValidation() -> Bool {This would help future maintainers understand the content-aware behavior at a glance.
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.kt (1)
233-266: LGTM! Content-aware validation logic correctly implements PR requirements.The multi-branch validation correctly implements the specified behavior:
- Invalid non-empty address → snackbar + focus ADDRESS
- Invalid non-empty amount → snackbar + focus AMOUNT
- Empty fields → focus only, no snackbar
- Both valid → proceed to next screen
The address-first validation order is reasonable for guiding users through corrections sequentially.
Optional: Extract duplicate snackbar logic
Proposed refactor to reduce duplication
The snackbar display logic in lines 242-247 and 254-259 is nearly identical. Consider extracting:
// Before the when block suspend fun showValidationError(message: String) { snackbarHostState.showSnackbar( message = message, duration = SnackbarDuration.Short, ) } // In the when block when { !addressValid -> { if (hasAddress) { validationScope.launch { showValidationError("Address not valid. Please try again.") } } presenter.focusField = SetAmountFocusField.ADDRESS } !amountValid -> { if (hasAmount) { validationScope.launch { showValidationError("Amount not valid. Please try again.") } } presenter.focusField = SetAmountFocusField.AMOUNT } else -> { sendFlowManager.dispatch(SendFlowManagerAction.FinalizeAndGoToNextScreen) } }
📜 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/SendFlow/EnterAmountView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowConfirmScreen.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.ktandroid/app/src/main/java/org/bitcoinppl/cove/ui/theme/Color.ktios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowConfirmScreen.kt
- android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAmountView.kt
🧰 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/ui/theme/Color.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.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/Flows/SendFlow/SendFlowSetAmountScreen.swift
🧬 Code graph analysis (1)
ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift (1)
ios/Cove/Flows/SendFlow/SendFlowManager.swift (2)
validateAddress(61-63)validateAmount(65-67)
⏰ 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: clippy
- GitHub Check: test (ubuntu-latest)
- GitHub Check: compile-ios
- GitHub Check: ktlint
- GitHub Check: test (macos-latest)
- GitHub Check: compile-android
🔇 Additional comments (3)
android/app/src/main/java/org/bitcoinppl/cove/ui/theme/Color.kt (2)
36-38: LGTM! iOS color parity correctly implemented.The swipe track background colors correctly match iOS systemGray5 values and will improve dark mode contrast on Android as intended.
87-87: LGTM! Theme-aware color integration follows existing pattern.The swipeTrackBg field is correctly integrated into the color scheme with proper light/dark mode initialization.
Also applies to: 94-94, 101-101
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.kt (1)
213-213: LGTM! Correct coroutine scope for validation feedback.The validationScope is properly created using
rememberCoroutineScope()and is composition-aware, making it suitable for launching snackbar display coroutines.
|
@greptile-apps re-review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Task { | ||
| await FloaterPopup( | ||
| text: "Address not valid. Please try again.", | ||
| backgroundColor: .orange, | ||
| textColor: .white, | ||
| iconColor: .white, | ||
| icon: "exclamationmark.triangle" | ||
| ).dismissAfter(1).present() | ||
| } |
There was a problem hiding this comment.
logic: Task creates detached async context - popup may not show before view dismisses/navigates
| Task { | |
| await FloaterPopup( | |
| text: "Address not valid. Please try again.", | |
| backgroundColor: .orange, | |
| textColor: .white, | |
| iconColor: .white, | |
| icon: "exclamationmark.triangle" | |
| ).dismissAfter(1).present() | |
| } | |
| if !address.wrappedValue.isEmpty { | |
| await FloaterPopup( | |
| text: "Address not valid. Please try again.", | |
| backgroundColor: .orange, | |
| textColor: .white, | |
| iconColor: .white, | |
| icon: "exclamationmark.triangle" | |
| ).dismissAfter(1).present() | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift
Line: 80:88
Comment:
**logic:** `Task` creates detached async context - popup may not show before view dismisses/navigates
```suggestion
if !address.wrappedValue.isEmpty {
await FloaterPopup(
text: "Address not valid. Please try again.",
backgroundColor: .orange,
textColor: .white,
iconColor: .white,
icon: "exclamationmark.triangle"
).dismissAfter(1).present()
}
```
How can I resolve this? If you propose a fix, please make it concise.Make performValidation() async and properly await FloaterPopup calls to ensure popups display before any potential navigation occurs.
Summary
swipeTrackBgcolor matching iOSsystemGray5(#E5E5EA light, #2C2C2E dark)Test plan
Summary by CodeRabbit
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.