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

[PRMP-1372] add pdfjs setup script - add pdfjs build to react app #514

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

Conversation

mark-start-nhs
Copy link
Contributor

add pdfjs setup script + custom_css to apply,
add iframe of pdfjs to react app,
add pdfjs to nginx.conf + remove some of the deprecated request permission-policies
update tests to point to the new iframe , <-- there needs to be a new test to check the iframe contents
update .gitignore to keep the pdfjs build but not the setup_pdfjs.py temp files,
add gulp-cli as a dev dependency used to compile pdfjs

Copy link
Contributor

@AndyFlintNHS AndyFlintNHS left a comment

Choose a reason for hiding this comment

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

Overall, it’s a really solid chunk of work I’m happy to go ahead with a few considerations:

  1. That we’re happy that the print/rotate buttons for the PDF are hidden in a menu
  2. That permitting 'unsafe-inline' is okay within the nginx.conf Content-Security-Policy
  3. I worry about maintainability, this approach means that we will have full customisability of our PDF viewer, at the cost of having to run Mark’s setup_pdfjs.py script to update versions and add the entire pdf.js code into our codebase.

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.

2 participants