Fixed wrong tailored text generation in getTailoredTextOrDefault (mic…

…rosoft#119) * Fixed wrong tailored text generation in getTailoredTextOrDefault * skip if length of the text is lower then tail * Enabled all tests * Changed way to identify if string is longer than ellipses * Fixed condition * Extended tests
visualbis · Feb 14, 2024 · 4929cd3 · 4929cd3
1 parent d2b7e4e
commit 4929cd3
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 6.1.0
+* Fixed wrong tailored text generation in getTailoredTextOrDefault
+
 ## 6.0.3
 * Update powerbi-visuals-utils-testutils to 6.0.3
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "powerbi-visuals-utils-formattingutils",
-  "version": "6.0.3",
+  "version": "6.1.0",
   "description": "FormattingUtils",
   "main": "lib/src/index.js",
   "module": "lib//src/index.js",

diff --git a/src/textMeasurementService.ts b/src/textMeasurementService.ts
@@ -273,17 +273,20 @@ export function getTailoredTextOrDefault(textProperties: TextProperties, maxWidt
     ensureDOM();
 
     const strLength: number = textProperties.text.length;
-
     if (strLength === 0) {
         return textProperties.text;
     }
 
     let width: number = measureSvgTextWidth(textProperties);
-
     if (width < maxWidth) {
         return textProperties.text;
     }
 
+    const ellipsesWidth: number = measureSvgTextWidth(textProperties, ellipsis);
+    if (ellipsesWidth >= width) {
+        return textProperties.text;
+    }
+
     // Create a copy of the textProperties so we don't modify the one that's passed in.
     const copiedTextProperties = Prototype.inherit(textProperties);
 
@@ -297,7 +300,7 @@ export function getTailoredTextOrDefault(textProperties: TextProperties, maxWidt
     let i = ellipsis.length;
 
     while (min <= max) {
-        // num | 0 prefered to Math.floor(num) for performance benefits
+        // num | 0 preferred to Math.floor(num) for performance benefits
         i = (min + max) / 2 | 0;
 
         copiedTextProperties.text = text.substring(0, i);
@@ -322,7 +325,7 @@ export function getTailoredTextOrDefault(textProperties: TextProperties, maxWidt
         i--;
     }
 
-    return text.substring(ellipsis.length, i) + ellipsis;
+    return textProperties.text.substring(0, i - ellipsis.length) + ellipsis;
 }
 
 /**

diff --git a/test/textMeasurementServiceTests.ts b/test/textMeasurementServiceTests.ts
@@ -233,14 +233,17 @@ describe("Text measurement service", () => {
     });
 
     describe("getTailoredTextOrDefault", () => {
-        it("without ellipsis", () => {
+        const defaultTextProperties: TextProperties = {
+            fontFamily: "Arial",
+            fontSize: "11px",
+            fontWeight: "700",
+            fontStyle: "italic",
+            whiteSpace: "nowrap",
+            fontVariant: "normal"
+        }
+        it("long text without ellipsis", () => {
             let properties: TextProperties = {
-                fontFamily: "Arial",
-                fontSize: "11px",
-                fontWeight: "700",
-                fontStyle: "italic",
-                whiteSpace: "nowrap",
-                fontVariant: "normal",
+                ...defaultTextProperties,
                 text: "PowerBI rocks!",
             };
 
@@ -252,14 +255,9 @@ describe("Text measurement service", () => {
             expect(properties).toEqual(originalProperties);
         });
 
-        it("with ellipsis", () => {
+        it("long text with ellipsis", () => {
             let properties: TextProperties = {
-                fontFamily: "Arial",
-                fontSize: "11px",
-                fontWeight: "700",
-                fontStyle: "italic",
-                whiteSpace: "nowrap",
-                fontVariant: "normal",
+                ...defaultTextProperties,
                 text: "PowerBI rocks!",
             };
 
@@ -271,6 +269,47 @@ describe("Text measurement service", () => {
             expect(stringExtensions.startsWithIgnoreCase(text, "Pow")).toBeTruthy();
             expect(properties).toEqual(originalProperties);
         });
+
+        it("long text replaced with ellipsis", () => {
+            const MAX_WIDTH = 0;
+            const ORIGINAL_TEXT = "PowerBI rocks!";
+            const EXPECTED_TEXT = "...";
+            const properties: TextProperties = {
+                ...defaultTextProperties,
+                text: ORIGINAL_TEXT,
+            };
+
+            const result = textMeasurementService.getTailoredTextOrDefault(properties, MAX_WIDTH);
+
+            expect(result).toEqual(EXPECTED_TEXT);
+        })
+
+        it("choose best text by width", () => {
+            const MAX_WIDTH = 3;
+            const ORIGINAL_TEXT = "..m";
+            const EXPECTED_TEXT = "...";
+            const properties: TextProperties = {
+                ...defaultTextProperties,
+                text: ORIGINAL_TEXT,
+            };
+
+            const result = textMeasurementService.getTailoredTextOrDefault(properties, MAX_WIDTH);
+
+            expect(result).toEqual(EXPECTED_TEXT);
+        })
+
+        it("short string without ellipsis", () => {
+            const MAX_WIDTH = 3;
+            const ORIGINAL_TEXT = "1";
+
+            const properties: TextProperties = {
+                ...defaultTextProperties,
+                text: ORIGINAL_TEXT,
+            }
+            const result = textMeasurementService.getTailoredTextOrDefault(properties, MAX_WIDTH);
+
+            expect(result).toEqual(ORIGINAL_TEXT);
+        })
     });
 
     describe("svgEllipsis", () => {