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

Download pdf feature #67

Merged
merged 3 commits into from
May 16, 2019
Merged

Download pdf feature #67

merged 3 commits into from
May 16, 2019

Conversation

pimotte
Copy link
Contributor

@pimotte pimotte commented May 15, 2019

Note that this is untested in the e2e test. This is due to the fact that we use a library to generate it, which means we can't really hook into the process in a solid way, and cypress not supporting this yet (see cypress-io/cypress#949)

Fixes #59

@pimotte pimotte requested review from bkaptijn, DennisHouterman and aozdmr and removed request for bkaptijn May 15, 2019 09:48
Copy link
Contributor

@DennisHouterman DennisHouterman left a comment

Choose a reason for hiding this comment

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

The pdf feature seems to be well implemented. However, exporting a pdf from an image may make it harder to display text while hovering on an element of the proof, which was brought up during a product meeting.

@pimotte
Copy link
Contributor Author

pimotte commented May 16, 2019

As far as I know, https://www.npmjs.com/package/canvas2pdf is the only candidate to render a pdf from a canvas in a way that is not just embedding the picture. No project has a dependency on this module, which made me hesitant to use it. If tooltips are indeed a needed feature at some point, we can switch to this and see if it works

@pimotte pimotte merged commit 277e50c into master May 16, 2019
@pimotte pimotte deleted the feature/download-button branch May 16, 2019 09:16
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.

As End user I want download to pdf ...
2 participants