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: Add OGP to package page #417

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

nakasyou
Copy link

If you merge this PR, you can show package thumbnail in SNS such as X and Discord.

Example thumbnail (@std/path):
IMG_2908

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

Cool! @donjo or @josh-collinsworth could you review the design of this OG image? The generation code looks good.

@josh-collinsworth
Copy link
Contributor

I think the image needs a background color; otherwise, it's pretty hard to read in dark mode.
image

@josh-collinsworth
Copy link
Contributor

josh-collinsworth commented Apr 22, 2024

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. :)

og-alignment

@nakasyou
Copy link
Author

@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.

@nakasyou
Copy link
Author

@josh-collinsworth, I fixed some.
Could you tell me what do you think?:
IMG_2917

Max name (scope: 20 chars, name: 32 chars)
IMG_2918

@josh-collinsworth
Copy link
Contributor

@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.

@nakasyou
Copy link
Author

nakasyou commented Apr 27, 2024

@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?

@lucacasonato
Copy link
Member

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).

@nakasyou
Copy link
Author

nakasyou commented May 1, 2024

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?

@lucacasonato
Copy link
Member

Sure!

@josh-collinsworth
Copy link
Contributor

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?

@nakasyou
Copy link
Author

@josh-collinsworth, sorry for late.
Could you tell me what about you think?
IMG_2992

@josh-collinsworth
Copy link
Contributor

josh-collinsworth commented May 15, 2024

@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) 🙈

CleanShot 2024-05-15 at 09 30 01

And second, less importantly: when the package title wraps to a new line, is it possible to keep the version aligned?

CleanShot 2024-05-15 at 09 27 58

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.

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.

None yet

3 participants