[Woo POS] [Variable Products] Add parent product to the POSItem enum, and make UI #14771

jaclync · 2024-12-30T00:46:08Z

Description

This pull request adds support for showing the parent product card for a variable product in the POS item selector. The main changes include adding a new ParentProductCardView, creating reusable view modifiers for item card styles and a reusable image view to share UI in the simple and parent product card views.

New components and views:

WooCommerce/Classes/POS/Presentation/ParentProductCardView.swift: Added a new ParentProductCardView struct to display parent products in the POS system. This view includes a detailed view for variable products. When supporting other product types with a parent product (e.g. bundle), the detailed view can be configured based on the parent product type.
WooCommerce/Classes/POS/Presentation/Reusable Views/POSItemImageView.swift: Introduced a new POSItemImageView struct to handle the display of product images within POS item cards. I was wondering if we could just use a placeholder image in ProductImageThumbnail, but didn't want to change the pre-existing behavior until we're working on UI/UX polishes.

Refactoring and reusable styles:

WooCommerce/Classes/POS/Presentation/Reusable Views/POSItemCardBorderStylesModifier.swift: Created a new POSItemCardBorderStylesModifier struct to apply consistent border styles to item cards. This modifier is used in both SimpleProductCardView and ParentProductCardView.
WooCommerce/Classes/POS/Presentation/SimpleProductCardView.swift: Refactored SimpleProductCardView to use the new POSItemImageView and POSItemCardBorderStylesModifier for improved code reusability and consistency. [1] [2]

These changes collectively improve the maintainability and visual consistency of the POS product card views.

Steps to reproduce

🗒️ As the variable products support require WC version 9.6+ which is not publicly released at the time of writing, I'd recommend commenting out L214 in ProductsRemote to include all product types.

Switch to a store with at least one variable product
Go to Menu > Point of Sale --> scroll down the list, and make sure the variable products show up in the list without a price and with "Options available" below the product name (no changes expected in the simple product card UI to minimize the risk of this PR)

Testing information

Compared the product card UI with trunk to ensure there are no UI changes in the simple product cards.

Screenshots

dark	light

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

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

…roducts based on product type in item service, and show new `ParentProductCardView` for parent products.

…card views.

wpmobilebot · 2024-12-30T00:57:17Z

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

	App Name	WooCommerce iOS
	Build Number	pr14771-093b952
	Version	21.3
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`093b952`
	App Center Build	WooCommerce - Prototype Builds #12307

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

staskus

Thanks for the work!

I'm approving. Good job on making certain parts reusable while splitting the parent and simple product use-cases which are likely to keep diverging in the future 👍 .

Before merging:

Address the large accessibility sizes issue

For this / future tasks:

Make sure we don't leave POS in a half-working state during the project development
Try to reuse existing colors / clarify if "Options available" is supposed to be a special case in the POS color pallete.

Thanks!

staskus · 2024-12-30T09:21:09Z

WooCommerce/Classes/POS/Presentation/SimpleProductCardView.swift

+                             imageSize: Constants.productCardSize * scale,
+                             scale: scale)
+            .frame(width: min(Constants.productCardSize * scale, Constants.maximumProductCardSize),
+                   height: Constants.productCardSize * scale)


Suggested change

height: Constants.productCardSize * scale)

height: Constants.productCardSize * scale)

.clipped()

I think we need to re-add clipped to limit the growth of images for extremely large accessibility sizes:

With Clipped Without Clipped

For ParentProductCardView as well.

Great catch, clipped needs to be placed after the frame. I think it'd be great to remove the if/else condition in the product image view and use a placeholder image instead of setting up the view differently based on the image source.

IMO the ideal UX is to scale to fit with the same aspect ratio, I checked a few product images and they are cropped too much. I compared it with trunk, the cropping behavior is the same but it's out of the scope of this PR. I created a separate issue for this: #14776.

Fixed in 093b952.

I created a separate issue for this: #14776.

Thank you!

staskus · 2024-12-30T09:21:35Z

WooCommerce/Classes/POS/Presentation/ParentProductCardView.swift

+                             imageSize: Constants.productCardSize * scale,
+                             scale: scale)
+            .frame(width: min(Constants.productCardSize * scale, Constants.maximumProductCardSize),
+                   height: Constants.productCardSize * scale)


Suggested change

height: Constants.productCardSize * scale)

height: Constants.productCardSize * scale)

.clipped()

Fixed in 093b952.

WooCommerce/Classes/POS/Presentation/ItemListView.swift

staskus · 2024-12-30T09:34:49Z

WooCommerce/Classes/POS/Utils/Color+WooCommercePOS.swift

@@ -87,6 +87,17 @@ extension Color {
        )
    }

+    // MARK: - POS Items
+
+    static var posItemSecondaryText: Color {


I wonder if we should have a separate color only for the "Options Available" text. We cleaned up after M1 to limit the number of colors and have a consistent POS color palette. I see we started reintroducing more view-specific colors. I suppose the reason is new designs that do not follow a consistent color palette. Did you take this color from Figma?

IMHO, ideally we would have primary/secondary/tertiary text and background colors which would cover most if not all the POS. For some special scenarios, we would define colors within specific views.

This comment is not a blocker, although it would be great if we reuse the existing color and clarify if this particular color is intended to be reused anywhere else.

Yes, this gray shade 60 is from the design rNAclWFAdQRw7EXEcsNVai-fi-2653_11376. It makes sense to reuse the colors from the POS color constants. I also had to guess the dark mode version, so we will want a design review anyways. Updated in d68a7f0.

jaclync · 2024-12-31T08:57:45Z

Thanks for catching the large font size issue and the feedback on reusing the colors @staskus! I believe I've addressed the comments and updated the PR. The screenshots in the PR description are also up to date with the existing POS secondary text color, which looks good in both light and dark modes.

jaclync added 4 commits December 25, 2024 16:19

Add parentProduct: POSParentProduct case to POSItem enum, parse p…

09768cc

…roducts based on product type in item service, and show new `ParentProductCardView` for parent products.

Reuse border styles and image view in both simple and parent product …

0a79a0f

…card views.

Revert changes on the simple product card UI.

8fda85c

ParentProductCardView: polishes.

bcda668

jaclync added type: task An internally driven task. status: feature-flagged Behind a feature flag. Milestone is not strongly held. feature: POS labels Dec 30, 2024

jaclync added this to the 21.4 milestone Dec 30, 2024

jaclync marked this pull request as ready for review December 30, 2024 01:12

jaclync requested a review from staskus December 30, 2024 01:13

staskus approved these changes Dec 30, 2024

View reviewed changes

jaclync added 2 commits December 31, 2024 16:36

Remove one-off color for parent product card secondary text.

d68a7f0

Move clipped to be after frame for it to work in large font sizes.

093b952

jaclync enabled auto-merge December 31, 2024 08:58

jaclync merged commit 0fbafb4 into trunk Dec 31, 2024
13 checks passed

jaclync deleted the feat/14700-support-variable-product-items branch December 31, 2024 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Woo POS] [Variable Products] Add parent product to the POSItem enum, and make UI #14771

[Woo POS] [Variable Products] Add parent product to the POSItem enum, and make UI #14771

jaclync commented Dec 30, 2024 •

edited

Loading

wpmobilebot commented Dec 30, 2024 •

edited

Loading

staskus left a comment

staskus Dec 30, 2024

jaclync Dec 31, 2024

jaclync Dec 31, 2024

jaclync Dec 31, 2024

staskus Dec 31, 2024

staskus Dec 30, 2024

jaclync Dec 31, 2024

staskus Dec 30, 2024

jaclync Dec 31, 2024 •

edited

Loading

jaclync commented Dec 31, 2024

	height: Constants.productCardSize * scale)
	height: Constants.productCardSize * scale)
	.clipped()

[Woo POS] [Variable Products] Add parent product to the POSItem enum, and make UI #14771

[Woo POS] [Variable Products] Add parent product to the POSItem enum, and make UI #14771

Conversation

jaclync commented Dec 30, 2024 • edited Loading

Description

Steps to reproduce

Testing information

Screenshots

Reviewer (or Author, in the case of optional code reviews):

wpmobilebot commented Dec 30, 2024 • edited Loading

staskus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaclync Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

jaclync commented Dec 31, 2024

jaclync commented Dec 30, 2024 •

edited

Loading

wpmobilebot commented Dec 30, 2024 •

edited

Loading

jaclync Dec 31, 2024 •

edited

Loading