Skip to content

Conversation

@kevinxh
Copy link
Contributor

@kevinxh kevinxh commented Sep 21, 2021

Description

Note: this work is built on top of the wishlist_rework branch, for easier PR review, the base branch for this PR is temporarily pointing to wishlist_rework. (after wishlist_rework is merged, this will point back to develop)

Enable wishlist functionalities for Einstein recommended products.

Screen Shot 2021-09-21 at 4 01 47 PM

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • Rename wishlist icon to heart icon to better represent the actual svg icon
  • Decouple ProductTile from ProductScroller so it's more easier to customize the tiles in the scroller
  • Update RecommendedProducts component to have wishlist functionalities
  • Re work ProductTile component, update the API of this component to be clean and easy to use.
    • decouple the concept of "wishlist" from product tile, instead use "favourite" (make this is an isolated React component for re-usability)

OLD product tile API:

<ProductTile
  isWishlistLoading={wishlistLoading.includes(
    productId
  )}
  productSearchItem={productSearchItem}
  onAddToWishlistClick={() =>
    addItemToWishlist(productSearchItem)
  }
  onRemoveWishlistClick={() => {
    removeItemFromWishlist(productSearchItem)
  }}
  isInWishlist={isInWishlist}
/>

NEW api:

<ProductTile
  product={product}
  enableFavourite={wishlist.isInitialized}
  isFavourite={isInWishlist}
  onFavouriteToggle={(isFavourite) => {
    if (isFavourite) {
      addItemToWishlist(productSearchItem)
    } else {
      removeItemFromWishlist(productSearchItem)
    }
  }}
/>

How to Test-Drive This PR

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

Base automatically changed from wishlist_rework to develop September 24, 2021 22:30
productSearchItem={product}
onClick={() => onProductClick(product)}
/>
renderProduct(product)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's hard to customize the product tiles in the scroller so this is my attempt to decouple the two components...

feedback is welcomed

} = props
const {currency, image, price, productName} = productSearchItem
const styles = useMultiStyleConfig('ProductTile', {isLoading: isWishlistLoading})
const {product, enableFavourite = false, isFavourite, onFavouriteToggle, ...rest} = props
Copy link
Contributor Author

@kevinxh kevinxh Sep 24, 2021

Choose a reason for hiding this comment

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

i'd like to de-couple the concept of wishlist from the components like ProductTile, so they are more re-usable.

i intentionally renamed wishlist related props to "favourite", which i think is more generic. feedback is welcomed

@bendvc
Copy link
Contributor

bendvc commented Sep 24, 2021

Couple of comments on general usability:

  1. For those users that are unfortunate to be on slow 3g connections, there is no immediate feedback when clicking the heart icon. To be more app like I'd assume we want to have instant feedback. Looking at https://pwa-kit.mobify-storefront.com/ it looks like there is some feedback in the form of turning the heart grey-ish. Looks like there might be a regression.
  2. Clicking on the heart for recommended items is being blocked on the far right item, I'm assuming it's from the scrollers button maybe?
    image

return null
}

// TODO: DRY the wishlist handlers when intl
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably shouldn't knowingly leave todo's in the production code.

Copy link
Contributor

@bendvc bendvc left a comment

Choose a reason for hiding this comment

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

Please remove the "todo" comment and we are good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants