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

✨ 🐛 Move meta description into ghost head #8150

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Mar 13, 2017

closes #4424

This is the final major functional change that I want to get into Ghost 1.0 for themes.

The idea, based on the discussion in #4424, is to move the meta title and meta description tags to be output in ghost head.

Meta title is always set, this has no effect other than some very old themes will have duplicate titles. We can implement a warning for this in gscan. I believe search engines use the first if there are 2.

Meta description is not always set. With this change, if it is unset, we don't output a meta description tag. This will also have the same issue where old themes will get duplicates & again we can add this as a warning in gscan.

Note that this change still does not allow us to do this in the templates:

{{#if meta_description}}
.. do something if there is a meta description
{{else}}
.. do something if there is  no meta_description
{{/unless}}

because meta description is still a function.

I've been holding off on making this change, because I thought I also needed to fix the ability to do an if statement on meta_description. However, having thought about it some more I can't think of a really good usecase. I mean, it's nice-to-have, but there's no clear-as-day jumps out at me reason other than my brain telling me we probably should allow it.

Adding the ability to do this later also would be an addition, rather than a breaking change so no reason to hold off the breaking change for Ghost 1.0 based on this... I think.

Thoughts please @JohnONolan ?

@ErisDS ErisDS added the themes label Mar 13, 2017
@ErisDS
Copy link
Member Author

ErisDS commented Mar 14, 2017

Hmmm... thinking about this some more - did I jump the 🔫 by moving the title into {{ghost_head}} as well?

I thought it would be easier to do both - then users don't have to think about any meta data at all, but then I got to thinking how the structure of a head section is usually:

<head>
   <title>{{meta_title}}</title>
   ... lots of other custom stuff...
   {{ghost_head}}
</head>

And wondered if it would be weird to have title output so far down? I know the search engines don't care but mebbe this is confusing for developers?

closes TryGhost#4424

- meta description is an optional SEO tag that we can provide when we have sensible output
- in the cases where we have no useful output, we should not output the tag at all
- ghost_head now takes care of this, and themes should not include their own meta description tag
@ErisDS ErisDS changed the title meta desc and title in ghost head ✨ 🐛 Move meta description into ghost head Mar 14, 2017
@ErisDS
Copy link
Member Author

ErisDS commented Mar 14, 2017

Based on the above (and with approval from JO'N) I have updated this PR to ONLY move meta description into ghost head.

There is no bug with the title, the title is always required, and it is normal to put it at the top of the markup. There is no benefit to moving it into ghost_head and requiring developers to change their themes.

However, meta description is a different case. It is optional. Search engines won't penalise you for not including it, or for having two (it would just grab the first one) but what will result in problems is having a tag with an empty content attribute as we have had in Ghost for such a long time.

By moving meta description into ghost_head we give ghost full control over the meta description to only output it when we have something sensible to output.

Moving forward we can look to making it possible to detect if there is a meta description available, but that is not required in order to make this change.

Copy link
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

LGTM 👍
@ErisDS will update casper, because otherwise we would have two meta descriptions in the head.

@kirrg001 kirrg001 merged commit b8162b1 into TryGhost:master Mar 14, 2017
ErisDS added a commit to TryGhost/Casper that referenced this pull request Mar 14, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
kirrg001 added a commit to TryGhost/Casper that referenced this pull request Mar 14, 2017
refs TryGhost/Ghost#4424

- as of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output
kirrg001 pushed a commit to TryGhost/Casper that referenced this pull request Mar 14, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
@ErisDS ErisDS deleted the meta-in-head branch March 18, 2017 12:58
baldwinjj pushed a commit to baldwinjj/Casper that referenced this pull request Apr 6, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
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.

Don't output meta tags with blank content
2 participants