Skip to content
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

Updates Dawn to use the new structured_data filter #3380

Merged
merged 2 commits into from
May 30, 2024

Conversation

lhoffbeck
Copy link
Contributor

@lhoffbeck lhoffbeck commented Mar 29, 2024

PR Summary:

Replace product and article structured data generated in the theme with structured data generated by the structured_data filter.

Why are these changes introduced?

Updating themes to stay on the greenpath of structured data developments is currently pretty painful.

What approach did you take?

Other considerations

Decision log

# Decision Alternatives Rationale Downsides
1

Visual impact on existing themes

Testing steps/scenarios

  • Step 1

Demo links

Checklist

@bakura10
Copy link

bakura10 commented Apr 4, 2024

That's a very nice update. A few notes though:

@willbroderick
Copy link

willbroderick commented Apr 4, 2024

May I suggest this be implemented as a containerless {{ structured_data }} added to layout/theme.liquid?

This would allow Shopify to contextually include Organization, WebSite, Article, BreadcrumbList, FAQPage, anything else that might be useful, and adapt to best practices in future.

@lhoffbeck
Copy link
Contributor Author

lhoffbeck commented Apr 4, 2024

That's a very nice update. A few notes though:

Awesome, I'll take a look at the gist 🙌 This approach will let us roll out support for new features like ProductGroup and multipage (hello combined listings) without requiring theme updates

Totally, it's on the roadmap

@lhoffbeck
Copy link
Contributor Author

May I suggest this be implemented as a containerless {{ structured_data }} added to layout/theme.liquid?

This would allow Shopify to contextually include Organization, WebSite, Article, BreadcrumbList, FAQPage, anything else that might be useful, and adapt to best practices in future.

That's great feedback, I'll take this back to the team

@tairli
Copy link
Contributor

tairli commented Apr 16, 2024

What would be the recommended way to integrate product reviews?
It was popular to add the data from metafields into the ld+json element like in @bakura10 gist.

@bakura10
Copy link

@lhoffbeck , just an idea here, but what about simplifying this to:

{% structured_data %}

or:

{{ request | structured_data }}

This way Shopify could automatically generate either product or blog post metadata, but also the website metadata that you guys already have ; see my Gist for all the data). That would simplify it a bit and would handle any other metadata that Google might add in the future.

@lhoffbeck
Copy link
Contributor Author

lhoffbeck commented Apr 24, 2024

@bakura10 yeah good idea, I have it in a backlog ticket. It could be a both/and improvement along with this :)

What would be the recommended way to integrate product reviews?
It was popular to add the data from metafields into the ld+json element like in @bakura10 gist.

@tairli great question! Shopify-generated data only includes information that does not require any inference. Since reviews tend to be stored as metafields and don't have a standard definition, they don't fall under this case. However, the Shopify data can be trivially extended by adding another block with the same @id to the page. For example:

<!-- data from {{ product | structured_data }} -->
<script type="application/ld+json">
{
   "@context":"http://schema.org/",
   "@id":"/products/my-product#product",
   "name":"My product",
   ... other fields ...
}
</script>

<!-- @tairli's enhanced data -->
<script type="application/ld+json">
{
   "@context":"http://schema.org/",
   "@id":"/products/my-product#product",
   "aggregateRating": {
        "@type": "AggregateRating",
        "ratingValue": "5",
        "reviewCount": "100"
   }
}
</script>

@bakura10
Copy link

@lhoffbeck I don't agree with that. This is what the standard rating metafield is for :).

@lhoffbeck
Copy link
Contributor Author

@bakura10 Because it's a metafield the type is standardized but not the data, so surfacing it to everyone using the filter could return the wrong values. Something we could potentially explore is adding an option in the metafield definition to surface it in structured_data 🤔 For now I'd recommend extending though.

@bakura10
Copy link

Makes sense, but I think this will cause us a lot of support :/. Not all review apps actually output this, so merchants will see the missing review and will just come back to us on how to add it. It would definitely be needed for merchants to have control over that.

@joshistoast
Copy link

One edge case to look out for is 3rd party review apps, would it be worth exploring an argument expecting a specific reviews structure so we can format and use those?

@lhoffbeck
Copy link
Contributor Author

Thanks @joshistoast ! This should support 3p reviews apps already, the flow would look something like this:

  • Review app sets metafields on the product
  • Merchant's theme outputs product structured data using the filter
  • App injects structured data for the review into the storefront, with an "@id": "{{ product.url }}#product". Google combines the app block with the structured_data filter block by the @id prop.

App dev guidance to the merchant would be something like: if you use the structured_data filter, review app structured data works automatically. If you customize structured_data, either ensure an "@id": "{{ product.url }}#product" is present or include this review block: {"aggregateRating": ...}

would it be worth exploring an argument expecting a specific reviews structure so we can format and use those

Can you tell me more about what you're thinking here?

@lhoffbeck lhoffbeck force-pushed the lh-migrate-product-structured-data-to-server branch from 047bf3a to d07e613 Compare May 14, 2024 17:10
@joshistoast
Copy link

Can you tell me more about what you're thinking here?

I'm more-so curious to overriding values in case the shape of data is odd or id I's like to change it in some way, in which case I'm sure the traditional structured data approach would be best.

@lhoffbeck

@lhoffbeck
Copy link
Contributor Author

Can you tell me more about what you're thinking here?

I'm more-so curious to overriding values in case the shape of data is odd or id I's like to change it in some way, in which case I'm sure the traditional structured data approach would be best.

@lhoffbeck

@joshistoast Gotcha. Google doesn't seem to allow for overriding values by linking multiple structured data blocks by @id, only extending the definitions. If you need more customization than that, then continuing with the liquid-generated structured data is probably your best bet.

@lhoffbeck lhoffbeck force-pushed the lh-migrate-product-structured-data-to-server branch from d07e613 to 8861398 Compare May 29, 2024 17:11
@lhoffbeck lhoffbeck marked this pull request as ready for review May 29, 2024 17:12
@lhoffbeck lhoffbeck requested a review from dan-menard May 29, 2024 17:12
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the property-based SEO from this file. Google interprets this as a duplicate item when present along with ld+json: https://search.google.com/test/rich-results/result?id=6ClESn2Nb85_l_LNc1XbVA

@PaulNewton
Copy link

Another reason we need to be able to override an objects properties , create custom objects*, or some sort of preprocessing in liquid.

Google doesn't seem to allow for overriding values by linking multiple structured data blocks by @id, only extending the definitions. If you need more customization than that, then continuing with the liquid-generated structured data is probably your best bet.

And shopify doesn't let us override object values either , for countless business scenarios situations such as contact-for-pricing ad nausem infinity there's all the surfaces that have to be edited AND merchants then get locked out of future enhancements like the structured_data filter that won't have granular parameters.

Though if there's a non-liquid workaround to that let us override ld+json with just another ld+json block containt only the slice that needs to be customized that would be just as amazing and simplify a lot of customizations by taking advantage of the structured_data for the core of the admin data then simple snippets for the one offs in the theme without the error proneness of editing themes liquid-templated-json, or capture & replace tactics.

*also can't use metaobjects because their access structure doesn't let use create pseudo products we could slap in. i.e. {% assign catalog_products = shop.metaobjects.catalog_product.values | first %} ...metaobject forloop... {{ catalog_product | structured_data }}

Copy link
Contributor

@tyleralsbury tyleralsbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. Loving the net -71 line diff, too. :chefs-kiss:

@lhoffbeck lhoffbeck merged commit abbf0dd into main May 30, 2024
8 checks passed
@lhoffbeck lhoffbeck deleted the lh-migrate-product-structured-data-to-server branch May 30, 2024 15:49
@haroldao
Copy link

image An error is returned in the article template...

@lhoffbeck
Copy link
Contributor Author

@haroldao can you send me which page this is on?

@haroldao
Copy link

I imported the updated theme on my demo store

@lhoffbeck
Copy link
Contributor Author

Can you drop a link so I can take a look?

@haroldao
Copy link

image
It seems like it has been fixed! Ty @lhoffbeck!

@lhoffbeck
Copy link
Contributor Author

No problem! Thanks for reporting it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants