Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

Add open-graph page metadata for social sharing #76

Merged
merged 4 commits into from
Sep 20, 2018

Conversation

dalibor
Copy link
Contributor

@dalibor dalibor commented Sep 12, 2018

Reference: http://ogp.me/

Copy link
Contributor

@chrisvogt chrisvogt left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. Now we won't see that default banner for every article shared!

<meta property="og:url" content="{{ site.url }}{{ page.url}}">

{% if page.cover %}
<meta name="og:image" content="{{ site.url }}{{ page.cover }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update these to use the property attribute instead of name? I've been seeing these warnings in the Facebook debugger for the live site:

screen shot 2018-09-12 at 8 23 15 am

{% else %}
<meta name="og:image" content="{{ site.url }}/assets/images/banner.png">
{% endif %}

{% if page.title %}
<meta property="og:title" content="{% if page.title %}{{ page.title }} — {% endif %}{{ site.title }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Since you are already in this file, would you mind fixing my redundant check around page.title? No problem if you don't. This line can just be:

<meta property="og:title" content="{{ page.title }} — {{ site.title }}">

{% endif %}

{% if page.title %}
<meta name="twitter:title" content="{{ page.title }} | @GodaddyOSS">
{% endif %}

{% if page.excerpt %}
<meta property="og:description" content="{{ page.excerpt }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It would be nice to go through and give all top-level pages excerpts, but that is also fine to save for another PR.

@dalibor
Copy link
Contributor Author

dalibor commented Sep 12, 2018

@chrisvogt Thanks, pushed the changes.

Copy link
Contributor

@chrisvogt chrisvogt left a comment

Choose a reason for hiding this comment

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

LG. Thanks!

@chrisvogt chrisvogt merged commit be9172f into godaddy:master Sep 20, 2018
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