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

feat(open-graph): Allow custom OG images from any paths or URLs #1592

Open
wants to merge 2 commits into
base: v4
Choose a base branch
from

Conversation

Stephen-X
Copy link
Contributor

@Stephen-X Stephen-X commented Nov 16, 2024

Proposing a small change to remove the static directory restriction when setting OG image path in frontmatter. Now it's either relative to the base URL, or a full URL.

So if someone wants to reuse an image assets/cover.png from their blog post in the content directory, they can set socialImage to say assets/cover.png. If the image is in the static directory, then they need to explicitly include it in the path: static/cover.png. If it's hosted elsewhere e.g. https://example.com/cover.png, then socialImage value should be the full URL including the HTTP protocol.

@Stephen-X Stephen-X changed the title feat(open-graph): Allow custom OG images from any directories feat(open-graph): Allow custom OG images from content directory Nov 16, 2024
@saberzero1
Copy link
Collaborator

saberzero1 commented Nov 16, 2024

Just a thought, but can't you already do this by prepending ../ to the link in frontmatter?

Basically the URL isn't restricted to the assets folder.

@Stephen-X
Copy link
Contributor Author

Stephen-X commented Nov 16, 2024

Just a thought, but can't you already do this by prepending ../ to the link in frontmatter?

Basically the URL isn't restricted to the assets folder.

I just learned about dot-segments in URIs for the first time 😄

Wouldn't it be a bit hacky and esoteric to non web developers though? I'd assume reusing an image from a post as OG image is a common pattern.

IMHO localhost:8080/static/../assets/cover.png is also a bad URI format even though it's supported, and it's not efficient for CDN caching too (it may be treated as a separate cache item at least for the first time? I'm not entirely sure if it's the browser that does the parsing here).

@saberzero1
Copy link
Collaborator

Just a thought, but can't you already do this by prepending ../ to the link in frontmatter?

Basically the URL isn't restricted to the assets folder.

I just learned about dot-segments in URIs for the first time 😄

Wouldn't it be a bit hacky and esoteric to non web developers though? I'd assume reusing an image from a post as OG image is a common pattern.

IMHO localhost:8080/static/../assets/cover.png is also a bad URI format even though it's supported, and it's not efficient for CDN caching too (it may be treated as a separate cache item at least for the first time? I'm not entirely sure if it's the browser that does the parsing here).

Arguably, one should not be putting an image from the note as the og-image, as those should be limited in resolution. Basically I assume IF you use an image from your note as your page-specific og-image, that you generally would use a lower resolution version.

Also, I'm pretty sure the og-image used on anything except the page itself. Unless you're rich-linking to the page you're currently on, which seems redundant.

@Stephen-X
Copy link
Contributor Author

Stephen-X commented Nov 18, 2024

I'd assume reusing an image from a post as OG image is a common pattern.

Arguably, one should not be putting an image from the note as the og-image, as those should be limited in resolution. Basically I assume IF you use an image from your note as your page-specific og-image, that you generally would use a lower resolution version.

Also, I'm pretty sure the og-image used on anything except the page itself. Unless you're rich-linking to the page you're currently on, which seems redundant.

I didn't mean the original images exactly when I said "reusing", I only meant a version of it: definitely needs to be downsized and cropped first, but reusing an image from the article is quite common. Many substack articles do this for example, here's one.

My 2 cents on why article-specific OG images (i.e. custom OG in frontmatter) may not typically be from the static dir:

  1. For professionals, they'd use some paid image resize service to do it. Take CloudFlare Images for example, you can do https://<ZONE>/cdn-cgi/image/width=80,quality=75/uploads/avatar1.jpg. So in frontmatter just setting socialImage: cdn-cgi/image/width=80,quality=75/uploads/avatar1.jpg is cleaner.

  2. For casual users who would be generating a low res image for each article, I'd think it's more convenient to keep everything in one place in the content directory (which also, bonus point, does not have a subdirectory in URL) rather than putting OG optimized images separately under quartz/static; quartz is also program directory, I felt it may not be the best place for frequently updated content assets.

@saberzero1 Please feel free to close this PR if you think there is a need to enforce OG image to be stored in quartz/static by default.

@saberzero1
Copy link
Collaborator

I'd assume reusing an image from a post as OG image is a common pattern.

Arguably, one should not be putting an image from the note as the og-image, as those should be limited in resolution. Basically I assume IF you use an image from your note as your page-specific og-image, that you generally would use a lower resolution version.

Also, I'm pretty sure the og-image used on anything except the page itself. Unless you're rich-linking to the page you're currently on, which seems redundant.

I didn't mean the original images exactly when I said "reusing", I only meant a version of it: definitely needs to be downsized and cropped first, but reusing an image from the article is quite common. Many substack articles do this for example, here's one.

My 2 cents on why article-specific OG images (i.e. custom OG in frontmatter) may not typically be from the static dir:

  1. For professionals, they'd use some paid image resize service to do it. Take CloudFlare Images for example, you can do https://<ZONE>/cdn-cgi/image/width=80,quality=75/uploads/avatar1.jpg. So in frontmatter just setting socialImage: cdn-cgi/image/width=80,quality=75/uploads/avatar1.jpg is cleaner.

  2. For casual users who would be generating a low res image for each article, I'd think it's more convenient to keep everything in one place in the content directory (which also, bonus point, does not have a subdirectory in URL) rather than putting OG optimized images separately under quartz/static; quartz is also program directory, I felt it may not be the best place for frequently updated content assets.

@saberzero1 Please feel free to close this PR if you think there is a need to enforce OG image to be stored in quartz/static by default.

My biggest caveat with your first point is that neither the current version nor your proposed change allows for arbitrary CDNs to work.

@Stephen-X
Copy link
Contributor Author

IMHO localhost:8080/static/../assets/cover.png is also a bad URI format even though it's supported, and it's not efficient for CDN caching too (it may be treated as a separate cache item at least for the first time? I'm not entirely sure if it's the browser that does the parsing here).

My biggest caveat with your first point is that neither the current version nor your proposed change allows for arbitrary CDNs to work.

It wouldn't after the PR, because a dev would be using the same URL (cache key) to point to the same asset as they no longer need to add /../ to their URLs to offset the hardcoded static subdirectory?

But in any case, I just ran a quick test, my theory about CDN cache miss got debunked as I seemingly could confirm the alternative behavior I put in parentheses, that the dot-segments are resolved on the client side. Sorry that thought only crossed my mind naturally because of my day job in networking, should have validated end to end first.

Screenshot 2024-11-18 at 6 49 18 PM

But I'd think it's a minor point compared to the other much more straightforward ones I mentioned, so it's not exactly a big caveat?

Again, this is kind of a design / UX thing so I respect whatever decision maintainers made on this. Please feel free to close this PR if you think there is a need to enforce OG image to be stored in quartz/static by default.

@saberzero1
Copy link
Collaborator

saberzero1 commented Nov 19, 2024

I was more talking about that IF you have an external CDN to host your images, it won't work in the current and proposed change anyway. So your point about external CDNs is kinda moot either way.

Arguably it should be supported, but that's not necessarily related to this PR.

A simple solution could be to do some checks. Like if the URL starts with http/https then assume external site and act accordingly. If not, look in the local files.

I'm not entirely sure if Quartz includes all image files in the content folder, even if they are not linked anywhere. If they aren't, we should tell Quartz to add any image linked in frontmatter to the build as well. The static folder always emits all files inside.

@Stephen-X
Copy link
Contributor Author

I'm not entirely sure if Quartz includes all image files in the content folder, even if they are not linked anywhere. If they aren't, we should tell Quartz to add any image linked in frontmatter to the build as well. The static folder always emits all files inside.

Yes Quartz does emit all files in the content folder. That's how I serve robots.txt and the CSP header file for my site actually, they have to be accessible from the root dir so can't be in quartz/static.

I was more talking about that IF you have an external CDN to host your images, it won't work in the current and proposed change anyway. So your point about external CDNs is kinda moot either way.

Arguably it should be supported, but that's not necessarily related to this PR.

A simple solution could be to do some checks. Like if the URL starts with http/https then assume external site and act accordingly. If not, look in the local files.

Thank you for explaining. I now understand that you interpreted "CDN" as "external image hosting service" initially, that's where the confusion began. Just to clarify, when I said "CDN" I meant "CDN service Quartz site owner owns and uses for their own domain", it does not include "CDN services used by any external endpoints included by the site owner".

I do like your suggestion to expand this PR to support full URLs if user provides one, I think hosting social images elsewhere is also a valid use case. I pushed in a change that does a simple URL scheme regex check to see if the frontmatter link is a valid URL.

@Stephen-X Stephen-X force-pushed the dev/og-image-any-path branch from a228274 to 73fc1ce Compare November 20, 2024 03:29
@Stephen-X Stephen-X changed the title feat(open-graph): Allow custom OG images from content directory feat(open-graph): Allow custom OG images from any paths or URLs Nov 20, 2024
@saberzero1
Copy link
Collaborator

You know, I was thinking, several problems would be resolved by pushing the content of the static folder to the site root.

There are some reasons to not do this, like potentially conflicting files. (Same file in static and content.)

At the same time, I suspect if I add a static folder to the content folder, it would cause the same conflicts. Not sure how to deal with that at the moment.

Anyhow, above is not related to your PR. I'll review later today.

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