Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Experiments/Experiments/DefaultFeatureFlagService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public struct DefaultFeatureFlagService: FeatureFlagService {
return buildConfig == .localDeveloper || buildConfig == .appStore
case .tapToPayOnIPhoneMilestone2:
return true
case .tapToPayBadge:
return buildConfig == .localDeveloper || buildConfig == .alpha
case .domainSettings:
return true
case .jetpackSetupWithApplicationPassword:
Expand Down
4 changes: 4 additions & 0 deletions Experiments/Experiments/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ public enum FeatureFlag: Int {
///
case tapToPayOnIPhoneMilestone2

/// Enables badging the route to Set up Tap to Pay on iPhone on eligible devices
///
case tapToPayBadge

/// Store creation MVP.
///
case storeCreationMVP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ public enum FeatureAnnouncementCampaign: String, Codable, Equatable {
case inPersonPaymentsCashOnDelivery = "ipp_not_user"
case inPersonPaymentsFirstTransaction = "ipp_new_user"
case inPersonPaymentsPowerUsers = "ipp_power_user"
case tapToPayHubMenuBadge = "tap_to_pay_hub_menu_badge"

/// Added for use in `test_setFeatureAnnouncementDismissed_with_another_campaign_previously_dismissed_keeps_values_for_both`
/// This can be removed when we have a second campaign, which can be used in the above test instead.
Expand Down
34 changes: 34 additions & 0 deletions WooCommerce/Classes/ViewModels/MainTabViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ final class MainTabViewModel {

listenToReviewsBadgeReloadRequired()
retrieveShouldShowReviewsBadgeOnHubMenuTabValue()

listenToNewFeatureBadgeReloadRequired()
retrieveShouldShowNewFeatureBadgeOnHubMenuTabValue()
}
}

Expand Down Expand Up @@ -197,6 +200,21 @@ private extension MainTabViewModel {
object: nil)
}


func listenToNewFeatureBadgeReloadRequired() {
NotificationCenter.default.addObserver(self,
selector: #selector(setUpTapToPayViewDidAppear),
name: .setUpTapToPayViewDidAppear,
object: nil)

}

/// Updates the badge after the Set up Tap to Pay flow did appear
///
@objc func setUpTapToPayViewDidAppear() {
shouldShowNewFeatureBadgeOnHubMenuTab = false
}

/// Retrieves whether we should show the reviews on the Menu button and updates `shouldShowReviewsBadge`
///
@objc func retrieveShouldShowReviewsBadgeOnHubMenuTabValue() {
Expand All @@ -211,6 +229,22 @@ private extension MainTabViewModel {
storesManager.dispatch(notificationCountAction)
}

/// Retrieves whether we should show the new feature badge on the Menu button
///
func retrieveShouldShowNewFeatureBadgeOnHubMenuTabValue() {
let action = AppSettingsAction.getFeatureAnnouncementVisibility(campaign: .tapToPayHubMenuBadge) { [weak self] result in
guard let self = self else { return }
switch result {
case .success(let visible):
self.shouldShowNewFeatureBadgeOnHubMenuTab = visible && self.featureFlagService.isFeatureFlagEnabled(.tapToPayBadge)
case .failure:
self.shouldShowNewFeatureBadgeOnHubMenuTab = false
}
}

storesManager.dispatch(action)
}

/// Listens for changes on the menu badge display logic and updates it depending on them
///
func synchronizeShouldShowBadgeOnHubMenuTabLogic() {
Expand Down
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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's convenient! 💯

Copy link
Contributor Author

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.

}

/// This view controller is used when no reader is connected. It assists
/// the merchant in connecting to a reader.
///
Expand Down Expand Up @@ -129,6 +135,7 @@ struct SetUpTapToPayInformationView: View {
.scrollVerticallyIfNeeded()
}
.padding()
.onAppear(perform: viewModel.viewDidAppear)
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 viewModel.viewDidAppear to trigger the notification, which we respond to in other places by removing all the badges... so we only want to do that when it actually appears.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ final class SetUpTapToPayInformationViewModel: PaymentSettingsFlowPresentedViewM
dismiss?()
}

func viewDidAppear() {
let action = AppSettingsAction.setFeatureAnnouncementDismissed(campaign: .tapToPayHubMenuBadge, remindAfterDays: nil, onCompletion: nil)
stores.dispatch(action)
NotificationCenter.default.post(name: .setUpTapToPayViewDidAppear, object: nil)
}

/// Updates whether the view this viewModel is associated with should be shown or not
/// Notifies the viewModel owner if a change occurs via didChangeShouldShow
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
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)
viewModel.$shouldBadgeTapToPayOnIPhone.sink { [weak self] _ in
guard let self = self else { return }
self.configureSections()
DispatchQueue.main.async {
self.tableView.reloadData()
}
}.store(in: &cancellables)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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() {
Expand Down Expand Up @@ -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
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,16 @@ final class InPersonPaymentsMenuViewModel {

@Published private(set) var isEligibleForTapToPayOnIPhone: Bool = false
@Published private(set) var shouldShowTapToPayOnIPhoneFeedbackRow: Bool = false
@Published private(set) var shouldBadgeTapToPayOnIPhone: Bool = false

let cardPresentPaymentsConfiguration: CardPresentPaymentsConfiguration

init(dependencies: Dependencies = Dependencies(),
cardPresentPaymentsConfiguration: CardPresentPaymentsConfiguration = CardPresentConfigurationLoader().configuration) {
cardPresentPaymentsConfiguration: CardPresentPaymentsConfiguration = CardPresentConfigurationLoader().configuration,
shouldShowBadgeOnSetUpTapToPay: Bool) {
self.dependencies = dependencies
self.cardPresentPaymentsConfiguration = cardPresentPaymentsConfiguration
self.shouldBadgeTapToPayOnIPhone = shouldShowBadgeOnSetUpTapToPay
}

func viewDidLoad() {
Expand Down Expand Up @@ -97,6 +100,10 @@ final class InPersonPaymentsMenuViewModel {
selector: #selector(refreshTapToPayFeedbackVisibility),
name: .firstInPersonPaymentsTransactionsWereUpdated,
object: nil)
NotificationCenter.default.addObserver(self,
selector: #selector(setUpTapToPayViewDidAppear),
name: .setUpTapToPayViewDidAppear,
object: nil)
}

@objc func refreshTapToPayFeedbackVisibility() {
Expand All @@ -106,6 +113,10 @@ final class InPersonPaymentsMenuViewModel {
checkShouldShowTapToPayFeedbackRow(siteID: siteID)
}

@objc private func setUpTapToPayViewDidAppear() {
self.shouldBadgeTapToPayOnIPhone = false
}

func orderCardReaderPressed() {
analytics.track(.paymentsMenuOrderCardReaderTapped)
showWebView = PurchaseCardReaderWebViewViewModel(configuration: cardPresentPaymentsConfiguration,
Expand All @@ -123,6 +134,9 @@ final class InPersonPaymentsMenuViewModel {
NotificationCenter.default.removeObserver(self,
name: .firstInPersonPaymentsTransactionsWereUpdated,
object: nil)
NotificationCenter.default.removeObserver(self,
name: .setUpTapToPayViewDidAppear,
object: nil)
}
}

Expand Down
76 changes: 48 additions & 28 deletions WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ struct HubMenu: View {
viewModel.presentSwitchStore()
} label: {
Row(title: viewModel.storeTitle,
badge: viewModel.planName,
titleBadge: viewModel.planName,
iconBadge: nil,
description: viewModel.storeURL.host ?? viewModel.storeURL.absoluteString,
icon: .remote(viewModel.avatarURL),
chevron: viewModel.switchStoreEnabled ? .down : .none,
Expand All @@ -51,7 +52,8 @@ struct HubMenu: View {
handleTap(menu: menu)
} label: {
Row(title: menu.title,
badge: nil,
titleBadge: nil,
iconBadge: menu.iconBadge,
description: menu.description,
icon: .local(menu.icon),
chevron: .leading)
Expand All @@ -68,7 +70,8 @@ struct HubMenu: View {
handleTap(menu: menu)
} label: {
Row(title: menu.title,
badge: nil,
titleBadge: nil,
iconBadge: menu.iconBadge,
description: menu.description,
icon: .local(menu.icon),
chevron: .leading)
Expand Down Expand Up @@ -102,7 +105,7 @@ struct HubMenu: View {
EmptyView()
}.hidden()
NavigationLink(destination:
InPersonPaymentsMenu()
InPersonPaymentsMenu(shouldShowBadgeOnSetUpTapToPay: viewModel.shouldShowNewFeatureBadgeOnPayments)
.navigationTitle(InPersonPaymentsView.Localization.title),
isActive: $showingPayments) {
EmptyView()
Expand Down Expand Up @@ -198,9 +201,13 @@ private extension HubMenu {
///
let title: String

/// Optional badge text. Render next to `title`
/// Text badge displayed adjacent to the title
///
let badge: String?
let titleBadge: String?

/// Badge displayed on the icon.
///
let iconBadge: HubMenuBadgeType?

/// Row Description
///
Expand All @@ -225,31 +232,42 @@ private extension HubMenu {

HStack(spacing: .zero) {
/// iOS 16, aligns the list dividers to the first text position.
/// This tricks the system by rendering an empty text and forcing the list lo align the divider to it.
/// This tricks the system by rendering an empty text and forcing the list to align the divider to it.
/// Without this, the divider will be rendered from the title and will not cover the icon.
/// Ideally we would want to use the `alignmentGuide` modifier but that is only available on iOS 16.
///
Text("")

// Icon
Group {
switch icon {
case .local(let asset):
Circle()
.fill(Color(.init(light: .listBackground, dark: .secondaryButtonBackground)))
.frame(width: HubMenu.Constants.avatarSize, height: HubMenu.Constants.avatarSize)
.overlay {
Image(uiImage: asset)
.resizable()
.frame(width: HubMenu.Constants.iconSize, height: HubMenu.Constants.iconSize)
}

case .remote(let url):
KFImage(url)
.placeholder { Image(uiImage: .gravatarPlaceholderImage).resizable() }
.resizable()
.frame(width: HubMenu.Constants.avatarSize, height: HubMenu.Constants.avatarSize)
.clipShape(Circle())
ZStack {
// Icon
Group {
switch icon {
case .local(let asset):
Circle()
.fill(Color(.init(light: .listBackground, dark: .secondaryButtonBackground)))
.frame(width: HubMenu.Constants.avatarSize, height: HubMenu.Constants.avatarSize)
.overlay {
Image(uiImage: asset)
.resizable()
.frame(width: HubMenu.Constants.iconSize, height: HubMenu.Constants.iconSize)
}

case .remote(let url):
KFImage(url)
.placeholder { Image(uiImage: .gravatarPlaceholderImage).resizable() }
.resizable()
.frame(width: HubMenu.Constants.avatarSize, height: HubMenu.Constants.avatarSize)
.clipShape(Circle())
}
}
.overlay(alignment: .topTrailing) {
// Badge
if case .dot = iconBadge {
Circle()
.fill(Color(.accent))
.frame(width: HubMenu.Constants.dotBadgeSize)
.padding(HubMenu.Constants.dotBadgePadding)
}
}
}
}
Expand All @@ -263,8 +281,8 @@ private extension HubMenu {
.headlineStyle()
.accessibilityIdentifier(titleAccessibilityID ?? "")

if let badge, badge.isNotEmpty {
BadgeView(text: badge)
if let titleBadge, titleBadge.isNotEmpty {
BadgeView(text: titleBadge)
}
}

Expand Down Expand Up @@ -298,6 +316,8 @@ private extension HubMenu {
static let chevronSize: CGFloat = 20
static let iconSize: CGFloat = 20
static let trackingOptionKey = "option"
static let dotBadgePadding = EdgeInsets(top: 6, leading: 0, bottom: 0, trailing: 2)
static let dotBadgeSize: CGFloat = 6

/// Spacing for the badge view in the avatar row.
///
Expand Down
Loading