Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Jul 2, 2025

For WOOMOB-743

Just one review is required.

Description

Now that the design is available in y0zZLi1kqybNUUUd8lmvYG-fi-4133_4658, this PR updates the POS ineligible UI based on the design.

POS ineligible UI changes

  • WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift: Updated the POSIneligibleView to show detailed suggestions instead of generic reasons for POS ineligibility. For example, unsupported countries now display a list of supported countries, and unsupported currencies show the supported currencies for the store settings. [1] [2]

Eligibility updates

Test updates

Steps to reproduce

Prerequisite: logged in to a WPCOM account connected to a store not eligible for POS (e.g. WooCommerce version below 9.6.0-beta, unsupported currency) but in an eligible country (US/UK), and a store eligible for POS

  1. Open the WooCommerce app on an iPad
  2. Navigate to a store not eligible for POS as in the prerequisite
  3. Tap on the POS tab --> after the loading UI, the ineligible view should show the reason why it's ineligible (if there are multiple reasons, the order follows the checks in POSTabEligibilityChecker.checkPluginEligibility) and how to fix it
  4. Switch to a store eligible for POS
  5. Tap on the POS tab --> POS should be launched as expected, to the state where products are loaded

Testing information

Screenshots

light dark
Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-02 at 15 05 47 Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-02 at 15 05 50
Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-07-02.at.15.03.50.mp4

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

@dangermattic
Copy link
Collaborator

dangermattic commented Jul 2, 2025

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@jaclync jaclync added type: task An internally driven task. feature: POS labels Jul 2, 2025
@jaclync jaclync added this to the 22.8 milestone Jul 2, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 2, 2025

App Icon📲 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 NameWooCommerce iOS Prototype
Build Numberpr15859-156eefe
Version22.7
Bundle IDcom.automattic.alpha.woocommerce
Commit156eefe
Installation URL31ic8ud46n2s8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync jaclync changed the title DI minimum WooCommerce version to unsupportedWooCommerceVersion case [POS as a tab i2] Show suggestion for each ineligible reason in ineligible UI with design updates Jul 2, 2025
@jaclync jaclync changed the title [POS as a tab i2] Show suggestion for each ineligible reason in ineligible UI with design updates [POS as a tab i2] Update POS ineligible UI with detailed text and design updates Jul 2, 2025
@joshheald joshheald self-assigned this Jul 3, 2025
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Works well, a few notes/suggestions inline 😊

.foregroundColor(Color.posOnSurfaceVariantLowest)
}

VStack(spacing: 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we have POSSpacing.none if you want. It doesn't make a big difference though – I can't see us ever changing the value of that constant!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, updated in 33eb215.

isLoading = false
} catch {
// TODO: WOOMOB-720 - handle error if needed, e.g., show an error message
print("Error refreshing eligibility: \(error)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: DDLogError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, updated in f95efc5.

.buttonStyle(POSOutlinedButtonStyle(size: .normal))
}
.containerRelativeFrame(.horizontal) { length, _ in
length * 0.5 - 132
Copy link
Contributor

Choose a reason for hiding this comment

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

This size prevents any text from showing on the buttons when in a narrow split view, and won't work when we add POS to the phone. I think this needs a minimum value...
narrow buttons in a split view with no text

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the width for the text and buttons to be at least 300px in 156eefe, given that the current iOS devices having 320px minimum width. There's still room for improvement for larger font sizes as you mentioned in WOOMOB-750.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Largest font size:

split view 100% width
Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-03 at 09 46 58 Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-03 at 09 51 32

Smaller font size:

split view 100% width
Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-03 at 09 52 41 Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-03 at 09 52 27

value: "Please update WooCommerce plugin to use POS.",
comment: "Ineligible reason: WooCommerce version too low")
return NSLocalizedString("pos.ineligible.suggestion.unsupportedIOSVersion",
value: "The POS system requires iOS 17 or later. Please update your device to iOS 17+ to use this feature.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
value: "The POS system requires iOS 17 or later. Please update your device to iOS 17+ to use this feature.",
value: "Point of Sale requires iOS 17 or later. Please update your device to iOS 17+ to use this feature.",

"The POS system" feels quite clumsy to read, I'd suggest replacing it with "Point of Sale" everywhere as it's clearer and shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy updated in 8ae5ba5.

comment: "Ineligible reason: not a tablet")
return NSLocalizedString("pos.ineligible.suggestion.notTablet",
value: "Please use a tablet to access POS features.",
comment: "Suggestion for not tablet: use iPad")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the tab show up on the phone? I can't see it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not now and in the near future, but it's an eligibility check that I thought it's better to throw an ineligible error than fatalError or no-op. I can separate the ineligible reason enum into two, one for visibility and the other for eligibility so that this case won't have to be handled. Created an issue in WOOMOB-756.

value: "POS feature is not enabled for your store.",
comment: "Ineligible reason: feature switch off")
return NSLocalizedString("pos.ineligible.suggestion.featureSwitchDisabled",
value: "The POS core feature must be enabled to proceed. " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
value: "The POS core feature must be enabled to proceed. " +
value: "Point of Sale must be enabled to proceed. " +

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy updated in 8ae5ba5.

comment: "Ineligible reason: feature switch off")
return NSLocalizedString("pos.ineligible.suggestion.featureSwitchDisabled",
value: "The POS core feature must be enabled to proceed. " +
"Please enable the POS feature from your WordPress admin under WooCommerce settings > Advanced > Features.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an API we can use to just do this when people tap a button on this screen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, there is pdfdoF-6VP-p2#endpoint, I created an issue to integrate with the API in WOOMOB-759.

let countryNames = supportedCountries.map { $0.readableCountry }
let formattedCountryList = ListFormatter.localizedString(byJoining: countryNames)
let format = NSLocalizedString(
"pos.ineligible.suggestion.unsupportedCountry",
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 see the tab in unsupported countries yet; I'm assuming that's just not changed yet 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to the previous comment https://github.com/woocommerce/woocommerce-ios/pull/15859/files#r2182744134, it's because the ineligible enum includes the cases that make the POS tab invisible. I will handle that in the separate issue linked there.

comment: "Suggestion for disabled feature flag: notify that POS is disabled remotely")
case .selfDeallocated:
return Localization.defaultReason
return NSLocalizedString("pos.ineligible.suggestion.selfDeallocated",
Copy link
Contributor

Choose a reason for hiding this comment

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

😬 hope we don't need this one!

@jaclync jaclync requested review from a team and removed request for a team July 3, 2025 12:45
@jaclync
Copy link
Contributor Author

jaclync commented Jul 3, 2025

Thanks for the testing and review feedback Josh! I believe I responded to all of your comments and am merging the PR now. Feel free to add more to the threads and I will respond to them after the merge.

@jaclync jaclync merged commit 335e348 into trunk Jul 3, 2025
13 checks passed
@jaclync jaclync deleted the feat/WOOMOB-743-ineligible-ui-suggestion branch July 3, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants