-
Notifications
You must be signed in to change notification settings - Fork 131
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
[POS] Simultaneous cash and card payment handling #13277
Changes from 7 commits
9deb05e
33dc451
4a506d8
5e5ec14
a87b61d
749b5f8
f61a258
629c416
5f9d68e
7ce4eea
929c42f
60ef6e2
6259911
cb990ff
da47b85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -278,9 +278,10 @@ class WooPosTotalsViewModel @Inject constructor( | |
uiState.value = InitialState | ||
} | ||
|
||
is ParentToChildrenEvent.OrderSuccessfullyPaid -> showSuccessfulPaymentState( | ||
event.paymentMethod | ||
) | ||
is ParentToChildrenEvent.OrderSuccessfullyPaid -> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking that it's ok that cancelation will be called also in the card reader payments cases There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The flow works correctly, but I switched to canceling payment intent only in case the payment success comes from cash payment to avoid redundant logic – 60ef6e2 |
||
cancelPaymentAction() | ||
showSuccessfulPaymentState(event.paymentMethod) | ||
} | ||
|
||
is ParentToChildrenEvent.ItemClickedInProductSelector -> Unit | ||
} | ||
|
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 you know if we need to have this code at all? Don't we handle the event in
WooPosHomeScreen
already?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.
The LaunchedEffect coroutine in WooPosHomeScreen is not active when we navigate to CashPayment screen. That's why I subscribed to the events in the CashPayment screen here again.
An alternative approach could be to move the subscription to navigation events up in the compose navigation hierarchy, to the
WooPosRootHost
, in order to avoid duplicate subscriptions – 5f9d68e. Let me know what you think, @kidinov !