fix(Textbox): minWidth of text, measure lines before wrapping

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(Textbox): minWidth of text, measure lines before wrapping [#8994](https://github.com/fabricjs/fabric.js/pull/8994)
 - ci(): Revert "invoke tests after changelog action (#8974)" [#9013](https://github.com/fabricjs/fabric.js/pull/9013)
 - fix(IText): empty line selection [#9019](https://github.com/fabricjs/fabric.js/pull/9019)
 - ci(): Added playwright testing [#8616](https://github.com/fabricjs/fabric.js/pull/8616)

src/shapes/Textbox.ts

@@ -4,6 +4,8 @@ import { IText } from './IText/IText';
 import { classRegistry } from '../ClassRegistry';
 import { createTextboxDefaultControls } from '../controls/commonControls';
 import { JUSTIFY } from './Text/constants';
+import type { GraphemeBBox } from './Text/Text';
+
 // @TODO: Many things here are configuration related and shouldn't be on the class nor prototype
 // regexes, list of properties that are not suppose to change by instances, magic consts.
 // this will be a separated effort
@@ -258,11 +260,26 @@ export class Textbox extends IText {
    * @param {Number} desiredWidth width you want to wrap to
    * @returns {Array} Array of lines
    */
-  _wrapText(lines: Array<any>, desiredWidth: number): Array<any> {
-    const wrapped = [];
+  _wrapText(lines: string[], desiredWidth: number, reservedSpace = 0) {
+    const wrapped: string[][] = [];
     this.isWrapping = true;
+    const { lines: data, minWidth } = this.measureLinesForWrapping(lines);
+    const additionalSpace = this._getWidthOfCharSpacing();
+    const maxWidth = Math.max(
+      desiredWidth - reservedSpace,
+      minWidth,
+      this.dynamicMinWidth
+    );
+    if (minWidth + reservedSpace > this.dynamicMinWidth) {
+      this.dynamicMinWidth = minWidth - additionalSpace + reservedSpace;
+    }
     for (let i = 0; i < lines.length; i++) {
-      wrapped.push(...this._wrapLine(lines[i], i, desiredWidth));
+      wrapped.push(
+        ...this._wrapLine(
+          { lines: data, minWidth, maxWidth, additionalSpace },
+          i
+        )
+      );
     }
     this.isWrapping = false;
     return wrapped;
@@ -278,24 +295,28 @@ export class Textbox extends IText {
    * @param {String} text
    * @param {number} lineIndex
    * @param {number} charOffset
-   * @returns {number}
    */
-  _measureWord(word, lineIndex: number, charOffset = 0): number {
+  _measureWord(word: string | string[], lineIndex: number, charOffset = 0) {
     let width = 0,
-      prevGrapheme;
-    const skipLeft = true;
-    for (let i = 0, len = word.length; i < len; i++) {
+      height = 0;
+    const data: GraphemeBBox<false>[] = [];
+    for (let i = 0, prevGrapheme: string | undefined; i < word.length; i++) {
       const box = this._getGraphemeBox(
         word[i],
         lineIndex,
         i + charOffset,
         prevGrapheme,
-        skipLeft
+        true
       );
+      // TODO: support vertical text
       width += box.kernedWidth;
+      height = Math.max(height, box.height);
+
+      data.push(box);
       prevGrapheme = word[i];
     }
-    return width;
+
+    return { width, height, data };
   }
 
   /**
@@ -308,6 +329,77 @@ export class Textbox extends IText {
     return value.split(this._wordJoiners);
   }
 
+  measureLinesForWrapping(lines: string[]) {
+    let min = 0;
+    const data = lines.map((line, lineIndex) => {
+      const parts = this.splitByGrapheme
+        ? this.graphemeSplit(line)
+        : this.wordSplit(line);
+
+      if (parts.length === 0) {
+        return { parts: [], minWidth: 0 };
+      }
+
+      const { data, minWidth } = parts.reduce(
+        ({ data, offset, minWidth }, part, index) => {
+          let infixWidth = 0;
+          let infix: string | null = null;
+          if (index > 0 && !this.splitByGrapheme) {
+            infix = ' ';
+            // measure infix at offset to respect styling etc.
+            infixWidth = this._measureWord(infix, lineIndex, offset).width;
+            // move cursor after infix
+            offset += infix.length;
+          }
+          // split words if necessary
+          const graphemes = !this.splitByGrapheme
+            ? this.graphemeSplit(part)
+            : // we must use concat for compatibility
+              [].concat(part);
+          // measure
+          const {
+            width,
+            height,
+            data: d,
+          } = this._measureWord(graphemes, lineIndex, offset);
+          data.push({
+            graphemes,
+            offset,
+            width,
+            height,
+            data: d,
+            infix,
+            infixWidth,
+          });
+          return {
+            offset: offset + graphemes.length,
+            data,
+            minWidth: Math.max(width, minWidth),
+          };
+        },
+        {
+          offset: 0,
+          data: [] as {
+            graphemes: string[];
+            offset: number;
+            width: number;
+            height: number;
+            data: GraphemeBBox<false>[];
+            /**
+             * space to insert before `graphemes`
+             */
+            infix: string | null;
+            infixWidth: number;
+          }[],
+          minWidth: 0,
+        }
+      );
+      min = Math.max(min, minWidth);
+      return { parts: data, minWidth };
+    });
+    return { lines: data, minWidth: min };
+  }
+
   /**
    * Wraps a line of text using the width of the Textbox and a context.
    * @param {Array} line The grapheme array that represent the line
@@ -316,84 +408,48 @@ export class Textbox extends IText {
    * @param {Number} reservedSpace space to remove from wrapping for custom functionalities
    * @returns {Array} Array of line(s) into which the given text is wrapped
    * to.
+   *
+   * @todo do we need to account for different spacing/offsetting in different languages?
    */
   _wrapLine(
-    _line,
-    lineIndex: number,
-    desiredWidth: number,
-    reservedSpace = 0
-  ): Array<any> {
-    const additionalSpace = this._getWidthOfCharSpacing(),
-      splitByGrapheme = this.splitByGrapheme,
-      graphemeLines = [],
-      words = splitByGrapheme
-        ? this.graphemeSplit(_line)
-        : this.wordSplit(_line),
-      infix = splitByGrapheme ? '' : ' ';
-
-    let lineWidth = 0,
-      line = [],
-      // spaces in different languages?
-      offset = 0,
-      infixWidth = 0,
-      largestWordWidth = 0,
-      lineJustStarted = true;
-    // fix a difference between split and graphemeSplit
-    if (words.length === 0) {
-      words.push([]);
-    }
-    desiredWidth -= reservedSpace;
-    // measure words
-    const data = words.map((word) => {
-      // if using splitByGrapheme words are already in graphemes.
-      word = splitByGrapheme ? word : this.graphemeSplit(word);
-      const width = this._measureWord(word, lineIndex, offset);
-      largestWordWidth = Math.max(width, largestWordWidth);
-      offset += word.length + infix.length;
-      return { word, width };
-    });
-
-    const maxWidth = Math.max(
-      desiredWidth,
-      largestWordWidth,
-      this.dynamicMinWidth
-    );
-    // layout words
-    offset = 0;
-    let i;
-    for (i = 0; i < words.length; i++) {
-      const word = data[i].word;
-      const wordWidth = data[i].width;
-      offset += word.length;
-
-      lineWidth += infixWidth + wordWidth - additionalSpace;
-      if (lineWidth > maxWidth && !lineJustStarted) {
-        graphemeLines.push(line);
-        line = [];
-        lineWidth = wordWidth;
-        lineJustStarted = true;
-      } else {
-        lineWidth += additionalSpace;
-      }
-
-      if (!lineJustStarted && !splitByGrapheme) {
-        line.push(infix);
-      }
-      line = line.concat(word);
+    {
+      lines,
+      additionalSpace,
+      maxWidth: desiredWidth,
+    }: ReturnType<this['measureLinesForWrapping']> & {
+      maxWidth: number;
+      additionalSpace: number;
+    },
+    lineIndex: number
+  ) {
+    const { parts: lineData } = lines[lineIndex];
 
-      infixWidth = splitByGrapheme
-        ? 0
-        : this._measureWord([infix], lineIndex, offset);
-      offset++;
-      lineJustStarted = false;
+    // fix a difference between split and graphemeSplit
+    if (!lineData.length) {
+      return [[]];
     }
 
-    i && graphemeLines.push(line);
-
-    if (largestWordWidth + reservedSpace > this.dynamicMinWidth) {
-      this.dynamicMinWidth = largestWordWidth - additionalSpace + reservedSpace;
-    }
-    return graphemeLines;
+    // layout
+    return lineData.reduce(
+      ({ lines, lineWidth }, { graphemes, infix, infixWidth, width }, i) => {
+        // `i === 0` => `infixWidth === 0`
+        const lineWidthAfter = lineWidth + infixWidth + width;
+        if (i === 0 || lineWidthAfter - additionalSpace > desiredWidth) {
+          // push a new line, we spread to protect `data` from mutation
+          lines.push([...graphemes]);
+          lineWidth = width;
+        } else {
+          const currentLine = lines[lines.length - 1];
+          // push infix if necessary, `i === 0 || splitByGrapheme` => `infix === null`
+          infix && currentLine.push(infix);
+          // push graphemes
+          currentLine.push(...graphemes);
+          lineWidth = lineWidthAfter;
+        }
+        return { lines, lineWidth };
+      },
+      { lines: [] as string[][], lineWidth: 0 }
+    ).lines;
   }
 
   /**

test/unit/textbox.js

@@ -381,15 +381,15 @@
     var textbox = new fabric.Textbox('xa xb xc xd xe ya yb id', {
       width: 2000,
     });
-    var line1 = textbox._wrapLine('xa xb xc xd xe ya yb id', 0, 100, 0);
+    var line1 = textbox._wrapText(['xa xb xc xd xe ya yb id'], 100, 0);
     var expected1 =  [
       ['x', 'a', ' ', 'x', 'b'],
       ['x', 'c', ' ', 'x', 'd'],
       ['x', 'e', ' ', 'y', 'a'],
       ['y', 'b', ' ', 'i', 'd']];
     assert.deepEqual(line1, expected1, 'wrapping without reserved');
     assert.deepEqual(textbox.dynamicMinWidth, 40, 'wrapping without reserved');
-    var line2 = textbox._wrapLine('xa xb xc xd xe ya yb id', 0, 100, 50);
+    var line2 = textbox._wrapText(['xa xb xc xd xe ya yb id'], 100, 50);
     var expected2 =  [
       ['x', 'a'],
       ['x', 'b'],
@@ -406,12 +406,24 @@
     var textbox = new fabric.Textbox('', {
       width: 10,
     });
-    var line1 = textbox._wrapLine('', 0, 100, 0);
+    var line1 = textbox._wrapText([''], 100, 0);
     assert.deepEqual(line1, [[]], 'wrapping without splitByGrapheme');
     textbox.splitByGrapheme = true;
-    var line2 = textbox._wrapLine('', 0, 100, 0);
+    var line2 = textbox._wrapText([''], 100, 0);
     assert.deepEqual(line2, [[]], 'wrapping with splitByGrapheme');
   });
+  QUnit.test('wrapping respects max line width', function (assert) {
+    const a = 'xaxbxc xdxeyaybid xaxbxc';
+    const b = 'xaxbxcxdxeyaybidxaxbxcxdxeyaybid';
+    [true, false].forEach(order => {
+      [true, false].forEach(space => {
+        const ordered = order ? [a, b] : [b, a];
+        const text = ordered.join(space ? ' ' : '\n');
+        const { _textLines: lines } = new fabric.Textbox(text);
+        assert.deepEqual(lines, ordered.map(line => line.split('')), `max line width should be respected for ${text}`);
+      });
+    });
+  });
   QUnit.test('texbox will change width from the mr corner', function(assert) {
     var text = new fabric.Textbox('xa xb xc xd xe ya yb id', { strokeWidth: 0 });
     canvas.add(text);