-
Notifications
You must be signed in to change notification settings - Fork 995
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
Add ability to toggle edit mode for vertical mode #3576
Changes from 10 commits
623cfd9
d1d2a17
355620b
0f1958a
4e2b58f
48f149a
0bead0c
c2ea128
a8078c0
deb23c0
fe7b63f
a5afc84
af76619
5005f65
4a087a9
676eeff
44825ce
f12e4dd
8af1de1
09c5a7d
98130cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,31 @@ class VerticalSavedPaymentMethodsViewController: UIViewController { | |
private let configuration: PaymentSheet.Configuration | ||
private let paymentMethods: [STPPaymentMethod] | ||
|
||
private var isEditingPaymentMethods: Bool = false { | ||
didSet { | ||
let additionalButtonTitle = isEditingPaymentMethods ? UIButton.doneButtonTitle : UIButton.editButtonTitle | ||
navigationBar.additionalButton.setTitle(additionalButtonTitle, for: .normal) | ||
headerLabel.text = headerText | ||
|
||
// If we are entering edit mode, put all buttons in an edit state, otherwise put back in their previous state | ||
if isEditingPaymentMethods { | ||
paymentMethodRows.map { $0.button }.forEach { $0.state = .editing } | ||
} else { | ||
paymentMethodRows.map { $0.button }.forEach { $0.state = $0.previousState } | ||
} | ||
// TODO(porter) Handle case where we delete the selected card | ||
} | ||
} | ||
|
||
private var headerText: String { | ||
if isEditingPaymentMethods { | ||
return .Localized.manage_payment_methods | ||
} | ||
|
||
let nonCardPaymentMethods = paymentMethods.filter({ $0.type != .card }) | ||
return nonCardPaymentMethods.isEmpty ? .Localized.select_card : .Localized.select_payment_method | ||
} | ||
|
||
// MARK: Internal properties | ||
weak var delegate: VerticalSavedPaymentMethodsViewControllerDelegate? | ||
|
||
|
@@ -29,15 +54,19 @@ class VerticalSavedPaymentMethodsViewController: UIViewController { | |
lazy var navigationBar: SheetNavigationBar = { | ||
let navBar = SheetNavigationBar(isTestMode: configuration.apiClient.isTestmode, | ||
appearance: configuration.appearance) | ||
navBar.setStyle(.back) | ||
// TODO(porter) Only show edit button if we should | ||
navBar.setStyle(.back(showAdditionalButton: true)) | ||
navBar.delegate = self | ||
navBar.additionalButton.setTitle(UIButton.editButtonTitle, for: .normal) | ||
navBar.additionalButton.accessibilityIdentifier = "edit_saved_button" | ||
navBar.additionalButton.titleLabel?.adjustsFontForContentSizeCategory = true | ||
navBar.additionalButton.addTarget(self, action: #selector(didSelectEditSavedPaymentMethodsButton), for: .touchUpInside) | ||
return navBar | ||
}() | ||
|
||
private lazy var headerLabel: UILabel = { | ||
let label = PaymentSheetUI.makeHeaderLabel(appearance: configuration.appearance) | ||
let nonCardPaymentMethods = paymentMethods.filter({ $0.type != .card }) | ||
label.text = nonCardPaymentMethods.isEmpty ? .Localized.select_card : .Localized.select_payment_method | ||
label.text = headerText | ||
return label | ||
}() | ||
|
||
|
@@ -76,8 +105,12 @@ class VerticalSavedPaymentMethodsViewController: UIViewController { | |
view.backgroundColor = configuration.appearance.colors.background | ||
configuration.style.configure(self) | ||
// TODO(porter) Pipe in selected payment method, default to selecting first for now | ||
paymentMethodRows.first?.button.isSelected = true | ||
view.addAndPinSubviewToSafeArea(stackView, insets: PaymentSheetUI.defaultSheetMargins) | ||
paymentMethodRows.first?.button.state = .selected | ||
view.addAndPinSubview(stackView, insets: PaymentSheetUI.defaultSheetMargins) | ||
} | ||
|
||
@objc func didSelectEditSavedPaymentMethodsButton() { | ||
isEditingPaymentMethods = !isEditingPaymentMethods | ||
} | ||
} | ||
|
||
|
@@ -113,17 +146,23 @@ extension VerticalSavedPaymentMethodsViewController: SheetNavigationBarDelegate | |
|
||
// MARK: - PaymentMethodRowButtonDelegate | ||
extension VerticalSavedPaymentMethodsViewController: PaymentMethodRowButtonDelegate { | ||
|
||
private func paymentMethod(from button: PaymentMethodRowButton) -> STPPaymentMethod? { | ||
return paymentMethodRows.first(where: { $0.button === button })?.paymentMethod | ||
} | ||
|
||
func didSelectButton(_ button: PaymentMethodRowButton) { | ||
guard let paymentMethod = paymentMethodRows.first(where: { $0.button === button })?.paymentMethod else { | ||
guard let paymentMethod = paymentMethod(from: button) else { | ||
// TODO(porter) Handle error - no matching payment method found | ||
return | ||
} | ||
|
||
// Deselect previous button | ||
paymentMethodRows.first { $0.button != button && $0.button.isSelected }?.button.isSelected = false | ||
paymentMethodRows.first { $0.button != button && $0.button.isSelected }?.button.state = .unselected | ||
|
||
// Disable interaction to prevent double selecting since we will be dismissing soon | ||
self.view.isUserInteractionEnabled = false | ||
self.navigationBar.isUserInteractionEnabled = false // Tint buttons in the nav bar to look disabled | ||
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. Not sure I understand this comment, is it supposed to be a TODO? 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. 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. Isn't the main rationale to disable interaction? You wouldn't want to be able to edit while we are in the middle of transitioning back to the main screen. |
||
|
||
// Give time for new selected row to show it has been selected before dismissing | ||
// Makes UX feel a little nicer | ||
|
@@ -132,4 +171,22 @@ extension VerticalSavedPaymentMethodsViewController: PaymentMethodRowButtonDeleg | |
self?.delegate?.didSelectPaymentMethod(paymentMethod) | ||
} | ||
} | ||
|
||
func didSelectRemoveButton(_ button: PaymentMethodRowButton) { | ||
guard let paymentMethod = paymentMethod(from: button) else { | ||
// TODO(porter) Handle error - no matching payment method found | ||
return | ||
} | ||
|
||
print("Remove payment method with id: \(paymentMethod.stripeId)") | ||
} | ||
|
||
func didSelectEditButton(_ button: PaymentMethodRowButton) { | ||
guard let paymentMethod = paymentMethod(from: button) else { | ||
// TODO(porter) Handle error - no matching payment method found | ||
return | ||
} | ||
|
||
print("Edit payment method with id: \(paymentMethod.stripeId)") | ||
} | ||
} |
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.
Picking up this convo #3573 (comment)
I understand your point but IMO it's worth making this payment method a property of
PaymentMethodRowButton
, it lets you get rid of this funny search and removes the possibility that the STPPaymentMethod is nil.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.
👍, refactored out the ViewModel and instead inject a
STPPaymentMethod
.