-
Notifications
You must be signed in to change notification settings - Fork 1k
ConfirmButton refactor 4/6 - Move update logic #5930
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
Conversation
| private(set) var status: Status = .enabled | ||
| private(set) var callToAction: CallToActionType |
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.
These properties are moved to the BuyButton
| self.status = status | ||
| self.callToAction = callToAction | ||
|
|
||
| // Enable/disable | ||
| isUserInteractionEnabled = (status == .enabled || status == .disabled) | ||
|
|
||
| // Update the buy button; it has its own presentation logic | ||
| self.buyButton.update(status: status, callToAction: callToAction, animated: animated) | ||
|
|
||
| if let completion = completion { | ||
| let delay: TimeInterval = { | ||
| guard animated else { | ||
| return 0 | ||
| } | ||
|
|
||
| return status == .succeeded | ||
| ? PaymentSheetUI.delayBetweenSuccessAndDismissal | ||
| : PaymentSheetUI.defaultAnimationDuration | ||
| }() | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: completion) | ||
| } |
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.
This function and the old BuyButton.update functions are unified into a single BuyButton.update function. For now the ConfirmButton.update functions are just wrappers that pass the data to the BuyButton to complete all logic
| return appearance.primaryButton.successBackgroundColor | ||
| } | ||
|
|
||
| private var status: Status = .enabled |
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.
Before, the ConfirmButton and BuyButton each had their own status property that needed to be kept in sync. Now there's only one, which is the BuyButton's
| self.callToAction = callToAction | ||
|
|
||
| // Enable/disable | ||
| isUserInteractionEnabled = (status == .enabled || status == .disabled) |
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.
This is from the old ConfirmButton.update function
|
|
||
| if let completion = completion { | ||
| let delay: TimeInterval = { | ||
| guard animated else { | ||
| return 0 | ||
| } | ||
|
|
||
| return status == .succeeded | ||
| ? PaymentSheetUI.delayBetweenSuccessAndDismissal | ||
| : PaymentSheetUI.defaultAnimationDuration | ||
| }() | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: completion) | ||
| } |
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.
This is from the old ConfirmButton.update function
|
✅ Dead code has been resolved in this PR. [find-dead-code] |
Summary
Part 4/x of a factor of the ConfirmButton. Previously the ConfirmButton class supported both the Apple Pay button and confirm buttons, but now it only supports confirm buttons and can be simplified, improve code clarity and SDK binary size.
Draft PR showing the whole change: #5678.
This PR flattens the update code to live directly in the BuyButton. This sets up the BuyButton implementation to replace the ConfirmButton wrapper and itself become the ConfirmButton
Motivation
Testing
Changelog