ALFMOB-162: Improve Wishlist Functionality on PLP & Wishlist #42

JoaoPinhoMinder · 2025-03-11T15:14:45Z

Ticket

https://mindera.atlassian.net/browse/ALFMOB-162

Depends on: #41

Description

Created WishlistProduct and BagProduct inheriting from SelectedProduct.
Made WishlistProduct unique by product ID and color ID.
Made BagProduct unique by product ID and variant SKU.
Removed size information from WishlistView and ProductListingView.
Updated "Add to Bag" action in WishlistView to redirect users to PDP for size selection instead of adding directly to the bag.
Disabled "Add to Bag" button on PDP until both color (pre-selected) and size have been selected.

Evidences

Screen.Recording.2025-03-12.at.09.40.43.mov

reisdev

Great additions! Just a couple comments for improvements 🤌🏼

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift

p4checo · 2025-03-24T19:49:24Z

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift


-    init(productId: String, product: Product?, dependencies: ProductDetailsDependencyContainer) {
+    var isAddToBagEnabled: Bool {
+        productHasStock


this could probably be inlined? 💅🏼

p4checo · 2025-03-24T19:50:19Z

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift


-        if let baseProduct {
+        switch (product, selectedProduct) {
+        case (.some(let product), .none):


please avoid explicit pattern matching to Optional's .some / .none, and use let foo, let bar? or nil instead

p4checo · 2025-03-24T19:51:16Z

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift

+    init(
+        productId: String,
+        product: Product?,
+        selectedProduct: SelectedProduct? = nil,


this API seems a bit overly complex, do we always need these 3 parameters, or could we have N different init overloads?

p4checo · 2025-03-24T19:53:10Z

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift

+        return SelectedProduct(product: product, selectedVariant: selectedVariant)
    }
-}
+} // swiftlint:disable:this file_length


we can add this at the top of the file via a single // swiftlint:disable file_length

however, would it be possible to modularize this logic so we keep file length under control? 🤓

p4checo · 2025-03-24T19:53:34Z

Alfie/Alfie/Views/WishlistView/WishlistView.swift

 #endif

 struct WishlistView<ViewModel: WishlistViewModelProtocol>: View {
+    @EnvironmentObject var coordinador: Coordinator


coordinador -> coordinator 😆

p4checo · 2025-03-24T19:56:52Z

Alfie/AlfieKit/Sources/Models/Models/Product/SelectedProduct/SelectedProduct.swift

@@ -0,0 +1,61 @@
+import Foundation
+
+public class SelectedProduct: Identifiable, Equatable, Hashable {


Hashable: Equatable, so the Equatable conformance is redundant

p4checo · 2025-03-24T19:57:11Z

Alfie/AlfieKit/Sources/Models/Models/Product/SelectedProduct/SelectedProduct.swift

+    public let product: Product
+    public let selectedVariant: Product.Variant
+
+    public var name: String {


please inline these computed properties for a more compact implementation 💄
(same thing for the other types)

p4checo · 2025-03-24T19:57:49Z

Alfie/AlfieKit/Sources/Models/Models/Product/SelectedProduct/SelectedProduct.swift

+
+    // MARK: Equatable
+
+    public static func == (lhs: SelectedProduct, rhs: SelectedProduct) -> Bool {


this is usually a bad™ idea (same thing for the custom Hashable conformance). Why do we need this?
(same thing for the other types)

Since i had opened this PR in a wrong way, some discussions can also be threaten in this PR #41. Now i have changed the target branch to point to the branch of whoich this one depends on.

p4checo · 2025-03-24T20:00:34Z

Alfie/AlfieKit/Sources/Models/Models/Product/SelectedProduct/BagProduct.swift

@@ -0,0 +1,9 @@
+import Foundation
+
+public class BagProduct: SelectedProduct {


not a big fan of having models as classes... why do we need this? (same thing for the other Product types)

I agree with you, did this only because for this use case, i needed to support in wishlist to have the "same" item multiple times if it is added in multiple color variants. Also, i require the Product.Variant to never have a size associated. So i end up with this strategy so i can override the id and also have specific initializations to omit size without polluting the VM with that specific logic.

But i'm open to ideas in this topic, i feel we are fighting the models we are receiving at the moment and exposed that situation which the solution was to mention that in the ticket 😅

p4checo · 2025-03-24T20:02:13Z

...eKit/Sources/StyleGuide/Theme/Components/SizingBanner/ColorAndSizingSelectorHeaderView.swift

                            .foregroundStyle(Colors.primary.mono400)
-                        Text.build(theme.font.paragraph.normal(configuration.selectedItem?.name ?? ""))
-                            .foregroundStyle(Colors.primary.mono900)
+                        Text.build(theme.font.paragraph.normal(


indentation is not right™, should probably be:

Text.build( theme.font.paragraph.normal( configuration.selectedItem?.name ?? configuration.noItemSelectedTitle ) )

- Created `WishlistProduct` and `BagProduct` inheriting from `SelectedProduct`. - Made `WishlistProduct` unique by **product ID and color ID**. - Made `BagProduct` unique by **product ID and variant SKU**. - Removed size information from `WishlistView` and `ProductListingView`. - Updated **"Add to Bag"** action in `WishlistView` to redirect users to **PDP** for size selection instead of adding directly to the bag. - Disabled **"Add to Bag"** button on **PDP** until both **color** (pre-selected) and **size** have been selected.

Fix rebases

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch 4 times, most recently from 37c5029 to 399378d Compare March 12, 2025 09:20

JoaoPinhoMinder marked this pull request as ready for review March 12, 2025 09:52

JoaoPinhoMinder requested review from DanielPrimoMinder, p4checo and reisdev March 12, 2025 09:52

JoaoPinhoMinder marked this pull request as draft March 12, 2025 10:16

JoaoPinhoMinder removed request for DanielPrimoMinder, p4checo and reisdev March 12, 2025 10:16

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch from 399378d to b07538a Compare March 12, 2025 10:38

JoaoPinhoMinder marked this pull request as ready for review March 12, 2025 10:38

JoaoPinhoMinder requested review from DanielPrimoMinder, p4checo and reisdev March 12, 2025 10:38

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch from b07538a to f5fc889 Compare March 12, 2025 16:47

JoaoPinhoMinder mentioned this pull request Mar 13, 2025

ALFMOB-158: Update PDP Button Copy Based on Item State #44

Draft

reisdev requested changes Mar 19, 2025

View reviewed changes

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift Outdated Show resolved Hide resolved

Alfie/Alfie/Views/ProductDetails/ProductDetailsViewModel.swift Outdated Show resolved Hide resolved

p4checo suggested changes Mar 24, 2025

View reviewed changes

JoaoPinhoMinder marked this pull request as draft March 26, 2025 12:26

JoaoPinhoMinder mentioned this pull request Mar 31, 2025

ALFMOB-149: Bag & Wishlist - Redirect to PDP #41

Merged

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch from f5fc889 to 50f05b4 Compare March 31, 2025 16:11

JoaoPinhoMinder changed the base branch from main to task/ALFMOB-149-bag-and-wishlist-redirect-to-pdp March 31, 2025 16:11

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch 2 times, most recently from 02e96df to f69d424 Compare April 1, 2025 16:06

JoaoPinhoMinder force-pushed the task/ALFMOB-149-bag-and-wishlist-redirect-to-pdp branch from 46f9f58 to 17c4347 Compare May 5, 2025 15:57

Base automatically changed from task/ALFMOB-149-bag-and-wishlist-redirect-to-pdp to main May 5, 2025 16:05

ALFMOB-162: Code review

18bc8b0

JoaoPinhoMinder force-pushed the task/ALFMOB-162-improve-wishlist-functionality-on-plp-and-wishlist branch from ae00fcf to 18bc8b0 Compare May 5, 2025 16:16

JoaoPinhoMinder added 2 commits May 5, 2025 17:34

ALFMOB-162:

3824e3a

Fix rebases

ALFMOB-162: Code review

e7722e1

JoaoPinhoMinder marked this pull request as ready for review May 5, 2025 16:42

JoaoPinhoMinder requested review from p4checo and reisdev May 5, 2025 16:42

		@@ -0,0 +1,61 @@
		import Foundation

		public class SelectedProduct: Identifiable, Equatable, Hashable {


		// MARK: Equatable

		public static func == (lhs: SelectedProduct, rhs: SelectedProduct) -> Bool {

		@@ -0,0 +1,9 @@
		import Foundation

		public class BagProduct: SelectedProduct {

ALFMOB-162: Improve Wishlist Functionality on PLP & Wishlist #42

Are you sure you want to change the base?

ALFMOB-162: Improve Wishlist Functionality on PLP & Wishlist #42

Uh oh!

Conversation

JoaoPinhoMinder commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Description

Evidences

Uh oh!

reisdev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JoaoPinhoMinder commented Mar 11, 2025 •

edited

Loading