Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- test(Textbox): add edge cases when wrapping line [#10639](https://github.com/fabricjs/fabric.js/pull/10639)
- fix(): Fix regex to parse the viewbox attribute to be more strict [#10636](https://github.com/fabricjs/fabric.js/pull/10636)
- chore(): enable @typescript-eslint/no-unnecessary-type-arguments lint rule [#10631](https://github.com/fabricjs/fabric.js/pull/10631)
- ci(): Changelog update action syncs with pr title [#10632](https://github.com/fabricjs/fabric.js/pull/10632)
Expand Down
177 changes: 158 additions & 19 deletions src/shapes/Textbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,31 @@ describe('Textbox', () => {
).toEqual(['xaxbx', 'cxdeyay', 'bid']);
});

it('wrapping with largestWordWidth and styles', () => {
const value = 'xaxbxc xdeyayb id sdgjhgsdg';
const textbox = new Textbox(value, {
width: 190,
styles: stylesFromArray(
[
{
style: {
fontWeight: 'bold',
fontSize: 64,
},
start: 0,
end: 10,
},
],
value,
),
});

expect(
textbox.textLines,
'lines match largestWordWidth with styles',
).toEqual(['xaxbxc', 'xdeyayb', 'id', 'sdgjhgsdg']);
});

it('wrapping with charspacing and splitByGrapheme positive', () => {
const textbox = new Textbox('xaxbxcxdeyaybid', {
width: 190,
Expand Down Expand Up @@ -654,36 +679,150 @@ describe('Textbox', () => {
});

it('missingNewlineOffset with splitByGrapheme', () => {
const textbox = new Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\naaa', {
const textbox = new Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\n aaa', {
width: 80,
splitByGrapheme: true,
});

const offset = textbox.missingNewlineOffset(0);
expect(offset, 'line 0 is interrupted by a \\n so has an offset of 1').toBe(
1,
);
const expected = {
lines: ['aaa', 'aaaa', 'aa', 'a', 'aaaa', 'aaaa', 'aaaa', ' aaa'],
hardBreaks: [1, 0, 1, 1, 0, 0, 1, 1],
cursor: [
{ selection: 1, lineIndex: 0, charIndex: 1 }, // a|aa
{ selection: 4, lineIndex: 1, charIndex: 0 }, // |aaaa
{ selection: 9, lineIndex: 2, charIndex: 1 }, // a|a
{ selection: 11, lineIndex: 3, charIndex: 0 }, // |a
{ selection: 14, lineIndex: 4, charIndex: 1 }, // a|aaa
{ selection: 20, lineIndex: 5, charIndex: 3 }, // aaa|a
{ selection: 22, lineIndex: 6, charIndex: 1 }, // a|aaa
{ selection: 29, lineIndex: 7, charIndex: 3 }, // aa|a
],
};

const offset2 = textbox.missingNewlineOffset(1);
expect(
offset2,
'line 1 is wrapped without a \\n so it does have an extra char count',
).toBe(0);
expect(textbox.textLines, 'wrap line by width').toEqual(expected.lines);

for (let i = 0; i < expected.hardBreaks.length; i++) {
const offset = textbox.missingNewlineOffset(i);
expect(
offset,
`line ${i} expect missingNewlineOffset: ${expected.hardBreaks[i]}`,
).toBe(expected.hardBreaks[i]);
}

let loc = textbox.get2DCursorLocation();
expect(loc.lineIndex, 'initial cursor line should be 0').toBe(0);
expect(loc.charIndex, 'initial cursor position should be 0').toBe(0);

for (let i = 0; i < expected.cursor.length; i++) {
const { selection, lineIndex, charIndex } = expected.cursor[i];
textbox.selectionStart = textbox.selectionEnd = selection;
loc = textbox.get2DCursorLocation();
expect(
loc.lineIndex,
`selection end ${selection} line ${lineIndex}`,
).toBe(lineIndex);
expect(
loc.charIndex,
`selection end ${selection} char ${charIndex}`,
).toBe(charIndex);
}
});

it('missingNewlineOffset with normal split', () => {
const texbox = new Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\naaa', {
width: 160,
it('missingNewlineOffset with normal split 1', () => {
const textbox = new Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\n aaa', {
width: 80,
});

const offset = texbox.missingNewlineOffset(0);
expect(offset, 'it returns always 1').toBe(1);
const expected = {
lines: ['aaa', 'aaaaaa', 'a', 'aaaaaaaaaaaa', ' aaa'],
hardBreaks: [1, 1, 1, 1, 1], // it has to always return 1
cursor: [
{ selection: 1, lineIndex: 0, charIndex: 1 }, // a|aa
{ selection: 4, lineIndex: 1, charIndex: 0 }, // |aaaaaa
{ selection: 12, lineIndex: 2, charIndex: 1 }, // a|
{ selection: 22, lineIndex: 3, charIndex: 9 }, // aaaaaaaaa|aaa
{ selection: 28, lineIndex: 4, charIndex: 2 }, // a|aa
],
};

expect(textbox.textLines, 'wrap by largestWordWidth').toEqual(
expected.lines,
);
for (let i = 0; i < expected.hardBreaks.length; i++) {
const offset = textbox.missingNewlineOffset(i);
expect(offset, `line ${i} expect ${expected.hardBreaks[i]}`).toBe(
expected.hardBreaks[i],
);
}

let loc = textbox.get2DCursorLocation();
expect(loc.lineIndex, 'initial cursor line should be 0').toBe(0);
expect(loc.charIndex, 'initial cursor position should be 0').toBe(0);

for (let i = 0; i < expected.cursor.length; i++) {
const { selection, lineIndex, charIndex } = expected.cursor[i];
textbox.selectionStart = textbox.selectionEnd = selection;
loc = textbox.get2DCursorLocation();
expect(
loc.lineIndex,
`selection end ${selection} line ${lineIndex}`,
).toBe(lineIndex);
expect(
loc.charIndex,
`selection end ${selection} char ${charIndex}`,
).toBe(charIndex);
}
});

const offset2 = texbox.missingNewlineOffset(1);
expect(offset2, 'it returns always 1').toBe(1);
it('missingNewlineOffset with normal split and short word', () => {
const textbox = new Textbox(
'aaa\naaaaaa \na\naaaaaaa aaaaa\n aaa',
{
width: 80,
},
);

const offset3 = texbox.missingNewlineOffset(2);
expect(offset3, 'it returns always 1').toBe(1);
const expected = {
lines: ['aaa', 'aaaaaa ', ' ', 'a', 'aaaaaaa', 'aaaaa', ' aaa'],
hardBreaks: [1, 1, 1, 1, 1, 1, 1], // Note: currently, lineIndex 2 and 4 no hardBreak but still removed a space
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i remember this 1 was fixing a bug that was in fabric since long.
Can we write here, this has to be 1 and we need to be careful if this condition changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I added this comment because the test logic here needs to be modified if we want to support text with non-space line breaks. Some thoughts updated in #10606.

cursor: [
{ selection: 1, lineIndex: 0, charIndex: 1 }, // a|aa
{ selection: 4, lineIndex: 1, charIndex: 0 }, // |aaaaaa
{ selection: 13, lineIndex: 2, charIndex: 1 }, // 8 space
{ selection: 22, lineIndex: 3, charIndex: 1 }, // a|
{ selection: 29, lineIndex: 4, charIndex: 6 }, // aaaaaa|a
{ selection: 32, lineIndex: 5, charIndex: 1 }, // a|aaaa
{ selection: 38, lineIndex: 6, charIndex: 1 }, // |aaa
],
};

expect(textbox.textLines, 'wrap by largestWordWidth').toEqual(
expected.lines,
);
for (let i = 0; i < expected.hardBreaks.length; i++) {
const offset = textbox.missingNewlineOffset(i);
expect(offset, `line ${i} expect ${expected.hardBreaks[i]}`).toBe(
expected.hardBreaks[i],
);
}

let loc = textbox.get2DCursorLocation();
expect(loc.lineIndex, 'initial cursor line should be 0').toBe(0);
expect(loc.charIndex, 'initial cursor position should be 0').toBe(0);

for (let i = 0; i < expected.cursor.length; i++) {
const { selection, lineIndex, charIndex } = expected.cursor[i];
textbox.selectionStart = textbox.selectionEnd = selection;
loc = textbox.get2DCursorLocation();
expect(
loc.lineIndex,
`selection end ${selection} line ${lineIndex}`,
).toBe(lineIndex);
expect(
loc.charIndex,
`selection end ${selection} char ${charIndex}`,
).toBe(charIndex);
}
});

it('_getLineStyle', () => {
Expand Down
Loading