Skip to content
This repository was archived by the owner on Aug 30, 2018. It is now read-only.

Fix structured data price rounding issue and usage warning#583

Merged
cshold merged 2 commits intoShopify:masterfrom
Jore:fix-product-price-structured-data
Sep 29, 2016
Merged

Fix structured data price rounding issue and usage warning#583
cshold merged 2 commits intoShopify:masterfrom
Jore:fix-product-price-structured-data

Conversation

@Jore
Copy link
Contributor

@Jore Jore commented Sep 27, 2016

Fix rounding issue caused by using divided_by Math filter.
> Divides an output by a number. The output is rounded down to the nearest integer.

i.e. 3499.95 was being rounded down to 3499
See: https://help.shopify.com/themes/liquid/filters/math-filters#divided_by

Bring price implementation inline with usage guidelines. No longer produces warning in Google's Structured Data Testing Tool for values greater than 999.99 that use comma separator for readability.

> Use '.' (Unicode 'FULL STOP' (U+002E)) rather than ',' to indicate a decimal point. Avoid using these symbols as a readability separator.

See: http://schema.org/price

Updated pull request based on solution in comments provided by @cshold

@Jore Jore changed the title Fix price rounding issue and usage warning Fix structured data price rounding issue and usage warning Sep 27, 2016
@cshold
Copy link
Contributor

cshold commented Sep 28, 2016

We've updated this on some of our other themes and used divided_by: 100.00 — specifying the decimals will tell it what to round to. Using remove isn't usually a great fix because of how many different currency types there are.

@Jore
Copy link
Contributor Author

Jore commented Sep 28, 2016

Fair call with different currency types. I have revised the pull request with divided_by: 100.00.

Much better solution, thank you.

@cshold
Copy link
Contributor

cshold commented Sep 29, 2016

Thanks for the PR and the update. 👍

@cshold cshold merged commit 6990367 into Shopify:master Sep 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants