-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
getTextDimensions should automatically infer numberOfLines if maxTextWidth is provided #2824
getTextDimensions should automatically infer numberOfLines if maxTextWidth is provided #2824
Conversation
…ically be inferred based on that and the text width
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.
Thanks for the PR.
The getTextDimensions
method should behave like the text
method:
- The option should be named
maxWidth
instead ofmaxTextLength
- It should also work if an array is passed. Maybe we should simply call
splitTextToSize
and calculate the text dimensions on the result. See jspdf.js#L3541
…with the naming for text
Good idea to make it consistent with the text method! I've changed the name of the configuration. |
Sorry long time without checking this library. one question ... if I pass a single (and large line) and maxWidth, How can I get the resulting number of lines? And the change in the height, to adjust the position of next text block. Thx in advance |
Currently, if an array is passed, the I'm getting more and more convinced, that we don't actually need this functionality, since the user can simply call |
@themacboy |
@HackbrettXXX, you're right! The code for determining number of lines should be moved outside of the for loop determining the width. That's a bug. |
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.
Ok, you convinced me. maxWidth
is a nice convenience feature.
src/modules/cell.js
Outdated
@@ -192,15 +192,18 @@ | |||
); | |||
} | |||
|
|||
text = Array.isArray(text) ? text : [text]; | |||
const maxWidthIsDefind = options.maxWidth !== undefined && options.maxWidth !== null | |||
text = Array.isArray(text) ? text : maxWidthIsDefind ? this.splitTextToSize(text, options.maxWidth) : [text]; |
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.
This behaves now slightly different than in the text
method if an array is passed. We could discuss what's best here, but for the moment it should behave the same. See jspdf.js#L3527.
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.
@HackbrettXXX I just pushed a new commit which should make it behave exactly the same as in the text
method
Thanks for the PR and for incorporating the changes I suggested :) |
Extend getTextDimensions such that a maxTextWidth can optionally be passed along and if it is, it is used to infer the number of lines