fix(storefront): STRF-2456 Fixing Schema Org meta tags #1233
Conversation
|
Autotagging @bigcommerce/storefront-team @davidchin |
davidwrpayne
changed the title
|
@mattolson @Ubersmake @Souldjinn @mjschock This is ready for review |
CHANGELOG.md
Outdated
| @@ -12,6 +12,7 @@ | |||
| - Fix for excess whitespace in multiline text field product option [#1222](https://github.com/bigcommerce/cornerstone/pull/1222) | |||
| - Fix for faceted search display. [#1225](https://github.com/bigcommerce/cornerstone/pull/1225) | |||
| - Fix for calls with empty files in Safari. [#1210](https://github.com/bigcommerce/cornerstone/pull/1210) | |||
| - Fix product pricing schema.org microdata. [#1233](https://github.com/bigcommerce/cornerstone/pull/1233) | |||
There was a problem hiding this comment.
This needs to go into the Draft section.
| @@ -1,5 +1,5 @@ | |||
| {{#and price_range.min.with_tax price_range.max.with_tax}} | |||
| <div class="price-section price-section--withTax" {{#if schema_org}}itemprop="offers" itemscope itemtype="http://schema.org/Offer"{{/if}}> | |||
There was a problem hiding this comment.
Why is this removed? We don't care about the withTax case? Wouldn't you need to add a div below for the microdata to be understood, or are you just orphaning that data?
There was a problem hiding this comment.
I believe the div with class can stay however the schema meta needs to be removed. Will fix.
There was a problem hiding this comment.
I don't understand. This is embedded in templates/components/products/price.html without any "offers" wrapper so why is it ok to remove this? If you are removing it here, should you add the wrapper div below?
davidwrpayne
force-pushed
the
STRF-2456
branch
6 times, most recently
from
d84e86e
to
0892688
Compare
|
Much slimmer changeset. |
| <abbr title="{{lang 'products.excluding_tax'}}">{{lang 'products.price_with_tax' tax_label=price_range.min.tax_label}}</abbr> | ||
| {{else if schema_org}} | ||
| <abbr title="{{lang 'products.including_tax'}}">{{lang 'products.price_with_tax' tax_label=price_range.min.tax_label}}</abbr> | ||
|
|
| <abbr title="{{lang 'products.including_tax'}}">{{lang 'products.price_with_tax' tax_label=price_range.min.tax_label}}</abbr> | ||
|
|
||
| {{/and}} | ||
| {{#if schema_org}} |
| @@ -13,7 +15,7 @@ | |||
| <meta itemprop="priceCurrency" content="{{currency_selector.active_currency_code}}"> | |||
| <meta itemprop="valueAddedTaxIncluded" content="true"> | |||
| </div> | |||
| {{/and}} | |||
| {{/if}} | |||
| @@ -28,6 +29,7 @@ <h2 class="productView-brand"{{#if schema}} itemprop="brand" itemscope itemtype= | |||
| {{/or}} | |||
| </div> | |||
| {{{region name="product_below_price"}}} | |||
| {{#if product.rating}} | |||
There was a problem hiding this comment.
This seems unrelated to the change. Is there another bug you are trying to fix?
There was a problem hiding this comment.
yes we were getting rating sections showing up on products without ratings
There was a problem hiding this comment.
Ok, with the introduction of this new block, please indent the contents
| <h2 class="productView-brand"{{#if schema}} itemprop="brand" itemscope itemtype="http://schema.org/Brand"{{/if}}> | ||
| <a href="{{product.brand.url}}"{{#if schema}} itemprop="url"{{/if}}><span{{#if schema}} itemprop="name"{{/if}}>{{product.brand.name}}</span></a> | ||
| </h2> | ||
| {{#if product.brand}} |
There was a problem hiding this comment.
This seems unrelated to the change. Is there another bug you are trying to fix?
There was a problem hiding this comment.
yes. we were getting brand sections showing up on products without brands.
davidwrpayne
force-pushed
the
STRF-2456
branch
2 times, most recently
from
b15e84b
to
ec64daa
Compare
…and moving some tags to the appropriate div
What?
We are adding schema.org offer tags on div's that schema.org doesn't care about what price was
it only cares about what price currently is afaict I need to validate still that price-range schema works.
Tickets / Documentation
Screenshots
Fixed product schema with "was" and "currently" prices
