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

og image update #656

Merged
merged 4 commits into from
Sep 28, 2023
Merged

og image update #656

merged 4 commits into from
Sep 28, 2023

Conversation

chrisdale-io
Copy link
Contributor

  1. Updating default og image (change of image path)
  2. Simplifying the previous code implementation in the route file. Replacing complex layering of tiled background texture image over background svg, to simply a background png.

@changeset-bot
Copy link

changeset-bot bot commented Sep 28, 2023

⚠️ No Changeset found

Latest commit: 9dbf838

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Sep 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
keystar-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 28, 2023 7:28am
keystatic ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 28, 2023 7:28am
keystatic-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 28, 2023 7:28am

Comment on lines 32 to 33
backgroundImage:
'url(https://thinkmill-labs.keystatic.net/keystatic-site/images/km9uvzfb8fqo/og-image-bg)',
Copy link
Member

Choose a reason for hiding this comment

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

This background image doesn't seem to be used? so the background is just the colour? I did some investigation and it seems like it's because of vercel/satori#273 and Keystatic Cloud serving webp by default, you should be able to fix that by using https://thinkmill-labs.keystatic.net/keystatic-site/images/km9uvzfb8fqo/og-image-bg?format=png (from testing that seemed to solve the error from webp but the image still wasn't actually being displayed)

Copy link
Contributor Author

@chrisdale-io chrisdale-io Sep 28, 2023

Choose a reason for hiding this comment

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

Hey Emma, thanks for looking into this. I had a bit of a look and test a few ideas from what you've come back with, with no success. I'm not sure why it's not loading up the background image, as it seemed to work fine from the implementation before the brand update work!?

The previous code for the background image in the original version was this:
backgroundImage: 'url(https://thinkmill-labs.keystatic.net/keystatic-site/images/nt24l5xes6jj/open-graph-bg)',

It's strange, if I add the old code into a fresh OG image playground, the image doesn't load up for 'SVG (Satori)' preview tab, but shows on the 'HTML (Native)' tab (which seems to be what's happening with my new implementation too), but the old code seemed to serve up dynamic og images (again before I adjusted things 🙈):

Original Dynamic OG Image

Copy link
Member

Choose a reason for hiding this comment

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

From checking out the commit before the brand updates (183e129) and trying it now, it seems as though it broke. I'm guessing that would have broke when we added image transforms to *.keystatic.net 😬 (though adding ?format=png to the url in 183e129 does seem to work)

Copy link
Member

Choose a reason for hiding this comment

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

Just pushed a commit that seems to fix it, it seems like having an image bigger than the image that's being created made something decide to not render it so resizing the image to be the same size as the output worked

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