Add stock level message to product page based on inventory settings #2619
Conversation
🦋 Changeset detectedLatest commit: 0e66b8f 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.
3 Skipped Deployments
|
| } | ||
|
|
||
| if ( | ||
| stockLevelDisplay === 'DONT_SHOW' || |
There was a problem hiding this comment.
Can we type 'DONT_SHOW' and other values in an enum?
There was a problem hiding this comment.
The type of stockLevelDisplay is already defined by bigcommerce graphql data types as a set of values. This is common for graphql fields in Catalyst.
| return null; | ||
| } | ||
|
|
||
| return t('ProductDetails.currentStock', { |
There was a problem hiding this comment.
It's defined on this line where translations related to "Product" page are fetched.
|
|
||
| if ( | ||
| stockLevelDisplay === 'DONT_SHOW' || | ||
| (stockLevelDisplay === 'SHOW_WHEN_LOW' && |
There was a problem hiding this comment.
This condition feels a bit overloaded and hard to read. Can this be simplified? Maybe we can use a util method?
There was a problem hiding this comment.
I'll decompose it to be more clear like:
if (stockLevelDisplay === 'DONT_SHOW') {
return null;
}
if (stockLevelDisplay === 'SHOW_WHEN_LOW') {
const { availableToSell, warningLevel } = product.inventory.aggregated ?? {};
if (!warningLevel) {
return null;
}
if (availableToSell && availableToSell > warningLevel) {
return null;
}
}
|
LGTM |
There was a problem hiding this comment.
Thanks @Tharaae for this! Small change, otherwise looks good on our end!
| {Boolean(product.stockLevelMessage) && ( | ||
| <p className="prose prose-sm">{product.stockLevelMessage}</p> | ||
| )} |
There was a problem hiding this comment.
Since you are a Streamable, we should wrap it in a <Stream> component to actually stream this data in on page load otherwise it'll be a blocking request. Additionally, we should be using the CSS variables for the stylings here so we get the benefit of Makeswift theming.
| {Boolean(product.stockLevelMessage) && ( | |
| <p className="prose prose-sm">{product.stockLevelMessage}</p> | |
| )} | |
| <Stream fallback={<ProductStockSkeleton />} value={product.stockLevelMessage}> | |
| {(stockLevelMessage) => | |
| Boolean(stockLevelMessage) && ( | |
| <p className="text-[var(--product-detail-secondary-text,hsl(var(--contrast-500)))]"> | |
| {stockLevelMessage} | |
| </p> | |
| ) | |
| } | |
| </Stream> |
There was a problem hiding this comment.
Thanks so much for adding this! I'm going to approve this, but one last thing before you merge, can you add a changeset to this PR? All you need to run is pnpm changeset from the root folder and it'll walk you through some questions. Space to select, enter to continue – you'll press enter on the first question in order to get to the next (you'll see what I mean 😅).
What/Why?
As part of backorders project, we want to give the merchant an option to display a backorders prompt to the shopper on the product details page for products that have backordering available. This need to be built on the stock level message feature (as in cornerstone) that is not implemented as a catalyst default feature.
This PR is to implement the stock level message on the product details page based on the store inventory settings.
Testing
Screenshots
When the product is in stock and "Show stock levels" is selected:
When the product is in stock and "Don's show stock levels" is selected:
When the product is in stock but below or equal warning level and "Only show stock levels when stock is low" is selected:
When the product is in stock and above warning level and "Only show stock levels when stock is low" is selected:
When the product is not in stock and "Out of stock message" is set:
Migration
None