[Woo POS][Modularization] Batch 1: Make Woo app dependencies reusable for PointOfSale module #15968

staskus · 2025-07-31T15:29:12Z

More context: pdfdoF-7It-p2
Last PRs: #15955, #15956
WOOMOB-935

Description

I identified a list of remaining dependencies that POS code depends on the Woo app. I started tracking them more tidily within the Linear task WOOMOB-935. Feel free to pick any of those dependencies to make them reusable for POS module.

I drew an arbitrary line to make a pull request before proceeding to further refactoring.

For each of the dependencies, I need to make a decision:

Moved to a shared module (WooFoundation, Yosemite...)
Injected to the POS through the adaptor or some other way
Duplicated in the POS target
... or just move away outside the POS folder since they are not really used

For now, I decided not to create another module (e.g WooFoundationUI). However, that would be an easy change in the future if we decide to split WooFoundation in more parts. We have many options for the future, including centralizing payments logic in some Payments module and then reusing it between IPP and POS. For now, this is outside the scope.

Moved dependencies

Moved to WooFoundation Module

Shared utilities and UI components used across multiple modules

Core Utilities:

VersionHelpers → Move to WooFoundation
- Decision: Shared version comparison utilities used across 13+ files

String & Collection Extensions:

String+Helpers → Move to WooFoundation
- Decision: Core string utilities (pluralize, isNotEmpty) used throughout app
Array+Helpers → Move to WooFoundation
- Decision: Collection convenience extensions (isNotEmpty, popFirst, etc.) widely used

SwiftUI View Modifiers:

View+Conditionals → Move to WooFoundation
- Decision: SwiftUI conditional view modifiers (renderedIf, if) used across modules
View+Measurements → Move to WooFoundation
- Decision: SwiftUI measurement helpers (measureHeight, measureWidth) used in multiple modules
View+RoundedBorder → Move to WooFoundation
- Decision: Border styling modifiers with dashed support used across UI
View+ScrollModifiers → Move to WooFoundation
- Decision: Scroll utilities including scrollVerticallyIfNeeded accessibility helper
View+SizeTracker → Move to WooFoundation
- Decision: View size tracking utility used in multiple components
View+AutofocusTextModifier → Move to WooFoundation
- Decision: Text field focus management used across forms

UI Components:

ScrollableVStack → Move to WooFoundation
- Decision: Reusable scrollable container component used across modules
WooRoundedBorderTextFieldStyle → Move to WooFoundation
- Decision: Shared text field styling used in authentication and forms
IndefiniteCircularProgressViewStyle → Move to WooFoundation
- Decision: Animated progress indicator style used across loading states
KeyboardObserver → Move to WooFoundation
- Decision: iOS 17+ keyboard state management with environment support

Moved to Yosemite Module

Business logic components that belong in data layer

OrderTotalsCalculator → Move to Yosemite
- Decision: Order calculation logic shared between main app and POS

Created POS-Specific Abstractions

Main app services requiring dependency injection

Navigation Services:

DeepLinkNavigator & AppDelegate → Create POSNavigationAdaptor

External View Services:

SupportFormViewModel → Create POSExternalViewProvider
- Decision: Inject support form functionality to remove main app UI dependency. I will use this approach for other Woo app views that have many dependencies as well.

Moved to POS Adaptors away from POS module

Components that bridge main app to POS module

CardPresentPaymentsTransactionAlertsProvider → Move to POS Adaptors
CardPresentPaymentInvalidatablePaymentOrchestrator → Move to POS Adaptors
CardPresentPaymentsConnectionControllerManager → Move to POS Adaptors
POSCollectOrderPaymentAnalyticsAdaptor → Move to POS Adaptors

Decision: These components need access to both POS module and main app services

POS-Specific Assets

POS-specific image assets

UIImage+Woo extensions → Create UIImage+POS
- Decision: Move only POS-used UIImage extensions to POS module

Removed Unused Dependencies

POSSessionManagerProviding → Remove
- Decision: Replaced by externalViewsProvider, no longer needed
CardPresentPaymentError → Delete
- Decision: Unused error type, clean up

Card Present Payment Dependencies

CardPresentPaymentsOnboardingViewModel IPP dependency → Remove and replace with factory that could be accessed by POS
- Decision: Abstract IPP types to remove coupling

Remaining dependencies to address

WooConstants - Shared constants and URLs (external dependencies)
FormattableAmountTextField - Used across order/payment flows
ProductImageThumbnail - Used throughout product displays
AddEditCoupon view - Used in POS and main coupon management
POSCouponCreationSheetModifier - Reusable modal pattern
PaymentCaptureCelebrationProtocol - Payment success celebration behavior
POSEntryPointEligibilityCheckerProtocol - POS access eligibility checking
SelectedSiteSettingsProtocol - Site configuration access
Parts of WooAnalytics+WooApp - Analytics tracking abstraction
WCSettingsWebView - Web view for settings pages
ReceiptEligibilityUseCase - Receipt generation eligibility logic
CollectOrderPaymentUseCaseError - Payment error handling
FormattableAmountTextFieldViewModel - Amount input business logic
PriceFieldFormatter - Price display formatting logic
WaitingTimeTracker - Time tracking for operations
readableCountry extension - CountryCode formatting from SiteAddress
CurrencySettings + Sanitized - Currency formatting extensions
SwiftUI Shimmer - Loading state animation effect framework
POSCollectOrderPaymentPreviewAnalytics - Preview-specific analytics
POSPreviewEntryPointEligibilityChecker - Preview eligibility checker
POSProductFactory - Delete

Steps to reproduce

CI should succeed
POS should launch
Support Form should work
Payments should succed

Testing information

Tested on iPad 18.5 simulator, iPad Air M2 26 device

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

…te dependency

…oardView

CardPresentPaymentsOnboardingViewModel is a strong IPP dependency on POS which drags a lot of dependencies. Solution is to pass CardPresentPaymentOnboardingViewFactory instead of CardPresentPaymentsOnboardingViewModel that hides away ViewModel implementation. ViewModel is injected at an adaptor level - CardPresentPaymentOnboardingAdaptor.

…ntPaymentsConnectionControllerManager to POS Adaptors

POSCollectOrderPaymentAnalytics depends on concrete IPP implementation that is not mean to be moved to POS. - Rename POSCollectOrderPaymentAnalytics to POSCollectOrderPaymentAnalyticsAdaptor - Split POSCollectOrderPaymentAnalyticsTracking POS protocol from IPP CollectOrderPaymentAnalyticsTracking protocol - Only connect POSCollectOrderPaymentAnalyticsTracking and CollectOrderPaymentAnalyticsTracking at the implementation point

…n POS module

…ewsProvider

dangermattic · 2025-07-31T15:29:56Z

	2 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-07-31T15:38:25Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`pr15968-41e86f1`
	Version	`22.9`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`41e86f1`
	Installation URL	5epuqfo6a3838

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

jaclync

Thanks for the updates! Noticed an issue when testing onboarding for a store with incomplete onboarding state where support and learn more CTAs aren't tappable:

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-08-01.at.12.15.22.mp4

This doesn't happen in trunk. I'll also take a look later.

jaclync · 2025-08-01T03:24:53Z

Modules/Sources/PointOfSale/Analytics/POSAnalyticsEvent.swift

+        static func orderCreationFailed(
+            usesGiftCard: Bool,
+            errorContext: String,
+            errorDescription: String,
+            errorType: OrderCreationErrorType? = nil
+        ) -> WooAnalyticsEvent {
+            var properties: [String: WooAnalyticsEventPropertyType] = [
+                "use_gift_card": usesGiftCard,
+                "error_context": errorContext,
+                "error_description": errorDescription
+            ]
+
+            if let errorType {
+                properties["error_type"] = errorType.rawValue
+            }
+
+            return WooAnalyticsEvent(statName: .orderCreationFailed, properties: properties)
+        }


nit: this seems to be tracked in both POS and IPP, WDYT having this extension in WooFoundation for example for use? this way, any modifications to the tracking apply to both use cases.

I don't know if we should strive to reuse 100%. Since it's the only event so far that had the same logic, I felt it was fine to keep it separately next to other POS events. In fact, I think we track OrderCreationErrorType only on POS for now.

jaclync · 2025-08-01T03:30:05Z

Modules/Sources/WooFoundation/Utilities/IndefiniteCircularProgressViewStyle.swift

-        static let lineWidth: CGFloat = 10.0
-        static let backgroundOpacity: CGFloat = 0.2
+public extension IndefiniteCircularProgressViewStyle {
+    public enum Constants {


nit: getting 'public' modifier is redundant for enum declared in a public extension warning

Suggested change

public enum Constants {

enum Constants {

jaclync · 2025-08-01T03:30:21Z

Modules/Sources/WooFoundation/Utilities/IndefiniteCircularProgressViewStyle.swift

    }

-    enum Localization {
+    public enum Localization {


nit: does this string need to be public?

jaclync · 2025-08-01T03:31:48Z

Modules/Sources/WooFoundation/Utilities/VersionHelpers.swift

 /// Helpers for working with versions (e.g. comparing two version strings)
 ///
-final class VersionHelpers {
+public final class VersionHelpers {


nice move, I've been hoping to use this in Yosemite while it was in the app layer.

jaclync · 2025-08-01T03:48:00Z

Modules/Sources/PointOfSale/DependencyProviders/POSDependencyProviding.swift

+
+/// External view service abstraction
+public protocol POSExternalViewProviding {
+    func createSupportFormView(isPresented: Binding<Bool>) -> AnyView


nit: a bit scary seeing AnyView usage due to the rumored performance impact, any chance this could return a view builder to avoid AnyView?

jaclync · 2025-08-01T03:50:30Z

Modules/Sources/PointOfSale/Extensions/UIImage+POS.swift

+    /// App icon (iPhone size) - used in receipt eligibility banner
+    ///
+    static var appIconDefault: UIImage {
+        return UIImage(named: "AppIcon60x60")!


is the image catalog shared automatically across modules? these images aren't used in POS yet.

So far as I have tested, it works. Both on the device and the simulator, although I wouldn't have expected that. I specified main .bundle now. We can tell once we move the POS code to the module if everything works all right.

jaclync · 2025-08-01T03:55:56Z

WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift

-        SafariView(url: WooConstants.URLs.pointOfSaleDocumentation.asURL())
+        DocumentationView()


just wondering, what are the reasons for this change that require a new DocumentationView from UIKit?

I thought it's easier to copy rather than to move it into the common place. But probably wiser to move it to WooFoundation instead.

jaclync · 2025-08-01T04:07:12Z

...ommerce/Classes/POS/Adaptors/Card Present Payments/CardPresentPaymentOnboardingAdaptor.swift

-        onboardingScreenViewModelSubject.send(.showOnboarding(viewModel: onboardingViewModel, onCancel: { [weak self] in
-            self?.readinessSubscription = nil
-        }))
+        onboardingScreenViewModelSubject.send(.showOnboarding(
+            factory: .init(
+                configuration: onboardingViewModel,
+                createView: { [weak self] in
+                    guard let self else { return EmptyView() }
+                    return CardPresentPaymentsOnboardingView(viewModel: onboardingViewModel)
+                }), onCancel: { [weak self] in
+                    self?.readinessSubscription = nil
+                })
+        )


is the requirement of the factory wrapper because CardPresentPaymentsOnboardingView isn't accessible in the POS module? Just wanted to confirm because AnyView down the line might come with performance tradeoff. Moving all the payments code to a new module in the future might resolve this.

I will check if I can work around AnyView change.

Yes, the factory wrapper is required since CardPresentPaymentsOnboardingView and CardPresentPaymentsOnboardingViewModel are IPP views, and then they drag some other dependencies such as CardPresentPaymentsOnboardingUseCase, which in turn drag even more dependencies together with them.

I imagine refactoring them is an option, but it would be a time-consuming option.

jaclync · 2025-08-01T06:27:56Z

Noticed an issue when testing onboarding for a store with incomplete onboarding state where support and learn more CTAs aren't tappable:

I'm jumping onto a task to fix a UI test failure in trunk, sharing my progress so far. The only change that I've tried and fixes the issue so far is:

 /// Displays the WooPayments onboarding UI based on the state.
 struct PointOfSaleCardPresentPaymentOnboardingView: View {
     @ObservedObject var viewModel: PointOfSaleCardPresentPaymentOnboardingViewModel
 
     var body: some View {
         VStack(spacing: Constants.verticalSpacing) {
-            AnyView(viewModel.onboardingViewFactory.createView())
+            CardPresentPaymentsOnboardingView(viewModel: viewModel.onboardingViewFactory.configuration as! CardPresentPaymentsOnboardingViewModel)
                 // Hides the navigation bar title `navigationTitle` in `CardPresentPaymentsOnboardingView`.
                 .toolbar(.hidden)
         }
         .posModalCloseButton(action: viewModel.cancelOnboarding,
                              accessibilityLabel: Localization.cancelOnboarding)
         .safariSheet(url: $viewModel.onboardingURL)
         .posModalSizing()
     }
 }

My guess is that there's something about SwiftUI updates. Before this PR, the payment onboarding VM changes probably trigger PointOfSaleCardPresentPaymentOnboardingView updates. Now that the view becomes an AnyView, the view updates might not be triggered anymore.

jaclync · 2025-08-04T06:44:36Z

Re Noticed an issue when testing onboarding for a store with incomplete onboarding state where support and learn more CTAs aren't tappable issue above:

With the limited debugging today without a working solution, it seems like the issue is with CardPresentPaymentsOnboardingViewModel life cycle management, potentially due to the SwiftUI hierarchy changes now that CardPresentPaymentsOnboardingView is returned as a generic view. The CardPresentPaymentsOnboardingViewModel instance, when the Learn More CTA is tapped (showURL), differs from the instance when initializing the CardPresentPaymentOnboardingViewFactory and PointOfSaleCardPresentPaymentOnboardingViewModel. Therefore, the showURL setup in PointOfSaleCardPresentPaymentOnboardingViewModel doesn't get carried over to the final view model instance.

It's possible that PointOfSaleCardPresentPaymentOnboardingView reacts to its @ObservedObject var viewModel: PointOfSaleCardPresentPaymentOnboardingViewModel changes but CardPresentPaymentsOnboardingViewConfiguration isn't an ObservableObject at least explicitly. I felt like extracting CardPresentPaymentsOnboardingViewModel and CardPresentPaymentsOnboardingView to a shared module might be worth checking. Let me know how you want to proceed, feel free to merge this PR and resolve the onboarding issue later.

staskus · 2025-08-04T07:08:20Z

@jaclync, thanks for checking it deeply! I was investigating the removal AnyView side of things on Friday, but I haven't reached a good solution.

wpmobilebot · 2025-08-08T06:11:24Z

Version 23.0 has now entered code-freeze, so the milestone of this PR has been updated to 23.1.

…ew creation

staskus · 2025-08-11T20:41:14Z

@jaclync, thanks for the review. Only came back to it now!

The reason links didn't work seemed to be easier. The self.onboardingViewModel was a computed variable. Since I used it both in the view, and the configuration I would pass different instances. Passing one seemed to fix the issue.
I couldn't find a good fix for AnyView for now. If we make it generic, we have to pass the type, which is exactly what I'm trying to hide away from POS. I know it's not optimal performance-wise but might be okay for a couple of views that cannot be easily moved away to WooFoundation due to many dependencies but have to be injected to POS. I understand it wouldn't be an option to any views that are performance sensitive, such as rows. I'll continue looking if we don't have too many of these.

staskus added 28 commits July 31, 2025 14:42

Create POSNavigationAdaptor to remove DeepLinkNavigator and AppDelega…

b29dd16

…te dependency

Remove SafariView dependency by moving SafariView to PointOfSaleDashb…

cf92419

…oardView

Create POSExternalViewProvider to extract SupportForm away from POS

5bd198c

Remove CardPresentPaymentsOnboardingViewModel dependency from POS tests

8d9111d

Delete unused CardPresentPaymentError

b785662

Move CardPresentPaymentInvalidatablePaymentOrchestrator and CardPrese…

efb555e

…ntPaymentsConnectionControllerManager to POS Adaptors

Move alerts provider to POS Adaptors

f7d89b1

Move Orders-related POS events to POSAnalyticsEvent to be reachable o…

dc4987f

…n POS module

Move OrderTotalsCalculator to Yosemite to share between Woo app and POS

85c6e66

Remove POSSessionManagerProviding since it was replaced by externalVi…

776fe4c

…ewsProvider

Move VersionHelpers to WooFoundation

b1781c1

Move Array+Helpers to WooFoundation

f8d0cec

Move some String+Helpers to WooFoundation

2e7d591

Move IndefiniteCircularProgressViewStyle to WooFoundation

f45e920

Move KeyboardObserver to WooFoundation

6b4ab2c

Move View+Measurements to WooFoundation

ad46f34

Move View+ScrollModifiers to WooFoundation

01280eb

Move ScrollableVStack to WooFoundation

b874f2c

Move View+Conditionals to WooFoundation

a13955e

Move View+RoundedBorder to WooFoundation

495c14c

Make additional WooFoundation imports

4509f48

Move WooRoundedBorderTextFieldStyle to WooFoundation

d46f71b

Move View+ AutofocusTextModifier to WooFoundation

ba85d6c

Move View+SizeTracker to WooFoundation

f5e18e6

Swiftlint updates

5edfbfc

Move UIImages used in POS to UIImage+POS

264a0c5

staskus added this to the 23.0 milestone Jul 31, 2025

staskus added the type: task An internally driven task. label Jul 31, 2025

staskus added the feature: POS label Jul 31, 2025

staskus requested review from iamgabrielma and jaclync July 31, 2025 20:05

jaclync self-assigned this Aug 1, 2025

jaclync approved these changes Aug 1, 2025

View reviewed changes

wpmobilebot modified the milestones: 23.0, 23.1 Aug 8, 2025

staskus added 4 commits August 11, 2025 22:06

Update IndefiniteCircularProgressViewStyle.swift

40eb857

Mark UIImage+POS specifically from the main bundle

e614f93

Move SafariView to WooFoundation

931b614

Use the same instance of onboardingViewModel for configuration and vi…

41e86f1

…ew creation

staskus merged commit c4810b3 into woomob-935-woo-pos-hack-week-pos-modularization Aug 11, 2025
8 of 13 checks passed

staskus deleted the woomob-935-woo-pos-hack-week-pos-modularization-woo-app-dependencies branch August 11, 2025 20:41

staskus mentioned this pull request Sep 16, 2025

[Woo POS] Modularization: Bring existing changes to trunk #16132

Merged

1 task

		SafariView(url: WooConstants.URLs.pointOfSaleDocumentation.asURL())
		DocumentationView()

[Woo POS][Modularization] Batch 1: Make Woo app dependencies reusable for PointOfSale module #15968

[Woo POS][Modularization] Batch 1: Make Woo app dependencies reusable for PointOfSale module #15968

Uh oh!

Conversation

staskus commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Moved dependencies

Moved to WooFoundation Module

Moved to Yosemite Module

Created POS-Specific Abstractions

Moved to POS Adaptors away from POS module

POS-Specific Assets

Removed Unused Dependencies

Card Present Payment Dependencies

Remaining dependencies to address

Steps to reproduce

Testing information

Uh oh!

dangermattic commented Jul 31, 2025

Uh oh!

wpmobilebot commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaclync left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaclync commented Aug 1, 2025

Uh oh!

jaclync commented Aug 4, 2025

Uh oh!

staskus commented Aug 4, 2025

Uh oh!

wpmobilebot commented Aug 8, 2025

Uh oh!

staskus commented Aug 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

staskus commented Jul 31, 2025 •

edited

Loading

wpmobilebot commented Jul 31, 2025 •

edited

Loading