-
Notifications
You must be signed in to change notification settings - Fork 121
[POS as a tab i2] Show loading & ineligible UI when tapping on the POS tab #15834
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
56e0123
71c16bb
695768f
f19283b
0d89676
44c00e8
5a05b7f
b6b3801
9feda59
33f29a8
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 |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import SwiftUI | ||
| import protocol Experiments.FeatureFlagService | ||
|
|
||
| @available(iOS 17.0, *) | ||
| @Observable final class POSEntryPointController { | ||
| private(set) var eligibilityState: POSEligibilityState? | ||
| private let posEligibilityChecker: POSEntryPointEligibilityCheckerProtocol | ||
| private let featureFlagService: FeatureFlagService | ||
|
|
||
| init(eligibilityChecker: POSEntryPointEligibilityCheckerProtocol, | ||
| featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) { | ||
| self.posEligibilityChecker = eligibilityChecker | ||
| self.featureFlagService = featureFlagService | ||
|
|
||
| guard featureFlagService.isFeatureFlagEnabled(.pointOfSaleAsATabi2) else { | ||
| self.eligibilityState = .eligible | ||
| return | ||
| } | ||
| Task { @MainActor in | ||
| eligibilityState = await posEligibilityChecker.checkEligibility() | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| func refreshEligibility() async throws { | ||
| // TODO: WOOMOB-720 - refresh eligibility | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| import SwiftUI | ||
|
|
||
| /// A view that displays when the Point of Sale (POS) feature is not available for the current store. | ||
| /// Shows the specific reason why POS is ineligible and provides a button to re-check eligibility. | ||
| struct POSIneligibleView: View { | ||
| let reason: POSIneligibleReason | ||
| let onRefresh: () async throws -> Void | ||
| @Environment(\.dismiss) private var dismiss | ||
| @State private var isLoading: Bool = false | ||
|
|
||
| var body: some View { | ||
| VStack(spacing: POSSpacing.large) { | ||
| HStack { | ||
| Spacer() | ||
| Button { | ||
| dismiss() | ||
| } label: { | ||
| Text(Image(systemName: "xmark")) | ||
| .font(POSFontStyle.posButtonSymbolLarge.font()) | ||
| } | ||
| .foregroundColor(Color.posOnSurfaceVariantLowest) | ||
| } | ||
|
|
||
| Spacer() | ||
|
|
||
| VStack(spacing: POSSpacing.medium) { | ||
| Image(PointOfSaleAssets.exclamationMark.imageName) | ||
| .resizable() | ||
| .frame(width: POSErrorAndAlertIconSize.large.dimension, | ||
| height: POSErrorAndAlertIconSize.large.dimension) | ||
|
|
||
| Text(reasonText) | ||
| .font(POSFontStyle.posHeadingBold.font()) | ||
| .multilineTextAlignment(.center) | ||
| .foregroundColor(Color.posOnSurface) | ||
|
|
||
| Button { | ||
| Task { @MainActor in | ||
| do { | ||
| isLoading = true | ||
| try await onRefresh() | ||
| isLoading = false | ||
| } catch { | ||
| // TODO-jc: handle error if needed, e.g., show an error message | ||
| print("Error refreshing eligibility: \(error)") | ||
| isLoading = false | ||
| } | ||
| } | ||
| } label: { | ||
| Text(Localization.refreshEligibility) | ||
| } | ||
| .buttonStyle(POSFilledButtonStyle(size: .normal, isLoading: isLoading)) | ||
| } | ||
|
|
||
| Spacer() | ||
| } | ||
| .padding(POSPadding.large) | ||
| } | ||
|
|
||
| private var reasonText: String { | ||
| switch reason { | ||
| case .notTablet: | ||
| return NSLocalizedString("pos.ineligible.reason.notTablet", | ||
| value: "POS is only available on iPad.", | ||
| comment: "Ineligible reason: not a tablet") | ||
| case .unsupportedIOSVersion: | ||
| return NSLocalizedString("pos.ineligible.reason.unsupportedIOSVersion", | ||
| value: "POS requires a newer version of iOS 17 and above.", | ||
| comment: "Ineligible reason: iOS version too low") | ||
| case .unsupportedWooCommerceVersion: | ||
| return NSLocalizedString("pos.ineligible.reason.unsupportedWooCommerceVersion", | ||
| value: "Please update WooCommerce plugin to use POS.", | ||
| comment: "Ineligible reason: WooCommerce version too low") | ||
| case .wooCommercePluginNotFound: | ||
| return NSLocalizedString("pos.ineligible.reason.wooCommercePluginNotFound", | ||
| value: "WooCommerce plugin not found.", | ||
| comment: "Ineligible reason: plugin missing") | ||
| case .featureSwitchDisabled: | ||
| return NSLocalizedString("pos.ineligible.reason.featureSwitchDisabled", | ||
| value: "POS feature is not enabled for your store.", | ||
| comment: "Ineligible reason: feature switch off") | ||
| case .featureSwitchSyncFailure: | ||
| return NSLocalizedString("pos.ineligible.reason.featureSwitchSyncFailure", | ||
| value: "Could not verify POS feature status.", | ||
| comment: "Ineligible reason: feature switch sync failed") | ||
| case .unsupportedCountry: | ||
| return NSLocalizedString("pos.ineligible.reason.unsupportedCountry", | ||
| value: "POS is not available in your country.", | ||
| comment: "Ineligible reason: country not supported") | ||
| case .unsupportedCurrency: | ||
| return NSLocalizedString("pos.ineligible.reason.unsupportedCurrency", | ||
| value: "POS is not available for your store's currency.", | ||
| comment: "Ineligible reason: currency not supported") | ||
| case .siteSettingsNotAvailable: | ||
| return NSLocalizedString("pos.ineligible.reason.siteSettingsNotAvailable", | ||
| value: "Unable to load store settings for POS.", | ||
| comment: "Ineligible reason: site settings unavailable") | ||
| case .featureFlagDisabled: | ||
| return NSLocalizedString("pos.ineligible.reason.featureFlagDisabled", | ||
| value: "POS feature is currently disabled.", | ||
| comment: "Ineligible reason: feature flag disabled") | ||
| case .selfDeallocated: | ||
| return Localization.defaultReason | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private extension POSIneligibleView { | ||
| enum Localization { | ||
| static let refreshEligibility = NSLocalizedString( | ||
| "pos.ineligible.refresh.button.title", | ||
| value: "Check Eligibility Again", | ||
| comment: "Button title to refresh POS eligibility check" | ||
| ) | ||
|
|
||
| /// Default message shown when POS eligibility reason is not available. | ||
| static let defaultReason = NSLocalizedString( | ||
| "pos.ineligible.default.reason", | ||
| value: "Your store is not eligible for POS at this time.", | ||
| comment: "Default message shown when POS eligibility reason is not available" | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| #Preview { | ||
| POSIneligibleView( | ||
| reason: .unsupportedCurrency, | ||
| onRefresh: {} | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,10 @@ protocol POSEntryPointEligibilityCheckerProtocol { | |
| } | ||
|
|
||
| final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { | ||
| private var siteSettingsEligibility: POSEligibilityState? | ||
| private var featureFlagEligibility: POSEligibilityState? | ||
| private var siteSettingsTask: Task<[SiteSetting], Never>? | ||
|
|
||
| private let siteID: Int64 | ||
| private let userInterfaceIdiom: UIUserInterfaceIdiom | ||
| private let siteSettings: SelectedSiteSettingsProtocol | ||
|
|
@@ -144,6 +148,7 @@ private extension POSTabEligibilityChecker { | |
| async let siteSettingsEligibility = checkSiteSettingsEligibility() | ||
| async let featureFlagEligibility = checkRemoteFeatureEligibility() | ||
|
|
||
| self.siteSettingsEligibility = await siteSettingsEligibility | ||
| switch await siteSettingsEligibility { | ||
| case .eligible: | ||
| break | ||
|
|
@@ -155,6 +160,7 @@ private extension POSTabEligibilityChecker { | |
| } | ||
| } | ||
|
|
||
| self.featureFlagEligibility = await featureFlagEligibility | ||
| switch await featureFlagEligibility { | ||
| case .eligible: | ||
| return true | ||
|
|
@@ -215,6 +221,10 @@ private extension POSTabEligibilityChecker { | |
|
|
||
| private extension POSTabEligibilityChecker { | ||
| func checkSiteSettingsEligibility() async -> POSEligibilityState { | ||
| if let siteSettingsEligibility { | ||
|
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. Until the app is killed and relaunched, will we keep the same eligibility state?
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.
In a future task WOOMOB-720, the refresh eligibility logic will refresh site settings with a remote sync when the ineligible reason is related to the site settings. The app also assumes the same store country/currency throughout the site session as the site settings are refreshed only during site initialization. We could also sync the site settings remotely every time tapping on the POS tab, but I felt like that's optimizing the accuracy for a small ratio of edge cases (site settings changed within the site session) with the tradeoff of an additional API request.
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. Got it, thanks for the explanation! |
||
| return siteSettingsEligibility | ||
| } | ||
|
|
||
| // Waits for the first site settings that matches the given site ID. | ||
| let siteSettings = await waitForSiteSettingsRefresh() | ||
| guard siteSettings.isNotEmpty else { | ||
|
|
@@ -229,14 +239,28 @@ private extension POSTabEligibilityChecker { | |
| } | ||
|
|
||
| func waitForSiteSettingsRefresh() async -> [SiteSetting] { | ||
| for await siteSettings in siteSettings.settingsStream { | ||
| guard siteSettings.siteID == siteID, siteSettings.settings.isNotEmpty, siteSettings.source != .initialLoad else { | ||
| continue | ||
| // Uses a shared task so that multiple calls can await the same result since site settings can be emitted only once. | ||
| if let existingTask = siteSettingsTask { | ||
| return await existingTask.value | ||
| } | ||
|
|
||
| let task = Task<[SiteSetting], Never> { [weak self] in | ||
| guard let self else { return [] } | ||
|
|
||
| for await siteSettings in siteSettings.settingsStream { | ||
| guard siteSettings.siteID == self.siteID, siteSettings.settings.isNotEmpty, siteSettings.source != .initialLoad else { | ||
| continue | ||
| } | ||
| siteSettingsTask = nil | ||
| return siteSettings.settings | ||
| } | ||
| return siteSettings.settings | ||
| // If we get here, the stream completed without yielding any values for our site ID which is unexpected. | ||
| siteSettingsTask = nil | ||
| return [] | ||
| } | ||
| // If we get here, the stream completed without yielding any values for our site ID which is unexpected. | ||
| return [] | ||
|
|
||
| siteSettingsTask = task | ||
| return await task.value | ||
| } | ||
|
|
||
| func isEligibleFromCountryAndCurrencyCode(countryCode: CountryCode, currencyCode: CurrencyCode) -> POSEligibilityState { | ||
|
|
@@ -263,9 +287,13 @@ private extension POSTabEligibilityChecker { | |
| private extension POSTabEligibilityChecker { | ||
| @MainActor | ||
| func checkRemoteFeatureEligibility() async -> POSEligibilityState { | ||
| if let featureFlagEligibility { | ||
| return featureFlagEligibility | ||
| } | ||
|
|
||
| // Only whitelisted accounts in WPCOM have the Point of Sale remote feature flag enabled. These can be found at D159901-code | ||
| // If the account is whitelisted, then the remote value takes preference over the local feature flag configuration | ||
| await withCheckedContinuation { [weak self] continuation in | ||
| return await withCheckedContinuation { [weak self] continuation in | ||
| guard let self else { | ||
| return continuation.resume(returning: .ineligible(reason: .selfDeallocated)) | ||
| } | ||
|
|
||
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.
There's this visible shift between one loading state and another. For some reason, even the background color changes.
Simulator.Screen.Recording.-.iPad.Air.13-inch.M2.-.2025-06-30.at.10.56.56.mov
Uh oh!
There was an error while loading. Please reload this page.
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.
Thanks for catching this! I was testing in dark mode most of the time and didn't notice it. I can reproduce this especially easily by adding a
await Task.sleep(2 * NSEC_PER_SEC)delay toPOSTabEligibilityChecker.checkEligibility.It turns out the background color of the loading state changes because
PointOfSaleLoadingViewdoes not have a background color set. Therefore, the background color depends on the parent view's background color.PointOfSaleLoadingViewis displayed inPointOfSaleEntryPointViewandPointOfSaleDashboardView. Before this PR, the loading state is mostly displayed to the user when embedded inPointOfSaleDashboardViewwith a background color set while loading the products, the use case inPointOfSaleEntryPointViewis just for the POS aggregate model's initialization intaskwhich is almost instant. In this PR, the loading state is being shown fromPointOfSaleEntryPointViewwhen checking POS eligibility async butPointOfSaleEntryPointViewdoesn't have a background color set. In 9feda59, I set the same background color asPointOfSaleDashboardViewtoPointOfSaleLoadingViewso that the loading state has a consistent background color no matter where it is embedded.before:
Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.13.10.01.mp4
after:
Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.12.43.46.mp4
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.
Thanks for addressing this!
I wonder if SwiftUI could keep the same animation running while we switch from entry point loading -> products loading. However, let's not address it now; we have this issue with other animations as well. Probably making
IndefiniteCircularProgressViewStylereliant on a current time when showing progress or something like that could make the transitions smooth.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.
Yea, certainly worth looking into streamlining the spinner animation for the loading view from 2 different places. I'm also still confirming the loading UI in p1751379425731039-slack-C0354HSNUJH, though I think it's pretty likely we will reuse the spinner animation. Created an issue WOOMOB-741.