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

Improvement to social meta tags + an open graph image generator #263

Draft
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

inxilpro
Copy link

@inxilpro inxilpro commented Oct 14, 2022

This PR adds custom "open graph" meta tags and images for each docs page. By default it relies on the page title and the first sizable code block on the page, but I've added support for customization that can come later (thru separate PRs directly to the docs repo).

I extracted out a Page class to encapsulate some of the logic, and had to tweak the layout a bit to allow for overriding the metadata.

Here's a side-by-side comparison. Note that not only is the image preview different, but the text below the image is different as well.

Before After

Should this PR get merged, additional customizations can be applied by adding frontmatter data to the individual docs page.

For example, the default open graph data for the installation page in the docs will show a code block from the “Database and Migration” section, since that's the first code on the page that's larger than a line or two. It's possible to add a custom code block to the installation.md file that will be used when generating the open graph image. It's also possible to add a custom title, if it makes sense to add something more specific for the image and social cards.

@inxilpro inxilpro marked this pull request as ready for review October 18, 2022 15:06
@inxilpro inxilpro changed the title Open graph/metadata improvements (initial concept) Improvement to social meta tags + an open graph image generator Oct 18, 2022
@driesvints
Copy link
Member

I'm not sure we actually want to commit the images? Can we ignore them and generate them on deploy?

@inxilpro
Copy link
Author

I'm not sure we actually want to commit the images? Can we ignore them and generate them on deploy?

I would be OK with generating them on deploy. I thought this might be an easier starting point (since all the other images are committed to the repo, and generating them does come with some overhead). Realistically, we could commit this without any of the images now and then worry about that later, since the docs markdown files will all need to be updated before the images can be generated.

@inxilpro
Copy link
Author

@driesvints I just updated the PR:

  • Added a sample workflow for auto-generating open graph images via github actions. I'm not 100% sure how Laravel.com is deployed, so I'm not certain this is the right approach
  • Removed the sample images from this PR
  • Updated the code to use the first code block if a custom one isn't present

To generate open graph images, you need to:

chromedriver --port=4444 &
php artisan serve &
php artisan og:generate-images

This will push a bunch of images to public/img/og/

Let me know if you'd like me to do anything else to help push this over the finish line.

@inxilpro
Copy link
Author

Oh interesting. Once I got the github workflow working it auto-committed the images to the repo. Let me know what you think makes the most sense, and I can update accordingly.

@driesvints
Copy link
Member

Hah I have to admit that workflow is pretty neat. The only thing is that it seems to take a while (4 minutes). If you push a lot it would trigger quite a few long running builds. I think it's best that if we go forward with generating those images we only build on master branch. But honestly I still think we should maybe set up a scheduled job that runs once a day to generate these. Committing all of these to the repo would increase its size unnecessarily.

@inxilpro
Copy link
Author

@driesvints how is Laravel.com deployed? We probably don't need to even run it every day… we could just run it as a post-deploy script. The only issue with running the command on the Laravel.com server is making sure that it's set up to run chromedriver/etc.

Two other options:

  1. Push the images somewhere like S3 rather than into the git repo
  2. Pull this code into a separate repository and then pull the images from that repo on deploy

@driesvints
Copy link
Member

Through Forge. I don't think those two options make sense imo, sorry. Think it's best to either do a scheduled task or on deploy.

@taylorotwell what are your thoughts on this PR?

@inxilpro
Copy link
Author

To summarize, here are the two options that seem to make the most sense:

  1. Go with the PR as-is, where images are generated via Github Actions and committed directly to the repo. It's possible to mitigate the overhead here by both a) only generating on push to master, and b) saving a manifest file that only rebuilds images when the source docs have changed.
  2. Run php artisan og:generate-images on a schedule on the production server. This way, the images don't need to be committed to the repo. The downside is that then chromedriver needs to be installed and kept up-to-date on the production server, and the existing command would need to be updated to spawn the chromedriver instance in the background (much like Dusk).

I think both options are fine. Option 1 is much simpler, but if repository size is a concern, I'm willing to make option 2 work.

@inxilpro
Copy link
Author

@driesvints just FYI—by applying some optimizations to the workflow, I was able to get the run down to about a minute and a half. It no longer downloads every version of the docs, caches composer data, and only generates images for markdown files that have actually changed since the last run.

@taylorotwell taylorotwell marked this pull request as draft October 30, 2022 16:18
@inxilpro
Copy link
Author

inxilpro commented Nov 8, 2022

I see that @taylorotwell marked this as draft — let me know if there's anything you need from me on this.

@TheBlckbird
Copy link
Contributor

Is this still being worked on?

@inxilpro
Copy link
Author

inxilpro commented Aug 7, 2023

Is this still being worked on?

I'm just waiting on @driesvints or @taylorotwell to make a call. If there's renewed interest in this PR, I'm happy to make changes or get it caught up with the current codebase…

@taylorotwell taylorotwell marked this pull request as ready for review August 8, 2023 14:46
@taylorotwell taylorotwell marked this pull request as draft August 15, 2023 19:12
@shaedrich
Copy link

That would be quite nice to have

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.

4 participants