feat: Add items stock/backorder messages to cart page #2758
Conversation
🦋 Changeset detectedLatest commit: fa85981 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
The failing type check is temporary because this storefront PR did not hit production yet. |
|
|
||
| const formattedLineItems = lineItems.map((item) => { | ||
| if (item.__typename === 'CartGiftCertificate') { | ||
| const formattedLineItems = await Promise.all( |
There was a problem hiding this comment.
This is quite a large method, maybe we can refactor this into smaller utils that are easier to follow and read.
There was a problem hiding this comment.
Actually, the Promise.all() is not needed anymore as we now get all the data we want from the line item data.
|
Already reviewed by UI/design team, and their comments are applied. |
matthewvolk
left a comment
matthewvolk
left a comment
There was a problem hiding this comment.
Quick comment and a question 👍
| <Plus | ||
| className={clsx( | ||
| 'text-[var(--cart-counter-icon,hsl(var(--contrast-300)))] transition-colors duration-300', | ||
| )} | ||
| size={18} | ||
| strokeWidth={1.5} | ||
| /> |
There was a problem hiding this comment.
Is it intentional for the "Plus" button to be missing the hover: style changes like the "Minus" button has?
There was a problem hiding this comment.
Actually no ... it was a change to be reversed but was not fully done. I changed it back.
There was a problem hiding this comment.
The only question I have left here is related to the screenshot from your PR desc:
If I have 4 in my cart, but only 3 are available in the store (where 1 is ready to ship, and 2 will be on backorder), what happens to the 4th item unaccounted for in the backorder messaging? Is the intent that clicking "Proceed to checkout" will return an error?
There was a problem hiding this comment.
Is the intent that clicking "Proceed to checkout" will return an error?
Yes. This is highlighting the error for the shopper to fix before "proceed to checkout". If they didn't fix the quantities, they will get the error of they attempt to checkout. I also changed the counter border color in case of quantity error to highlight the error (as in the updated screenshot in the PR description).
migueloller
left a comment
migueloller
left a comment
There was a problem hiding this comment.
I have two minor comments. Thanks!
| defaultOutOfStockMessage | ||
| showOutOfStockMessage | ||
| showBackorderMessage |
There was a problem hiding this comment.
Out of curiosity, are these messages translated?
There was a problem hiding this comment.
Yes, the admin setup is still in progress. As part of the final definition of done, the messages will be translated in the inventory service via the unified translations API.
| /> | ||
| </button> | ||
| </div> | ||
| {!!lineItem.inventoryMessages?.outOfStockMessage && ( |
There was a problem hiding this comment.
Instead of coercing to a boolean with !! can we do != null? That would be more consistent with the rest of the codebase.
Tharaae
requested review from
andrewreifman,
animesh1987,
matthewvolk and
migueloller
What/Why?
Add ready-to-ship quantity, backordered quantity, full/partial out-of-stock messages to cart line items on cart page.
Testing
Screenshots
Migration
While rebasing, there might be some conflicts if there is some custom styling on cart page. Those conflicts may occur in the following files: