[Scan to Pay] Show Scan to Pay screen from Payments Methods #9747
Conversation
toupper
added
type: task
You can test the changes from this Pull Request by:
|
There was a problem hiding this comment.
This is looking good! When testing, I couldn't make work the last step (both simulator and physical device):
6. Tap on Done
7. See that the flow is dismissed you see the Order Created notice
The QR is dismissed and the order is created, but I don't see the Order Created notice, neither I'm redirected outside of the flow, it remains on the "Take payment" view 🤔
As the flow is not dismissed this causes confusion as you can go back, attempt a different payment method, or even swipe-down to dismiss/cancel order, but this has already been created.
| func generateQRCodeImage() -> UIImage? { | ||
| guard let paymentURLString = paymentURL?.absoluteString else { | ||
| return nil | ||
| } | ||
|
|
||
| let context = CIContext() | ||
| let filter = CIFilter.qrCodeGenerator() | ||
| filter.message = Data(paymentURLString.utf8) | ||
|
|
||
| guard let outputImage = filter.outputImage, | ||
| let cgImage = context.createCGImage(outputImage, from: outputImage.extent) else { | ||
| return nil | ||
| } | ||
|
|
||
| return UIImage(cgImage: cgImage) | ||
| } |
There was a problem hiding this comment.
Not a blocker: Should we abstract and extract this method somewhere else? Let's say an extension of UIImage, or as an Utility method in WooFoundation, then change the signature to something like the following generateQRCodeFromPaymentDetails(paymentURLString: URL, message: String) -> UIImage?
This way the view model doesn't have to know about the QR generation implementation details, we just pass the URL, the message, and let the system return it.
There was a problem hiding this comment.
Good point, I also considered that and preferred leaving it in the view model. The rationale is that at the moment we only generate a QR code here, and the function is simple enough to keep it in the view model. Furthermore, generating an image that we're going to display in the view from model data matches the responsibility of a view model.
If more logic is added to the QR code generation, e.g., adding the woo logo to it, I will extract it to its own class/extension.
| @@ -0,0 +1,74 @@ | |||
| import SwiftUI | |||
| import WooFoundation | |||
There was a problem hiding this comment.
| import WooFoundation |
Seems that this import is not needed! We only use FullScreenCoverClearBackgroundView on the PaymentMethodsView.
|
Thanks for your review @iamgabrielma !
I couldn't reproduce that behavior, but after taking a look at it together, we found a solution that was committed in edb450b. Please take a look at it now. |
Generated by 🚫 dangerJS |
|
Works as expected! 🚢 |
Closes: #9672
Description
With this PR we show the Scan to Pay screen with the QR code so it can be scanned.
Testing instructions
Screenshots
RELEASE-NOTES.txtif necessary.