-
Notifications
You must be signed in to change notification settings - Fork 4
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
Pdfjs #774
Pdfjs #774
Conversation
Also notes where I found a particular fix for loading the PDF
Ready for review, addressed all of the comments, and made slight cleanups to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So psyched for this! A couple things to maybe change and a couple notes. I think the report changes are the main things and the others are dealer's choice. Sorry I didn't catch some of this before.
Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
Based on the existing accessibility code for checking all of the steps before now. Continues either if the PDF to compare couldn't be found (a developer error), or if the PDFs are different, since it doesn't affect interview progression. Also corrected a missed `scope.addToReport -> reports.addToReport` refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks for digging into this, it's a fantastic addition 🚀
Add to the changelog if you have a mo |
I added the ability to compare PDFs! It's currently one step,
I expect the baseline PDF "something-in-sources.pdf" and the new PDF "downloaded.pdf" to be the same
, but happy to add more, depending on what we think would be useful for folks.Making this a draft to make sure the API is as we want it.
In this PR, I have:
Links to any solved or related issues
Fix #25.
Any manual testing I have done to ensure my PR is working
Just added the automated tests.