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

Added feature to export as PDF #456

Closed

Conversation

billyLumberjack
Copy link

@billyLumberjack billyLumberjack commented May 4, 2020


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@CLAassistant
Copy link

CLAassistant commented May 4, 2020

CLA assistant check
All committers have signed the CLA.

@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label May 4, 2020
@ZeroX-DG
Copy link
Member

ZeroX-DG commented May 4, 2020

Thank you for the PR, can you give us some comparison on why you think jsPDF should be used instead of the native print to PDF feature of chromium?

@billyLumberjack
Copy link
Author

Hello , sorry for the poor level of information attached to the PR.
Honestly I am not aware of a native print to PDF method, are you referring to this?
https://www.w3schools.com/jsref/met_win_print.asp

At the moment jspdf to me looks the solution which better fits the already written code, giving also the support to get the "blob"

Thanks for your feedback,
I remain available for any improvement

@ZeroX-DG
Copy link
Member

ZeroX-DG commented May 5, 2020

Hmm, that's actually not a bad reason, this will have consistency for the whole app because printing may be different between web and electron version. I'll ask @Rokt33r for his thought on this

@Rokt33r
Copy link
Member

Rokt33r commented May 10, 2020

Great work! I'll review this tonight.

Copy link
Member

@Rokt33r Rokt33r left a comment

Choose a reason for hiding this comment

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

Exporting as PDF doesn't work well. It doesn't keep css style.

package.json Outdated Show resolved Hide resolved
@billyLumberjack
Copy link
Author

Exporting as PDF doesn't work well. It doesn't keep css style.

Thanks for the feedback, the pdf exported style is the same as the html one. So it should be corrected even this one right ?

@Rokt33r
Copy link
Member

Rokt33r commented Jun 6, 2020

@billyLumberjack
5fb9083
It just moves the type dep to dev deps. You need to install jspdf. @types/jspdf only has type defs not real code.

Jspdf seems to apply styles in the html. But styles for code fences and math blocks are from external css file. Please check those blocks are styled properly in a exported pdf file.

@ZeroX-DG ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Jun 8, 2020
@ZeroX-DG
Copy link
Member

ZeroX-DG commented Jul 7, 2020

@billyLumberjack can you fix the conflict please?

@ZeroX-DG
Copy link
Member

@billyLumberjack ping!

@Rokt33r
Copy link
Member

Rokt33r commented Jul 21, 2020

I'm going to take over this task

@billyLumberjack
Copy link
Author

@billyLumberjack
5fb9083
It just moves the type dep to dev deps. You need to install jspdf. @types/jspdf only has type defs not real code.

Jspdf seems to apply styles in the html. But styles for code fences and math blocks are from external css file. Please check those blocks are styled properly in a exported pdf file.

Hello, actually jspdf is not including the style, I am trying with html2canvas but getting document does not have window attached

@Rokt33r
Copy link
Member

Rokt33r commented Jul 22, 2020

@billyLumberjack I'm going to do this later. If you want to continue trying, please try it! I'll let you know when I can actually start working on this.

@mcmxcdev
Copy link

mcmxcdev commented Sep 10, 2020

Can you not simply use the electron native method for this:
https://www.electronjs.org/docs/api/web-contents#contentsprinttopdfoptions ?

Then you can avoid an extra 3rd party dependency.

@Rokt33r
Copy link
Member

Rokt33r commented Nov 3, 2020

This will be resolved in #652

@Rokt33r Rokt33r closed this Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants