-
Notifications
You must be signed in to change notification settings - Fork 615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CATALOG-1447 - Update product cards to use price ranges #1143
CATALOG-1447 - Update product cards to use price ranges #1143
Conversation
@@ -1,3 +1,35 @@ | |||
{{#with price_range}} | |||
{{min.formatted}} - {{max.formatted}} | |||
{{/with}} | |||
{{#and price_range.min.with_tax price_range.max.with_tax}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're using a {{#with price_range}}
above we can do {{#and min.with_tax max.with_tax}}
instead and {{#and}}
seems fine to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will update
Autotagging @bigcommerce/storefront-team @davidchin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
{{#and price_range.min.without_tax price_range.max.without_tax}} | ||
<abbr title="{{lang 'products.excluding_tax'}}">{{lang 'products.price_with_tax' tax_label=price_range.min.tax_label}}</abbr> | ||
{{else if schema_org}} | ||
<meta itemprop="availability" content="{{product.availability}}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if it makes sense to move the meta into a price schema component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good to me.
-Remove "as low as" feature -Add new "price range" boolean theme editor setting, defaulting to true, to display ranges -Augment price component to support ranges if available and enabled -Update schema.org to use PriceSpecification so that we can correctly support ranges and also inc tax/exc tax
What?
-Remove "as low as" feature (no longer needed with price ranges)
-Add new "price range" boolean theme editor setting, defaulting to true, to display ranges
-Augment price component to support ranges if available and enabled (graceful fallback for stores not enabled)
-Update schema.org to use PriceSpecification so that we can correctly support ranges and also inc tax/exc tax
Tickets / Documentation
Add links to any relevant tickets and documentation.
Screenshots (if appropriate)
Other important notes