Skip to content
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

Fix inaccurate text size calculations #2505

Merged
merged 8 commits into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pretty-moles-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"victory-core": patch
---

Improve accuracy of text size measurements (fixes #2475)
2 changes: 1 addition & 1 deletion .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const STORIES = path.resolve(ROOT, "stories");
module.exports = {
webpackFinal: async (config) => {
// Read all the victory packages and alias.
glob.sync(path.join(PKGS, "victory*/package.json")).forEach((pkgPath) => {
glob.sync(path.join(PKGS, "victory*/package.json").replaceAll("\\", "/")).forEach((pkgPath) => {
const key = path.dirname(path.relative(PKGS, pkgPath));
config.resolve.alias[key] = path.resolve(path.dirname(pkgPath));
});
Expand Down
45 changes: 24 additions & 21 deletions packages/victory-core/src/victory-util/textsize.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { TextSize } from "victory-core";

const approximate = (text, style?) =>
TextSize._approximateTextSizeInternal.impl(text, style, true);

const testString = "ABC";

describe("victory-util/textsize", () => {
Expand All @@ -17,64 +20,64 @@ describe("victory-util/textsize", () => {

describe("approximateWidth", () => {
it("return zero width when no style", () => {
expect(TextSize.approximateTextSize(testString).width).toEqual(0);
expect(approximate(testString).width).toEqual(0);
});
it("return correct width with signed angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
angle: -45,
fontSize: 14,
}).width.toFixed(2),
).toEqual("31.71");
});
it("return correct width with pixel fontsize", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: "14px",
}).width.toFixed(2),
).toEqual("28.74");
});
it("return appropriate width with defined fontSize", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
}).width.toFixed(2),
).toEqual("24.64");
});
it("consider font", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 16,
}).width.toFixed(2),
).toEqual("32.85");
});
it("consider letterSpacing", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
letterSpacing: "1px",
}).width.toFixed(2),
).toEqual("26.64");
});
it("consider angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
angle: 30,
}).width.toFixed(2),
).toEqual("28.24");
});
it("not consider lineHeight without angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
lineHeight: 2,
}).width.toFixed(2),
).toEqual("24.64");
});
it("consider lineHeight with angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
lineHeight: 2,
angle: 30,
Expand All @@ -83,15 +86,15 @@ describe("victory-util/textsize", () => {
});
it("return width of widest string in text", () => {
expect(
TextSize.approximateTextSize("ABC\nDEFGH\nIJK", {
approximate("ABC\nDEFGH\nIJK", {
fontSize: 12,
}).width.toFixed(2),
).toEqual("41.94");
});

it("returns width of widest string in array if array has an empty string", () => {
expect(
TextSize.approximateTextSize(["06-14-20", ""], {
approximate(["06-14-20", ""], {
fontSize: 12,
}).width.toFixed(2),
).toEqual("47.93");
Expand All @@ -100,56 +103,56 @@ describe("victory-util/textsize", () => {

describe("approximateHeight", () => {
it("return zero width when no style", () => {
expect(TextSize.approximateTextSize(testString).height).toEqual(0);
expect(approximate(testString).height).toEqual(0);
});
it("return correct height with signed angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
angle: -45,
fontSize: 14,
}).height.toFixed(2),
).toEqual("33.29");
});
it("return correct height with pixel fontsize", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: "14px",
}).height.toFixed(2),
).toEqual("16.90");
});
it("return appropriate height with expected precision", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
}).height.toFixed(2),
).toEqual("14.49");
});
it("consider font", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 16,
}).height.toFixed(2),
).toEqual("19.32");
});
it("consider angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
angle: 30,
}).height.toFixed(2),
).toEqual("25.48");
});
it("not consider letterSpacing without angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
letterSpacing: "1px",
}).height.toFixed(2),
).toEqual("14.49");
});
it("consider letterSpacing with angle", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
angle: 30,
letterSpacing: "1px",
Expand All @@ -158,15 +161,15 @@ describe("victory-util/textsize", () => {
});
it("consider lineHeight", () => {
expect(
TextSize.approximateTextSize(testString, {
approximate(testString, {
fontSize: 12,
lineHeight: 2,
}).height.toFixed(2),
).toEqual("28.98");
});
it("consider multiLines text", () => {
expect(
TextSize.approximateTextSize(`ABC\n${"DBCDEFG"}\n123`, {
approximate(`ABC\n${"DBCDEFG"}\n123`, {
fontSize: 12,
}).height.toFixed(2),
).toEqual("43.47");
Expand Down
125 changes: 107 additions & 18 deletions packages/victory-core/src/victory-util/textsize.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// http://www.pearsonified.com/2012/01/characters-per-line.php
/* eslint-disable no-magic-numbers */
import { assign, defaults } from "lodash";
import { assign, defaults, memoize } from "lodash";

// Based on measuring specific character widths
// as in the following example https://bl.ocks.org/tophtucker/62f93a4658387bb61e4510c37e2e97cf
Expand Down Expand Up @@ -243,9 +243,97 @@ const _approximateTextHeightInternal = (text: string | string[], style) => {
}, 0);
};

const _approximateDimensionsInternal = (
text: string | string[],
style?: TextSizeStyleInterface,
) => {
const angle = Array.isArray(style)
? style[0] && style[0].angle
: style && style.angle;
const height = _approximateTextHeightInternal(text, style);
const width = _approximateTextWidthInternal(text, style);
const widthWithRotate = angle
? _getSizeWithRotate(width, height, angle)
: width;
const heightWithRotate = angle
? _getSizeWithRotate(height, width, angle)
: height;
return {
width: widthWithRotate,
height: heightWithRotate * coefficients.heightOverlapCoef,
};
};

const _getMeasurementContainer = memoize(() => {
const element = document.createElementNS("http://www.w3.org/2000/svg", "svg");
element.setAttribute("xlink", "http://www.w3.org/1999/xlink");

const containerElement = document.createElementNS(
"http://www.w3.org/2000/svg",
"text",
);
element.appendChild(containerElement);

element.style.position = "fixed";
element.style.top = "-9999px";
element.style.left = "-9999px";

document.body.appendChild(element);

return containerElement;
});

const styleToKeyComponent = (style) => {
if (!style) {
return "null";
}

return `${style.angle}:${style.fontFamily}:${style.fontSize}:${style.letterSpacing}:${style.lineHeight}`;
};

const _measureDimensionsInternal = memoize(
(text: string | string[], style?: TextSizeStyleInterface) => {
const containerElement = _getMeasurementContainer();

const lines = _splitToLines(text);
let heightAcc = 0;
for (const [i, line] of lines.entries()) {
const textElement = document.createElementNS(
"http://www.w3.org/2000/svg",
"tspan",
);
const params = _prepareParams(style, i);
textElement.style.fontFamily = params.fontFamily;
textElement.style.transform = `rotate(${params.angle})`;
textElement.style.fontSize = `${params.fontSize}px`;
textElement.style.lineHeight = params.lineHeight;
textElement.style.fontFamily = params.fontFamily;
textElement.style.letterSpacing = params.letterSpacing;
textElement.textContent = line;
textElement.setAttribute("x", "0");
textElement.setAttribute("y", `${heightAcc}`);
heightAcc += params.lineHeight * params.fontSize;

containerElement.appendChild(textElement);
}

const { width, height } = containerElement.getBoundingClientRect();

containerElement.innerHTML = "";

return { width, height };
},
(text, style) => {
const totalText = Array.isArray(text) ? text.join() : text;
const totalStyle = Array.isArray(style)
? style.map(styleToKeyComponent).join()
: styleToKeyComponent(style);
return `${totalText}::${totalStyle}`;
},
);

export interface TextSizeStyleInterface {
angle?: number;
characterConstant?: string;
fontFamily?: string;
fontSize?: number | string;
letterSpacing?: string;
Expand All @@ -254,22 +342,23 @@ export interface TextSizeStyleInterface {

// Stubbable implementation.
export const _approximateTextSizeInternal = {
impl: (text: string | string[], style?: TextSizeStyleInterface) => {
const angle = Array.isArray(style)
? style[0] && style[0].angle
: style && style.angle;
const height = _approximateTextHeightInternal(text, style);
const width = _approximateTextWidthInternal(text, style);
const widthWithRotate = angle
? _getSizeWithRotate(width, height, angle)
: width;
const heightWithRotate = angle
? _getSizeWithRotate(height, width, angle)
: height;
return {
width: widthWithRotate,
height: heightWithRotate * coefficients.heightOverlapCoef,
};
impl: (
text: string | string[],
style?: TextSizeStyleInterface,
__debugForceApproximate = false,
) => {
// Attempt to first measure the element in DOM. If there is no DOM, fallback
// to the less accurate approximation algorithm.
const isClient =
typeof window !== "undefined" &&
typeof window.document !== "undefined" &&
typeof window.document.createElement !== "undefined";

if (!isClient || __debugForceApproximate) {
return _approximateDimensionsInternal(text, style);
}

return _measureDimensionsInternal(text, style);
},
};

Expand Down
7 changes: 3 additions & 4 deletions packages/victory-vendor/.babelrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ module.exports = {
/^node_modules/,
"lib-vendor",
);
const relPathToPkg = path.relative(
path.dirname(currentFileVendor),
vendorPkg,
).replaceAll("\\", "/");
const relPathToPkg = path
.relative(path.dirname(currentFileVendor), vendorPkg)
.replaceAll("\\", "/");

return relPathToPkg;
}
Expand Down
24 changes: 24 additions & 0 deletions stories/victory-label.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,30 @@ export const LineHeight = () => {
/>
}
/>
<VictoryScatter
{...defaultScatterProps}
labelComponent={
<VictoryLabel
lineHeight={[2, 1, 3]}
text={["测试汉字", "不在正常的 ASCII 范围内", "最后一行"]}
backgroundStyle={[{ stroke: "blue", fill: "none" }]}
/>
}
/>
<VictoryScatter
{...defaultScatterProps}
labelComponent={
<VictoryLabel
lineHeight={[2, 1, 3]}
text={[
"اختبار اللغات التي تُقرأ من اليمين إلى اليسار",
"مثل العربية",
"هناك أكثر من ذلك بكثير",
scottrippey marked this conversation as resolved.
Show resolved Hide resolved
]}
backgroundStyle={[{ stroke: "blue", fill: "none" }]}
/>
}
/>
</div>
);
};
Expand Down