-
Notifications
You must be signed in to change notification settings - Fork 7
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
add link preview image #328
base: main
Are you sure you want to change the base?
Conversation
@@ -24,13 +24,17 @@ | |||
<meta property="og:title" content="{{title}} - Data.gov" /> | |||
<meta property="og:description" content="{{excerpt}}" /> | |||
<meta property="og:site_name" content="{{ site.title }}"/> | |||
{% assign img = "https://s3-us-gov-west-1.amazonaws.com/cg-0817d6e3-93c4-4de8-8b32-da6919464e61/hero-image-bg.png" %} |
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.
When we have a new image in the future, this img will have a broken link. can we add a test on this to ensure the link is valid?
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.
Yes, that is a good idea. Ideally when/if we update the image we just replace the old image with a new with the same filename. I do think it would be wise to make sure that it is a valid image, though. Working on a test now.
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.
can you test it with this shortcode on your local and see if that works?
// Image exists in S3
{% image "my-image.png" "My PNG Image Alternative Name" false %}
https://github.com/GSA/datagov-11ty/tree/main?tab=readme-ov-file#referencing-images
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.
or better, we can make it a 11ty config item, so that it will always point to the latest url if the image is updated in the 11ty config.
I dont like relying on developer to remember to edit this line whenever there is a image update.
I dont like adding a test either. Even there is a updated image, the old image might not get deleted from S3 therefore the old url is still valid and will pass the test.
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.
or better, we can make it a 11ty config item, so that it will always point to the latest url if the image is updated in the 11ty config.
wouldn't we then have to update the config whenever the image is updated? 🌀
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.
Working on a test now.
Did you ever get that test finished @robert-bryson ?
Related to GSA/data.gov#4723
Changes proposed in this pull request:
security considerations
[Note the any security considerations here, or make note of why there are none]