-
Notifications
You must be signed in to change notification settings - Fork 104
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
Support vertical-align and alignment-baseline #114
Conversation
src/svg2pdf.js
Outdated
var textRenderingMode = getTextRenderingMode(attributeState); | ||
_pdf.text( | ||
transformedText, | ||
textX + dx - xOffset, | ||
textY + dy, | ||
{ | ||
baseline: verticalAlign, |
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 will fail when other properties as the alignment-baseline are defined through the vertical-align shorthand. We probably should only save the alignment-baseline part in AttributeState and parse the vertical-align property with a RegExp.
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.
In jspdf.debug.js at line 3794, it seems to cause no problems if the value of the baseline option is not supported - jspdf just does nothing and jumps into the default case assuming there is nothing to do. Or didn't I get that right?
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.
Yeah that's right, but if vertical-align is something like first sub middle
, that would fall into jspdf's default case, although it supports middle
.
src/svg2pdf.js
Outdated
var textRenderingMode = getTextRenderingMode(attributeStates[i]); | ||
_pdf.text(this.texts[i], xs[i] - textOffset, ys[i], {angle: transform, renderingMode: textRenderingMode === "fill" ? void 0 : textRenderingMode}); | ||
_pdf.text(this.texts[i], xs[i] - textOffset, ys[i], { | ||
baseline: verticalAlign, |
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.
same as below
tests/text-placement/spec.svg
Outdated
<text alignment-baseline="hanging" x="60" y="10">A hanging</text> | ||
<text alignment-baseline="middle" x="60" y="65">A middle</text> | ||
<text alignment-baseline="baseline" x="60" y="110">A baseline</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.
We could add tests for other possible values
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.
We should test all possible values, even if jspdf does not support them:
baseline | text-bottom | alphabetic | ideographic | middle | central | mathematical | text-top | bottom | center | top
and also test the same if we use the vertical-align shorthand instead.
Maybe it's a good idea to move it to a separate test case, as well.
src/svg2pdf.js
Outdated
var alignmentBaseline = getAttribute(node, "vertical-align") || getAttribute(node, "alignment-baseline"); | ||
if (alignmentBaseline) { | ||
var matchArr = alignmentBaseline.match(/(baseline|text-bottom|alphabetic|ideographic|middle|central|mathematical|text-top|bottom|center|top|hanging)/); | ||
if (isArray(matchArr)) { |
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.
or simply if (match)
. Where is isArray
defined? Usually, it's Array.isArray
.
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.
That's right, I messed this up.
src/svg2pdf.js
Outdated
@@ -1888,8 +1888,32 @@ SOFTWARE. | |||
var alignmentBaseline = getAttribute(node, "vertical-align") || getAttribute(node, "alignment-baseline"); | |||
if (alignmentBaseline) { | |||
var matchArr = alignmentBaseline.match(/(baseline|text-bottom|alphabetic|ideographic|middle|central|mathematical|text-top|bottom|center|top|hanging)/); | |||
if (matchArr) { | |||
attributeState.alignmentBaseline = matchArr[0]; | |||
switch (matchArr) { |
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.
Don't save the "mapped" property in the attributeState
(it is meant to mirror the SVG property state). Instead, use the mapped properties only for the pdf.text(...)
call. Instead of the switch you could also use an object:
{
"text-bottom": "bottom",
...
}
Closes issue #109