-
Notifications
You must be signed in to change notification settings - Fork 121
[Mobile Payments] Add badges leading to Set up Tap to Pay #9801
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| import SwiftUI | ||
|
|
||
| extension NSNotification.Name { | ||
| /// Posted whenever the hub menu view did appear. | ||
| /// | ||
| public static let setUpTapToPayViewDidAppear = Foundation.Notification.Name(rawValue: "com.woocommerce.ios.setUpTapToPayViewDidAppear") | ||
| } | ||
|
|
||
| /// This view controller is used when no reader is connected. It assists | ||
| /// the merchant in connecting to a reader. | ||
| /// | ||
|
|
@@ -129,6 +135,7 @@ struct SetUpTapToPayInformationView: View { | |
| .scrollVerticallyIfNeeded() | ||
| } | ||
| .padding() | ||
| .onAppear(perform: viewModel.viewDidAppear) | ||
|
Contributor
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 to clarify if I understood correctly why this method call: This is basically a configuration method, right? We're not adding it to the viewmodel's init because we don't want to rely on the viewModel's lifecycle, and wait for this to be initialized to trigger this configuration. And at the same time, we want the config logic to be testable, so we extract it from the view to the model.
Contributor
Author
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. IIRC, we're not using the init because the view model is created optimistically, the user may not actually open the screen. We use this |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,7 +22,7 @@ final class InPersonPaymentsMenuViewController: UIViewController { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private lazy var inPersonPaymentsLearnMoreViewModel = LearnMoreViewModel.inPersonPayments(source: .paymentsMenu) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private let viewModel: InPersonPaymentsMenuViewModel = InPersonPaymentsMenuViewModel() | ||||||||||||||||||||||||||||||||
| private let viewModel: InPersonPaymentsMenuViewModel | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private let cashOnDeliveryToggleRowViewModel: InPersonPaymentsCashOnDeliveryToggleRowViewModel | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -68,9 +68,11 @@ final class InPersonPaymentsMenuViewController: UIViewController { | |||||||||||||||||||||||||||||||
| /// - viewDidLoadAction: Provided as a one-time callback on viewDidLoad, originally to handle universal link navigation correctly. | ||||||||||||||||||||||||||||||||
| init(stores: StoresManager = ServiceLocator.stores, | ||||||||||||||||||||||||||||||||
| featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, | ||||||||||||||||||||||||||||||||
| shouldShowBadgeOnSetUpTapToPay: Bool, | ||||||||||||||||||||||||||||||||
| viewDidLoadAction: ((InPersonPaymentsMenuViewController) -> Void)? = nil) { | ||||||||||||||||||||||||||||||||
| self.stores = stores | ||||||||||||||||||||||||||||||||
| self.featureFlagService = featureFlagService | ||||||||||||||||||||||||||||||||
| self.viewModel = InPersonPaymentsMenuViewModel(shouldShowBadgeOnSetUpTapToPay: shouldShowBadgeOnSetUpTapToPay) | ||||||||||||||||||||||||||||||||
| self.cardPresentPaymentsOnboardingUseCase = CardPresentPaymentsOnboardingUseCase() | ||||||||||||||||||||||||||||||||
| self.cashOnDeliveryToggleRowViewModel = InPersonPaymentsCashOnDeliveryToggleRowViewModel() | ||||||||||||||||||||||||||||||||
| self.setUpFlowOnlyEnabledAfterOnboardingComplete = !featureFlagService.isFeatureFlagEnabled(.tapToPayOnIPhoneMilestone2) | ||||||||||||||||||||||||||||||||
|
|
@@ -304,7 +306,7 @@ private extension InPersonPaymentsMenuViewController { | |||||||||||||||||||||||||||||||
| configureCollectPayment(cell: cell) | ||||||||||||||||||||||||||||||||
| case let cell as LeftImageTitleSubtitleToggleTableViewCell where row == .toggleEnableCashOnDelivery: | ||||||||||||||||||||||||||||||||
| configureToggleEnableCashOnDelivery(cell: cell) | ||||||||||||||||||||||||||||||||
| case let cell as LeftImageTableViewCell where row == .setUpTapToPayOnIPhone: | ||||||||||||||||||||||||||||||||
| case let cell as BadgedLeftImageTableViewCell where row == .setUpTapToPayOnIPhone: | ||||||||||||||||||||||||||||||||
| configureSetUpTapToPayOnIPhone(cell: cell) | ||||||||||||||||||||||||||||||||
| case let cell as LeftImageTableViewCell where row == .tapToPayOnIPhoneFeedback: | ||||||||||||||||||||||||||||||||
| configureTapToPayOnIPhoneFeedback(cell: cell) | ||||||||||||||||||||||||||||||||
|
|
@@ -366,11 +368,12 @@ private extension InPersonPaymentsMenuViewController { | |||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func configureSetUpTapToPayOnIPhone(cell: LeftImageTableViewCell) { | ||||||||||||||||||||||||||||||||
| func configureSetUpTapToPayOnIPhone(cell: BadgedLeftImageTableViewCell) { | ||||||||||||||||||||||||||||||||
| prepareForReuse(cell) | ||||||||||||||||||||||||||||||||
| cell.accessibilityIdentifier = "set-up-tap-to-pay" | ||||||||||||||||||||||||||||||||
| cell.configure(image: .tapToPayOnIPhoneIcon, | ||||||||||||||||||||||||||||||||
| text: Localization.tapToPayOnIPhone) | ||||||||||||||||||||||||||||||||
| text: Localization.tapToPayOnIPhone, | ||||||||||||||||||||||||||||||||
| showBadge: viewModel.shouldBadgeTapToPayOnIPhone) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if setUpFlowOnlyEnabledAfterOnboardingComplete { | ||||||||||||||||||||||||||||||||
| cell.accessoryType = enableSetUpTapToPayOnIPhoneCell ? .disclosureIndicator : .none | ||||||||||||||||||||||||||||||||
|
|
@@ -418,6 +421,14 @@ private extension InPersonPaymentsMenuViewController { | |||||||||||||||||||||||||||||||
| self?.configureSections(shouldShowTapToPayOnIPhoneFeedback: shouldShowFeedbackRow) | ||||||||||||||||||||||||||||||||
| self?.tableView.reloadData() | ||||||||||||||||||||||||||||||||
| }.store(in: &cancellables) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| viewModel.$shouldBadgeTapToPayOnIPhone.sink { [weak self] _ in | ||||||||||||||||||||||||||||||||
| self?.configureSections() | ||||||||||||||||||||||||||||||||
| // ensures that the cell will be configured with the correct value for the badge | ||||||||||||||||||||||||||||||||
| DispatchQueue.main.async { | ||||||||||||||||||||||||||||||||
| self?.tableView.reloadData() | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }.store(in: &cancellables) | ||||||||||||||||||||||||||||||||
|
Comment on lines
+425
to
+431
Contributor
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. I don't think is necessary as we're already weakifying self, but should we add a guard just to remove optionality?
Suggested change
Contributor
Author
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. I don't think there's a benefit to removing the optionality here, but thanks for the suggestion 😊 |
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private func configureWebViewPresentation() { | ||||||||||||||||||||||||||||||||
|
|
@@ -732,6 +743,8 @@ private enum Row: CaseIterable { | |||||||||||||||||||||||||||||||
| return LeftImageTitleSubtitleTableViewCell.self | ||||||||||||||||||||||||||||||||
| case .toggleEnableCashOnDelivery: | ||||||||||||||||||||||||||||||||
| return LeftImageTitleSubtitleToggleTableViewCell.self | ||||||||||||||||||||||||||||||||
| case .setUpTapToPayOnIPhone: | ||||||||||||||||||||||||||||||||
| return BadgedLeftImageTableViewCell.self | ||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||
| return LeftImageTableViewCell.self | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
@@ -754,8 +767,10 @@ private extension InPersonPaymentsMenuViewController { | |||||||||||||||||||||||||||||||
| /// SwiftUI wrapper for CardReaderSettingsPresentingViewController | ||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||
| struct InPersonPaymentsMenu: UIViewControllerRepresentable { | ||||||||||||||||||||||||||||||||
| let shouldShowBadgeOnSetUpTapToPay: Bool | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func makeUIViewController(context: Context) -> some UIViewController { | ||||||||||||||||||||||||||||||||
| InPersonPaymentsMenuViewController() | ||||||||||||||||||||||||||||||||
| InPersonPaymentsMenuViewController(shouldShowBadgeOnSetUpTapToPay: shouldShowBadgeOnSetUpTapToPay) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func updateUIViewController(_ uiViewController: UIViewControllerType, context: Context) { | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
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.
That's convenient! 💯
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.
Yep. Notifications need to be used with care, but I think it's appropriate here.