[TTPReq] Integrate Education steps after Terms of Service are accepted #14580
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| /// A type that represents the possible states of the built-in (Tap to Pay) card reader onboarding flow. | ||
| public enum BuiltInCardReaderOnboardingState { | ||
| /// User has accepted the terms of service | ||
| case didAcceptTermsOfService | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,3 +67,24 @@ struct TapToPayEducationView: View { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Hosting Controller | ||
|
|
||
| final class TapToPayEducationViewViewHostingController: UIHostingController<TapToPayEducationView> { | ||
staskus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| init(onDismiss: @escaping () -> Void) { | ||
| let viewModel = TapToPayEducationViewModel(flow: .onboarding, onDismiss: onDismiss) | ||
| super.init(rootView: TapToPayEducationView(viewModel: viewModel)) | ||
|
|
||
| viewModel.onDismiss = { [weak self] in | ||
| guard let self else { return } | ||
|
|
||
| self.dismiss(animated: true) { | ||
staskus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| onDismiss() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @MainActor @preconcurrency required dynamic init?(coder aDecoder: NSCoder) { | ||
|
There was a problem hiding this comment. Ooh, what's a There was a problem hiding this comment. Xcode 16 generated this one for me 😀 |
||
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ final class TapToPayEducationViewModel: ObservableObject { | |
| let cardPresentPaymentsOnboardingUseCase: CardPresentPaymentsOnboardingUseCaseProtocol | ||
| let siteID: Int64 | ||
|
|
||
| var onDismiss: () -> Void | ||
|
There was a problem hiding this comment.
|
||
|
|
||
| init(flow: Flow = .onboarding, | ||
| steps: [TapToPayEducationStepViewModel]? = nil, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,4 +45,8 @@ final class SilenceablePassthroughCardPresentPaymentAlertsPresenter<AlertPresent | |
| alertsPresenter = nil | ||
| alertSubscription?.cancel() | ||
| } | ||
|
|
||
| func presentMerchantEducation(completion: @escaping () -> Void) { | ||
| completion() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ protocol CardPresentPaymentAlertsPresenting<AlertDetails> { | |
| associatedtype AlertDetails | ||
| func present(viewModel: AlertDetails) | ||
| func presentWCSettingsWebView(adminURL: URL, completion: @escaping () -> Void) | ||
| func presentMerchantEducation(completion: @escaping () -> Void) | ||
|
There was a problem hiding this comment. ❓ Just feeling a bit uncertain about this, as this is just for TTP and I'm not sure if it requires a separate function in fear of this protocol growing very long in the future with new views like this 🤔 @joshheald WDYT about having a separate function here (maybe renaming to There was a problem hiding this comment. Yes, I was going back and forth on how to better approach this. Integrating within We could also create a separate presenter whose sole responsibility would be to present TTP education on top of the existing context. (Example) There was a problem hiding this comment.
I was thinking of refactoring the UIKit version
Nice, I like this approach - it doesn't add complexity to the alerts presenter and the merchant education UI is not an "alert" anyway. When the time comes to support TTP in POS, we can update There was a problem hiding this comment. I will start manual testing after this thread concludes. There was a problem hiding this comment. Yes, I think the second approach of having a separate presenter for education is reasonable. Non-TTP-related controllers don't have to have any knowledge of it. I will include it in the main PR. There was a problem hiding this comment. I can see why you went back and forth here!
In some ways it does seem a little strange not to include alerts which (currently) only show during a built in reader connection in It would be perfectly valid to show education outside a connection flow as well, and if we did that, it would seem strange if we needed a The new design is more flexible, because Unfortunately under the existing UIKit design, it's not so clean, as we have the same type of concrete view model class produced for every event, and it does need to be used to present a view. So unless we refactor the existing code to use the new approach, we can't reasonably include the education step in this way. It would probably be ideal to do the refactor. But, it's also not factored in to the project so probably not a reasonable choice here. Since we're not doing it now, let's avoid making it harder to do it in the future. I think that having a separate presenter meets that need. We should try to keep it simple though! |
||
| func foundSeveralReaders(readerIDs: [String], | ||
| connect: @escaping (String) -> Void, | ||
| cancelSearch: @escaping () -> Void) | ||
|
|
@@ -143,4 +144,14 @@ final class CardPresentPaymentAlertsPresenter: CardPresentPaymentAlertsPresentin | |
| dismissCommonAndPresent(animated: true) | ||
| dismissSeveralFoundAndPresent(animated: true) | ||
| } | ||
|
|
||
| func presentMerchantEducation(completion: @escaping () -> Void) { | ||
| let viewController = TapToPayEducationViewViewHostingController(onDismiss: completion) | ||
|
|
||
| if let modalController { | ||
| modalController.present(viewController, animated: true) | ||
| } else { | ||
| rootViewController?.present(viewController, animated: true) | ||
| } | ||
staskus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,10 @@ final class MockCardPresentPaymentAlertsPresenter: CardPresentPaymentAlertsPrese | |
| func dismiss() { | ||
| // no-op | ||
| } | ||
|
|
||
| func presentMerchantEducation(completion: @escaping () -> Void) { | ||
| completion() | ||
| } | ||
| } | ||
|
|
||
| enum MockCardPresentPaymentAlertsPresenterMode { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,9 @@ public enum CardPresentPaymentAction: Action { | |
| /// Check the state of available software updates. | ||
| case observeCardReaderUpdateState(onCompletion: (AnyPublisher<CardReaderSoftwareUpdateState, Never>) -> Void) | ||
|
|
||
| /// Check the state of the built-in card reader onboarding | ||
| case observeBuiltInCardReaderOnboardingState(onCompletion: (AnyPublisher<BuiltInCardReaderOnboardingState, Never>) -> Void) | ||
|
|
||
| /// Update card reader firmware. | ||
| case startCardReaderUpdate | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,6 +190,7 @@ public typealias StoredProductSettings = Networking.StoredProductSettings | |
| public typealias CardReader = Hardware.CardReader | ||
| public typealias CardReaderConnectionOptions = Hardware.CardReaderConnectionOptions | ||
| public typealias BuiltInCardReaderConnectionOptions = Hardware.BuiltInCardReaderConnectionOptions | ||
| public typealias BuiltInCardReaderOnboardingState = Hardware.BuiltInCardReaderOnboardingState | ||
| public typealias CardReaderDiscoveryMethod = Hardware.CardReaderDiscoveryMethod | ||
| public typealias CardReaderEvent = Hardware.CardReaderEvent | ||
| public typealias CardReaderInput = Hardware.CardReaderInput | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,6 +129,8 @@ public final class CardPresentPaymentStore: Store { | |
| cancelRefund(onCompletion: completion) | ||
| case .observeCardReaderUpdateState(onCompletion: let completion): | ||
| observeCardReaderUpdateState(onCompletion: completion) | ||
| case .observeBuiltInCardReaderOnboardingState(let completion): | ||
| observeBuiltInCardReaderOnboardingState(onCompletion: completion) | ||
| case .startCardReaderUpdate: | ||
| startCardReaderUpdate() | ||
| case .reset: | ||
|
|
@@ -400,6 +402,10 @@ private extension CardPresentPaymentStore { | |
| onCompletion(cardReaderService.softwareUpdateEvents) | ||
| } | ||
|
|
||
| func observeBuiltInCardReaderOnboardingState(onCompletion: @escaping (AnyPublisher<BuiltInCardReaderOnboardingState, Never>) -> Void) { | ||
| onCompletion(cardReaderService.builtInCardReaderOnboardingEvents) | ||
| } | ||
|
|
||
| func startCardReaderUpdate() { | ||
| cardReaderService.installUpdate() | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,10 @@ final class MockCardReaderService: CardReaderService { | |
| CurrentValueSubject<CardReaderSoftwareUpdateState, Never>(.none).eraseToAnyPublisher() | ||
| } | ||
|
|
||
| var builtInCardReaderOnboardingEvents: AnyPublisher<Hardware.BuiltInCardReaderOnboardingState, Never> { | ||
| PassthroughSubject<BuiltInCardReaderOnboardingState, Never>().eraseToAnyPublisher() | ||
| } | ||
|
|
||
| /// Boolean flag Indicates that clients have called the start method | ||
| var didHitStart = false | ||
|
|
||
|
|
||
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.
non-blocking question: do we plan to add more cases to the onboarding state? If not, I wonder if emitting
Voidfrom the publisher (likevar builtInCardReaderAcceptToSEvents: AnyPublisher<Void, Never> { get }) on ToS acceptance event is sufficient.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.
I don't plan to add any more cases. I created it following the logic of other similar states. However,
builtInCardReaderAcceptToSEventsis a good idea and would avoid creating this additionalenum.