feat: Add items stock/backorder messages to cart page #2758
Conversation
🦋 Changeset detectedLatest commit: ea74a65 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.
2 Skipped Deployments
|
|
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.
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?
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.
| </button> | ||
| </div> | ||
| {!!lineItem.inventoryMessages?.outOfStockMessage && ( | ||
| <span className="text-xs/4 font-light text-red-500"> |
There was a problem hiding this comment.
@Tharaae There needs to be a little more space between the number counter and text that may appear below it. Could you please also increase the line-height/leading of the backorder messages too?
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: