Skip to content

lineHeightFactor is ignored if specified as a text option #3234

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

Closed
novsstation opened this issue Aug 19, 2021 · 4 comments · Fixed by #3283
Closed

lineHeightFactor is ignored if specified as a text option #3234

novsstation opened this issue Aug 19, 2021 · 4 comments · Fixed by #3283

Comments

@novsstation
Copy link

Currently lineHeightFactor is ignored, if it is specified as text option.
It correctly checked for calculating lineHeight variable.
But this variable is not used for calculating descent value (still used global lineHeightFactor from document).
As result, those two options produces the different PDF files

https://github.com/MrRio/jsPDF/blob/cef97fb34eda41a8704c9f3983e680919a328ce4/src/jspdf.js#L3612

Steps to reproduce:

  1. Open the sample https://codepen.io/SNovikov/pen/VwbJOzG?editors=0010
  2. Export PDF with specified document option
   doc.setLineHeightFactor(1.5);
   doc.text('Some text', 10, 10, { baseline: 'middle' });
  1. Export PDF with specified text option
   doc.text('Some text', 10, 10, { lineHeightFactor: 1.5, baseline: 'middle' });

As you can see, PDF files are different
image

@HackbrettXXX
Copy link
Collaborator

Thanks for reporting this. Could you prepare a pull request?

@Megharth
Copy link
Contributor

Megharth commented Oct 2, 2021

I have added a test that checks output of pdf generated using setLineHeightFactor() method and one where we pass lineHeightFactor as an option to the text() method. The tests are passing locally by running npm run test-unit and npm run test-local but the test that I included is failing on saucelab. I tried debugging it, but locally it provides the expected results. I have no idea how to debug on saucelab. If anyone can guide me, I am happy to resolve the issue with failing tests on saucelab.

@Megharth
Copy link
Contributor

Megharth commented Oct 2, 2021

I saw the karma.conf.js for saucelab and it was taking the jspdf from the dist folder. I created a new build and now all the tests passed. I saw in contribution guide that we should not update files in the dist folder. So, how should I go about this?

@HackbrettXXX
Copy link
Collaborator

On CI, the files in dist are built before running the tests. For local testing you can npm run build && npm run test-local. We could/should probably either improve the contribution guide or add a pretest-local npm script similar to pretest.

@HackbrettXXX HackbrettXXX linked a pull request Oct 6, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants