-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix twitter card not showing our image #124
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently our specified image is not being shown on twitter cards, for example in https://twitter.com/msuxg/status/1407254592333697024 In our html we currently have ``` <meta property="og:image" content="https://www.gov.uk/alerts/assets/images/opengraph-image.png?f86f1d0dd106bfbcd8ad1ee5ea68e75e"> ... <meta property="og:image" content="/assets/images/govuk-opengraph-image.png"> ``` where the first is the image we want to show and the second is html added by the GOV.UK Frontend. https://github.com/alphagov/govuk-frontend/blob/v3.12.0/src/govuk/template.njk#L26 Note the second meta image currently 404s for us because it is a relative url rather than an absolute url. Twitter should be following the spec of open graph, where the first image is given preference and it ignores the second - https://ogp.me/#array. However, I think it is not following this correctly and that is why our image is not showing on twitter cards. This hypothesis backed by a manual test by me to remove the second meta tag and I see that twitter renders our card with our first image correctly. Therefore it leaves us with a few options 1. Get twitter to fix a potential bug with their twitter card implementation 2. Change the second meta tag to use an absolute URL 3. Get the design system to remove their fallover tag 4. Add in specific meta tags for twitter images Option 1 and 3 will take time and effort. Option 2 may not work because we won't have a file hosted in our assets directory at that location because of #118 Therefore I've gone for option 4 because I've tested it and it appears to work and it is also quick and non controversial. You can see the spec documented at https://developer.twitter.com/en/docs/twitter-for-websites/cards/overview/markup
rjbaker
approved these changes
Jun 25, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good and passes Twitter Card validation.
I've seen a few pages on GOV.UK that only specify the og:image
, rather than the twitter:image
specifically, but this will definitely work. Thanks for links to context.
36degrees
added a commit
to alphagov/govuk-frontend
that referenced
this pull request
Jun 24, 2022
We previously told users that if they needed to override the OpenGraph image they should add their own meta tag to the head block and that this would override the one in the template (based on the spec). Some users have found that this isn’t reliable [1] or can result in multiple images being displayed [2] – and it’s a waste of bytes to include a redundant meta tag. Instead, allow the OpenGraph image URL in the template to be set using a new `opengraphImageUrl` Nunjucks option. If no `opengraphImageUrl` is provided, keep the existing behaviour that uses `assetUrl` as a prefix for a non-customisable path of `images/govuk-opengraph-image.png`. However, when neither `opengraphImageUrl` or `assetUrl` are passed, no longer fall back to using the `assetPath` option. OpenGraph image URLs must be absolute [3] and using the `assetPath` only gives us a relative URL, so we should just omit the OpenGraph image tag in these cases. This is a change in behaviour, as previously an `og:image` meta tag would always be included, but it only be valid if `assetUrl` had been set. Fixes #1920. [1]: alphagov/emergency-alerts-govuk#124 [2]: #1920 (comment) [3]: https://ogp.me/#data_types
36degrees
added a commit
to alphagov/govuk-frontend
that referenced
this pull request
Jun 27, 2022
We previously told users that if they needed to override the OpenGraph image they should add their own meta tag to the head block and that this would override the one in the template (based on the spec). Some users have found that this isn’t reliable [1] or can result in multiple images being displayed [2] – and it’s a waste of bytes to include a redundant meta tag. Instead, allow the OpenGraph image URL in the template to be set using a new `opengraphImageUrl` Nunjucks option. If no `opengraphImageUrl` is provided, keep the existing behaviour that uses `assetUrl` as a prefix for a non-customisable path of `images/govuk-opengraph-image.png`. However, when neither `opengraphImageUrl` or `assetUrl` are passed, no longer fall back to using the `assetPath` option. OpenGraph image URLs must be absolute [3] and using the `assetPath` only gives us a relative URL, so we should just omit the OpenGraph image tag in these cases. This is a change in behaviour, as previously an `og:image` meta tag would always be included, but it only be valid if `assetUrl` had been set. Fixes #1920. [1]: alphagov/emergency-alerts-govuk#124 [2]: #1920 (comment) [3]: https://ogp.me/#data_types
querkmachine
pushed a commit
to alphagov/govuk-frontend
that referenced
this pull request
Oct 17, 2022
We previously told users that if they needed to override the OpenGraph image they should add their own meta tag to the head block and that this would override the one in the template (based on the spec). Some users have found that this isn’t reliable [1] or can result in multiple images being displayed [2] – and it’s a waste of bytes to include a redundant meta tag. Instead, allow the OpenGraph image URL in the template to be set using a new `opengraphImageUrl` Nunjucks option. If no `opengraphImageUrl` is provided, keep the existing behaviour that uses `assetUrl` as a prefix for a non-customisable path of `images/govuk-opengraph-image.png`. However, when neither `opengraphImageUrl` or `assetUrl` are passed, no longer fall back to using the `assetPath` option. OpenGraph image URLs must be absolute [3] and using the `assetPath` only gives us a relative URL, so we should just omit the OpenGraph image tag in these cases. This is a change in behaviour, as previously an `og:image` meta tag would always be included, but it only be valid if `assetUrl` had been set. Fixes #1920. [1]: alphagov/emergency-alerts-govuk#124 [2]: #1920 (comment) [3]: https://ogp.me/#data_types
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently our specified image is not being shown on twitter cards, for
example in https://twitter.com/msuxg/status/1407254592333697024
In our html we currently have
where the first is the image we want to show and the second is html
added by the GOV.UK Frontend.
https://github.com/alphagov/govuk-frontend/blob/v3.12.0/src/govuk/template.njk#L26
Note the second meta image currently 404s for us because it is
a relative url rather than an absolute url.
Twitter should be following the spec of open graph, where the first
image is given preference and it ignores the second -
https://ogp.me/#array.
However, I think it is not following this correctly and that is why our image is not showing
on twitter cards. This hypothesis backed by a manual test by me to remove the
second meta tag and I see that twitter renders our card with our first
image correctly.
Therefore it leaves us with a few options
implementation
Option 1 and 3 will take time and effort.
Option 2 may not work because we won't have a file hosted in our assets
directory at that location because of #118
Therefore I've gone for option 4 because I've tested it and it appears
to work and it is also quick and non controversial. You can see the spec documented
at
https://developer.twitter.com/en/docs/twitter-for-websites/cards/overview/markup
For extra context, see https://gds.slack.com/archives/CAF8JA25U/p1624438487103700