Convert Delegation Pattern to Completion Handler #293
Conversation
| if let error { | ||
| self.setUpdateSetupTokenFailureResult(vaultError: error) | ||
| } else if let result { | ||
| self.approveResultSuccessResult( |
There was a problem hiding this comment.
qq: should this method be named approveOrderSuccessResult()?
There was a problem hiding this comment.
That's a good call. There are many changes I want to make on the demo app, but I stuck to ones relevant to the conversion.
Like needing the loading progressView where I forgot them, to see if there were delays introduced in PayPal vault.
|
I do see how completion handlers could be useful though. We do have some time in the beta to figure out the best strategy. Ultimately offering both sounds like a viable option to me. |
|
| await paypalVaultViewModel.vault(setupTokenID: setupTokenID) | ||
| ZStack { | ||
| Button("Vault PayPal") { | ||
| Task { |
There was a problem hiding this comment.
random q: would it be cleaner if we created the Task within the paypalVaultViewModel.vault() method?
There was a problem hiding this comment.
Button actions are synchronous contexts, so I need to create a Task block there to call an async/await function.
There was a problem hiding this comment.
It makes sense what you are saying, make the views look cleaner. Let me add that to demo app refactor ticket.
CHANGELOG.md
Outdated
| * Remove `vault(vaultRequest:)` method that uses delegate callbacks | ||
| * Add `approveOrder(request:completion:(CardResult?, Error?) -> Void)` to `CardClient` | ||
| * Add `vault(request:completion:(CardVaultResult?, Error?) -> Void)` to `CardClient` | ||
| * Add `approveOrder(request:) async throws -> Cardresult` |
There was a problem hiding this comment.
| * Add `approveOrder(request:) async throws -> Cardresult` | |
| * Add `approveOrder(request:) async throws -> CardResult` |
There was a problem hiding this comment.
Thank you! Nice catch!
Summary of changes
Checklist
Authors