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

Fixes and improvements to meta tags #192

Merged
merged 5 commits into from
Sep 4, 2020
Merged

Fixes and improvements to meta tags #192

merged 5 commits into from
Sep 4, 2020

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented Aug 14, 2020

  • Fix invalid meta tags (using invalid property attribute rather than name EDIT: non opengraph tags using the non-standard property attribute, which should only be used for opengraph / RDFa tags)
  • Move robots and Google site verification meta tag logic to the meta_tags helper
  • Make it possible to exclude individual pages from search engine indexes by setting prevent_indexing: true in the page frontmatter

@36degrees 36degrees marked this pull request as draft August 17, 2020 07:53
@36degrees
Copy link
Contributor Author

Realised that OpenGraph meta tags do in fact use the non-standard property attribute. I think that means the non-opengraph description meta tag is incorrect, but the others were actually correct before.

Will need to work out how to make the meta tags helper handle both cases.

The `property` attribute is non-standard for `<meta>` elements, and is only intended to be used for opengraph meta tags (taken from RDFa). The correct attribute for other meta tags [1] (including Twitter's meta tags [2]) is `name`.

Split the opengraph meta tags into a separate function which we can call from the view, allowing us to loop over the 'standard' and 'opengraph' meta tags separately, using `name` and `property` respectively.

[1]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta#Attributes
[2]: https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started
Make it possible to add noindex robots meta tags to individual pages by adding `prevent_indexing: true` to the page frontmatter.

Because the meta_tags helper now checks for the prevent_indexing property on the data object, we would now have to add the `prevent_indexing` attribute to every page_metadata double otherwise we would see the error:

> #<Double "page_frontmatter"> received unexpected message :prevent_indexing with (no args)

To avoid this, replace the current `data` test doubles with hashes, and update the meta_tags helper to use the standard Ruby syntax to read values out of the hashes, rather than relying on the method access [1] feature that we get from current_page.data being a Middleman::Util::EnhancedHash [2].

[1]: https://github.com/hashie/hashie/tree/b24d6dca2c545637bc3cc3ac4d89f565fc27a9d0#methodaccess
[2]: https://github.com/middleman/middleman/blob/d7f0ed06d3bf10bd8bb7f9abfda87fcdfbb3c363/middleman-core/lib/middleman-core/util/data.rb#L18
@36degrees
Copy link
Contributor Author

I've updated the meta tags helper to return opengraph and non-opengraph tags separately, which I think solves the issue mentioned in the previous comment…

@36degrees 36degrees marked this pull request as ready for review September 4, 2020 10:40
Copy link
Contributor

@freesteph freesteph left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

36degrees added a commit to alphagov/tdt-documentation that referenced this pull request Sep 4, 2020
@36degrees 36degrees merged commit 3164ce3 into master Sep 4, 2020
@36degrees 36degrees deleted the meta-tags branch September 4, 2020 12:06
36degrees added a commit to alphagov/tdt-documentation that referenced this pull request Sep 4, 2020
Depends on alphagov/tech-docs-gem#192 being merged and released.

Co-authored-by: Mark Green <mark.green@digital.cabinet-office.gov.uk>
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.

2 participants