-
Notifications
You must be signed in to change notification settings - Fork 137
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
[Woo POS][Cash & Receipts] Order completion call. Navigation to the payment success #13086
base: trunk
Are you sure you want to change the base?
Conversation
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
…all-from-the-mocked-cash-payment-screen' into 13084-woo-poscash-receipts-implement-order-completion-call-during-cash-payment-flow-with-navigation-to-the-payment-success # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt
…all-from-the-mocked-cash-payment-screen' into 13084-woo-poscash-receipts-implement-order-completion-call-during-cash-payment-flow-with-navigation-to-the-payment-success
site = selectedSite.get(), | ||
newStatus = statusModel, | ||
newPaymentMethodId = CASH_ON_DELIVERY_PAYMENT_TYPE, | ||
codGateway?.title ?: "Pay in Person", |
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.
codGateway?.title ?: "Pay in Person", | |
newPaymentMethodTitle = codGateway?.title ?: "Pay in Person", |
💡 np
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.
Looks good, @kidinov. I left some non-blocking comments.
I also noticed that the Totals screen is visible for a while before Payment successful is shown. Can we avoid that? See video:
Screen_recording_20241211_190849.mp4
@@ -196,11 +199,15 @@ fun WooPosTotalsPaymentCashScreenScreen() { | |||
enteredAmount = "5$", | |||
changeDue = "5$", | |||
total = "10$", | |||
canBeOrderBeCompleted = true, | |||
button = WooPosCashPaymentState.Collecting.Button( | |||
text = "Complete order", |
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.
💡 To Res
|
||
private fun handleOrderCompletion() { | ||
viewModelScope.launch { | ||
val stateBeforeCompleting = _state.value as? WooPosCashPaymentState.Collecting ?: return@launch |
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.
❓ Do we expect other states – shouldn't we crash? What will happen if we return here?
Closes: #13084
Description
Testing information
Try back navigation from the cash payment screen, too
The tests that have been performed
Above
Images/gif
12-09--11-06.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: