Skip to content

Conversation

@marshmallowrobot
Copy link
Collaborator

@marshmallowrobot marshmallowrobot commented Jan 10, 2023

Hey ya'll,

This PR introduces puppeteer to the code base. It is a headless browser that has a nifty screenshot function, which is used here to create images for the workbooks. All workbook-related functions have been moved to their own new module workbook_generators.js. It has two public-facing functions:

  • image.generate() (new!)
  • content.generate() (new fcn, pre-existing code moved from elsewhere)

When generate_learning_path_markdown.js is run, TWO new directories are created

  • workbooks (new!)
  • newsite (pre-existing, unchanged)

The workbooks folder is created by image.generate() function and contains all the workbook images together. Since it only contains images, it is copied to the website's /static/images/learn/* (instead of sharing /content/en/learn/* with contents of newsite).

Please let me know your questions of feedbacks, I'm happy to make changes. TY!

@marshmallowrobot marshmallowrobot requested a review from a team as a code owner January 10, 2023 21:19
@marshmallowrobot marshmallowrobot linked an issue Jan 10, 2023 that may be closed by this pull request
@lenucksi lenucksi requested a review from tsadler1988 January 11, 2023 11:46
@lenucksi
Copy link
Member

Great stuff and interesting to hear that there's a successor to the the PhantomJS's of the 2010s with puppeteer.
I can't review since my JS skills still are no good (Scala skills were better in PhantomJS' times) but thanks a lot for making this! 💯
Maybe something for @tsadler1988?

@rrrutledge
Copy link
Contributor

🎉for this work, @marshmallowrobot ❗️ I will take a look likely sometime next week.

@rrrutledge
Copy link
Contributor

OK so you're generating images on-the-fly when building the website? Cool! It's neat that you figured out not only how to make the images but how to make it easy for the next person to do the same.

I am thinking that it would be good to adjust so that the image is created once manually and then checked in to the repo. You can still check in the script for generating the image again if ever needed. Update /scripts/README.md with instructions of how to to do so.

Great work ‼️

@lenucksi
Copy link
Member

I am thinking that it would be good to adjust so that the image is created once manually and then checked in to the repo. You can still check in the script for generating the image again if ever needed. Update /scripts/README.md with instructions of how to to do so.

I would wonder how long it takes to regenerate the images in the GitHub action run?
If it's really short this could be a set & forget way of always having current images for everything.

No really firm opinion here though.

Great work bangbang

Indeed! 🎉

@rrrutledge
Copy link
Contributor

how long it takes to regenerate

Well, I wasn’t really thinking about the time involved but just having less moving parts during the build and then making it easy to see what the images look like when browsing the repo instead of needing to run a command in order to see what image will show up on the website.

@lenucksi
Copy link
Member

how long it takes to regenerate

Well, I wasn’t really thinking about the time involved but just having less moving parts during the build and then making it easy to see what the images look like when browsing the repo instead of needing to run a command in order to see what image will show up on the website.

Fair points. I guess just knowing how the things needs to be called and what it depends upon is then what's needed.

@marshmallowrobot
Copy link
Collaborator Author

I took a little time to chew on this feedback. I like the idea of reducing complexity during site generation. There's already enough going on with translations.

I think if we're going to go with static images, then I might kill off this PR entirely. It would be good enough to keep only the template HTML page. When the image needs to get updated, we could modify the HTML and take a literal screenshot of that page to create the static image we need.

I'll close this PR for now without merging and resubmit a new PR with just the template and static images.

@marshmallowrobot marshmallowrobot deleted the 375-workbook-images branch January 15, 2023 19:17
@rrrutledge
Copy link
Contributor

Oh - sorry that some of the work wasn’t used, but this approach does strike me as simpler to maintain. Thank you for updating the image generation to be just HTML where it can be done in a web browser instead of the old way, which was some kind of image creation tool.

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.

Image for workbooks on new site

5 participants