-
Notifications
You must be signed in to change notification settings - Fork 857
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
[Bug]: textWrap is not calculating properly text size when using custom fonts / size (Bootstrap repro) #2295
Comments
Now you must explicitly set all font-related attributes in the Now it is probably a good time to use the |
@kumilingus is this going to make it in the next patch release (3.7.6?) or will it land in 4.0.0? If the later, is there any workaround that I can apply? |
Sorry, haven't made the decision yet. A workaround is to re-define the class MyRect extends joint.dia.Element {
static attributes = {
textWrap: {
qualify: joint.util.isPlainObject,
set: function(value, refBBox, node, attrs) {
const computedFontSize = parseFloat(getComputedStyle(node).fontSize);
const computedAttrs = { ...attrs };
computedAttrs.fontSize = computedFontSize;
joint.dia.attributes.textWrap.set.call(this, value, refBBox, node, computedAttrs);
}
}
};
} Here's updated version of your codesandbox: |
The break text depends not only on the font-size, but also on font-family, font-weight, letter-spacing. static attributes = {
textWrap: {
qualify: joint.util.isPlainObject,
set: function (value, refBBox, node, attrs) {
const computedStyles = getComputedStyle(node);
const fontSize = parseFloat(computedStyles.fontSize);
const fontFamily = computedStyles.fontFamily;
const computedAttrs = { ...attrs, fontSize, fontFamily };
joint.dia.attributes.textWrap.set.call(
this,
value,
refBBox,
node,
computedAttrs
);
}
}
}; Updated example: https://codesandbox.io/s/textwrap-bug-forked-svgjq9?file=/src/index.js |
Oh, I see 🤔. I was about to ask why / how the different attributes are handled, then I remembered I can see the code 😂. I had a look at the
I think the following extra attributes should be taken into account:
|
I think that only IMHO Note that there is no |
Fix waits for #2335. For the time being please use the workaround. It's currently not guaranteed that the |
Another workaround that I've used in our project for a similar problem (body style, including font family/size/weight affected the SVG text but did not get taken into account when calculating wrapping) is to set the text attributes explicitly with references to CSS variables. For me this solved the text wrapping without being forced to duplicate values between the CSS file and the TypeScript file containing the shape definition. E.g.
Recent versions of Bootstrap already define CSS vars, so you might be able to use them as-is. |
Nice, I didn't know this is even possible: https://jsfiddle.net/kumilingus/qc9geah6/ :root {
--rect-color: red;
} <svg>
<rect width="100" height="100" fill="var(--rect-color)" />
</svg> |
I thought this was going to land in 4.0 since it's a breaking change 🤔 |
The necessary breaking change was made and the issue can be fixed with a patch now ( |
Hi @alexandernst, the fix was released as part of JointJS v4.0.2 (JointJS+ v4.0.1). |
Thank you @zbynekstara !! |
After this change: For our custom shapes the export default class CustomElement extends dia.Element {
preinitialize() {
super.preinitialize(...arguments);
// etc.
}
// etc.
defaults() {
return {
type: 'xyz',
size: { width: 200, height 60 },
attrs: {
// etc.
label: {
textAnchor: 'start',
text: '',
textWrap: {
width: 160,
ellipsis: true,
maxLineCount: 3
},
x: 20,
fontSize: 'var(--test-font-size, 14px)',
fontFamily: 'var(--test-font-family, sans-serif)',
fill: 'var(--test-fill-color, #333)'
}
}
};
}
// etc.
}
This was working flawlessly before this change. I will write an override to take those values from the Thanks. EDIT: forgot to mention, you can test this in your example as well, simply change the font-size to e.g. 20. No bueno. |
Hi @frnora , that was already fixed in https://github.com/clientIO/joint/pull/2679/files I believe. |
Hi @kumilingus, thank you, that's great to hear! Sadly, it's not part of the latest joint+ dist, thus I can't use it. (must rely on that file) I will keep in mind to remove the override once a new version of joint+ hits. |
What happened?
It seems that textWrap isn't working as expected when using css styling. A simple repro: https://codesandbox.io/s/textwrap-bug-76f2md?file=/src/index.js
Note that commenting the bootstrap import fixes the issue.
Version
3.7.5
What browsers are you seeing the problem on?
Firefox, Chrome, Safari
What operating system are you seeing the problem on?
Mac
The text was updated successfully, but these errors were encountered: