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

[PDF] Automatic PDF Generation #225

Merged
merged 10 commits into from
Dec 23, 2022
Merged

[PDF] Automatic PDF Generation #225

merged 10 commits into from
Dec 23, 2022

Conversation

seshrs
Copy link
Member

@seshrs seshrs commented Nov 29, 2022

Context

Closes #136! 🚀
As mentioned in #136, I've seen Piazza questions with students asking how to download the spec as a PDF (either because they don't know about print-to-PDF, or because they assume that the PDF won't look good). Hence, let's make the PDF version of the page easily available!

This turned out to be a complex feature, and was only recently made possible by GitHub Pages + GitHub Actions + GitHub Support.

NOTE: "Download as PDF" feature only works when site is built with build-primer-spec-action

If you're a course instructor, and your course website does not yet use build-primer-spec-action, your website's pages will not show the "Download as PDF" button.

Follow the instructions in that action's README to set up GitHub Pages deployment via the action!

Validation

Visit one of the pages in the PR preview website. If you click the "download" button, your browser will download the PDF version of the page! Example page: https://preview.sesh.rs/previews/eecs485staff/primer-spec/225/docs/USAGE_ADVANCED.html

Screen Shot 2022-11-28 at 11 20 46 PM

Known issue: Some of the pages are not rendered correctly. I'm pretty sure this is related to my repo setup and will not happen on real spec websites, but I'll need to keep an eye out for this after this PR merges.

@seshrs seshrs added enhancement New feature or request semver/minor Pull Request proposes "minor" change labels Nov 29, 2022
@seshrs seshrs added this to the WN 2023 milestone Nov 29, 2022
@github-actions github-actions bot changed the base branch from main to develop November 29, 2022 06:11
@github-actions
Copy link
Contributor

Thanks for opening this pull request! 🙏

I've changed this PR's base branch to develop. A maintainer will review your PR soon!

@github-actions
Copy link
Contributor

github-actions bot commented Nov 29, 2022

The spec from this PR is available at https://preview.sesh.rs/previews/eecs485staff/primer-spec/225/.

(Available until Thu Jan 19 2023.)

@seshrs
Copy link
Member Author

seshrs commented Nov 29, 2022

Ugh, we have a circular dependency between PDF-generation and PR preview uploads — the PDF generation step requires that the CSS assets be available at the PR Preview URL 😖

The issue is transient, only affects this repo (Primer Spec aka me), and will fix itself when the PR Preview action re-runs. I'm not going to fix this for now, but will track this in #226.


EDIT 1: Oops, this is a bit more complicated, there's some other issue that's causing the generated PDFs to be blank pages.


EDIT 2: Figured it out, it was a CORS issue.

By default, Same Origin Policy (SOP) applies, preventing resources from preview.sesh.rs from being accessed by other websites.

I updated the Primer Spec Preview server to add a header allowing all CORS origins to access files in the preview directory (seshrs/primer-spec-preview@63b41b0). Now, when GitHub Actions runs Chromedriver to generate the PDFs, the JS from the preview website loads correctly!

(CC @awdeorio, you might find this interesting. Maybe this might make for an interesting addition to the HTTP lecture?)


EDIT 3: Ugh, looks like some pages generate PDFs correctly and some don't. I wonder why it's non-deterministic...

@seshrs seshrs marked this pull request as ready for review December 20, 2022 03:43
@seshrs seshrs merged commit d890268 into develop Dec 23, 2022
@seshrs seshrs deleted the pdf branch December 23, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver/minor Pull Request proposes "minor" change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Download as PDF
1 participant