jsr
jsr copied to clipboard
feat: Add OGP to package page
If you merge this PR, you can show package thumbnail in SNS such as X and Discord.
Example thumbnail (@std/path):
I think the image needs a background color; otherwise, it's pretty hard to read in dark mode.
This is great work overall, and I'm happy to see this. As far as design, however, I want to be sure we're very thorough.
Most importantly, I'd like to see tests across a wide range of packages. What happens when the package namespace and title are wider than the image, for example? Will the version number and "latest" badge wrap to a new line? If they do, will it push other content down? Are there any scenarios where text could overlap?
Same for the description; how does it flow when a package has very long text? Does it still look right when the package doesn't work with as many things, or has a different score or publish date?
Then, aesthetically, there are a few nitpicks I think we could tighten up before this is fully ready (see the below image to help illustrate):
- The spacing around the edges of the image are inconsistent (left and bottom spacing are the same, top is a little bigger, and right is much bigger)
- The package name, version, and "latest" badge don't quite line up visually. Ideally, I'd like to see the bottom baseline of the text align with the bottom of the pill shape of the badge.
- That "latest" badge has a lot more padding on top than on bottom. Ideally, the text should be vertically centered inside it.
- All the text on the left edge should align (either bring everything to the left to line up with the
@, or move the package name to the right to line up with everything else) - Can we space the three stat columns (Published/JSR Score/Works with) a little more evenly? Right now there's a lot of unused space between the first and second columns
- I think all three columns should be left-aligned. Right now, it looks like "Published" and "JSR Score" are center-aligned, and "Works-with" is right-aligned, and I think just making them all left-aligned would be better for consistency.
- Should the "4 days ago" text be center-aligned with the score and logos, instead of having them all top-aligned? I could go either way on this one.
I don't know exactly how easy or difficult all of this is given that we're just rendering an image on the fly, but I think the more of these we can address, the better the final outcome will be. :)
@josh-collinsworth, thank you for your advice! I worry about OGP design, so your advice is very helpful.
I'll improve it, following your advice.
@josh-collinsworth, I fixed some.
Could you tell me what do you think?:
Max name (scope: 20 chars, name: 32 chars)
@nakasyou That's looking much better.
Question: how can I see these generated images on the site? The social share preview seems to still be the default image when I'm attempting to see the preview locally.
@josh-collinsworth
That's looking much better.
Thank you!
Question: how can I see these generated images on the site? The social share preview seems to still be the default image when I'm attempting to see the preview locally.
OGP was in conflict with the default of app.tsx, so I fixed it.
Also, OGP URL has to be absolute URL. Should I add code that switching to jsr.test when Development mode?
Also, OGP URL has to be absolute URL. Should I add code that switching to jsr.test when Development mode?
Yes, that'd be great - if you grep for API_ROOT you can see how to wire through an env var that contains the domain being served. To inject the env var in production, you can add "FRONTEND_ROOT" = "https://${var.domain_name}" in frontend_envs in cloud_run_frontend.tf. If you need help with this, let me know and I can do it :)
You can then default the frontend root to http://jsr.test if no env var is passed (like we do for API_ROOT).
Hi @lucacasonato, thank you for advice. OGP URL has been changed to the code that uses FRONTEND_ROOT.
To inject the env var in production, you can add "FRONTEND_ROOT" = "https://${var.domain_name}" in frontend_envs in cloud_run_frontend.tf. If you need help with this, let me know and I can do it :)
It's difficult for me to do it. And even if I could do it, I may destroy environment. So could you do it, please?
Sure!
One more change I'd like to see: it seems how descriptions are truncated a little too aggressively now. Even packages with very short descriptions are getting cut off.
Could we at least allow the description to wrap to two lines or so? Maybe even move the three columns down a little if we need to?
@josh-collinsworth, sorry for late.
Could you tell me what about you think?
@nakasyou This is looking really good! Thanks for all your work on this! I have two remaining nitpicks, but I think only one is immediately important.
First, and more importantly: can we prevent text from wrapping mid-word in the description? This isn't great for legibility, and it could also cause some unwanted "words" to appear where they shouldn't. (e.g., A package to create a beautiful butt on) 🙈
And second, less importantly: when the package title wraps to a new line, is it possible to keep the version aligned?
I'm much less worried about the second item; I think that can be an edge case we clean up later. But the first is bound to affect quite a few packages, and potentially cause confusion or unwanted appearance.
Hi @josh-collinsworth, thank you for reviewing.
I resolved first problem, I'm working to resolve the second problem.
@josh-collinsworth, I fixed second problem, that was a bug.
@nakasyou Looks like CI is picking up some formatting issues. deno fmt should fix them.
@josh-collinsworth
Love this and appreciate the work you've put into it. I think this can and should be shipped. 👍
Thank you! I'm glad.
One minor note: it looks like there may be an issue with non-English glyphs. Still, I think we should ship this as-is and file that for future improvements.
I think it is font and line-breaking problem, i see.
deno fmtshould fix them.
I run deno fmt. Please re-run workflow for it.
@nakasyou Sorry, looks like there's still one more CI issue:
@josh-collinsworth I'm sorry that I missed checking well, I fixed it.