fix #2938: textWithLink -> link centered correctly

[vbinithyanandamv]

Updated the changes as per request from here

#2964

Let me know if need any more improvement.

[vbinithyanandamv]

Closing this PR as it is breaking the annotation test cases.

Edit: Fixed it based on the feedback.

[HackbrettXXX]

I think we should do this conditionally depending on the options.align and options.maxWidth properties.

[vbinithyanandamv]

Hey @HackbrettXXX I guess you are right, checked it based on align condition. But due to some reason build got interrupted and ci throws error. Can you do a re-run or help me out on that part.

[vbinithyanandamv]

center link.pdf

[HackbrettXXX]

I think we should fix it for "right" and "justify" as well. See https://github.com/MrRio/jsPDF/blob/master/src/jspdf.js#L3776

[vbinithyanandamv]

@HackbrettXXX Justify seems working fine, just updated with right. Not sure why build is failing though after i synced again with master. Can you just re-run it.

[vbinithyanandamv]

textlink align (2).pdf

[HackbrettXXX]

Thanks for incorporating the changes.

• Please add a test case. The second PDF looks good and would work well as test case: https://github.com/MrRio/jsPDF/blob/master/CONTRIBUTING.md#pull-requests
• For 'justify', I think we need a test with maxWidth set.
• Please run prettier before committing

[vbinithyanandamv]

Thanks for incorporating the changes.

• Please add a test case. The second PDF looks good and would work well as test case: https://github.com/MrRio/jsPDF/blob/master/CONTRIBUTING.md#pull-requests
• For 'justify', I think we need a test with maxWidth set.
• Please run prettier before committing

not sure about the maxWidth set. can you brief it a bit?

[vbinithyanandamv]

@HackbrettXXX need your help on this test case, using the same pdf output but its still throwing error. Am i missing anything?

[HackbrettXXX]

I think this is an encoding issue when saving or committing the PDF files. Happens on some machines and I haven't figured out why.

[HackbrettXXX]

not sure about the maxWidth set. can you brief it a bit?

I mean a test case with something like this:

doc.textWithLink('Justify aligned Link', (doc.internal.pageSize.getWidth() / 2), 30, { align: 'justify', url: 'https://www.google.com', maxWidth: 10 });

[vbinithyanandamv]

I think this is an encoding issue when saving or committing the PDF files. Happens on some machines and I haven't figured out why.

@HackbrettXXX any work around for this? or any suggestion on how to go for the test cases, in those situation?

[HackbrettXXX]

@HackbrettXXX any work around for this? or any suggestion on how to go for the test cases, in those situation?

In this case I can create the reference file and check it in. I will do so after you added a test case for "justify" with maxWidth.

[vbinithyanandamv]

@HackbrettXXX any work around for this? or any suggestion on how to go for the test cases, in those situation?

In this case I can create the reference file and check it in. I will do so after you added a test case for "justify" with maxWidth.

passing maxWidth not working on any alignment though. Is it a bug already? @HackbrettXXX

[HackbrettXXX]

Ah, ok, I just saw that the method was never designed to work with multiline strings. I guess then it would be fine if we don't support maxWidth for now. But implementing it should also be not difficult. We just need to consider the actual (multi-line) height of the text. We can get the height with getTextDimensions. Your call ;)