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(Textbox) Textbox inputs with new lines #9192

Merged
merged 14 commits into from
Sep 9, 2023
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]

- fix() Textbox inputs with new lines, regression from #9097 [#9192](https://github.com/fabricjs/fabric.js/pull/9192)
- docs(): add link to contributing guide [#8393](https://github.com/fabricjs/fabric.js/pull/8393)
- test(e2e): Drag&Drop tests [#9112](https://github.com/fabricjs/fabric.js/pull/9112)
- fix(CanvasEvents): regression of `getPointer` usages + BREAKING: drop event data [#9186](https://github.com/fabricjs/fabric.js/pull/9186)
Expand Down
85 changes: 85 additions & 0 deletions e2e/tests/text/adding-text/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { CanvasUtil } from '../../../utils/CanvasUtil';
import { expect, test } from '@playwright/test';
import { ObjectUtil } from '../../../utils/ObjectUtil';
import type { Textbox } from 'fabric';
import '../../../setup';

[false, true].forEach((splitByGrapheme) => {
test(`adding new lines and copy paste - splitByGrapheme: ${splitByGrapheme}`, async ({
page,
context,
}) => {
await context.grantPermissions(['clipboard-read', 'clipboard-write']);
const canvasUtil = new CanvasUtil(page);
const textBoxutil = new ObjectUtil(page, 'text');

await textBoxutil.executeInBrowser(
(textbox: Textbox, context) => {
textbox.splitByGrapheme = context.splitByGrapheme;
textbox.set('dirty', true);
textbox.initDimensions();
textbox.canvas.renderAll();
},
{ splitByGrapheme }
);

await expect(await canvasUtil.screenshot()).toMatchSnapshot({
name: `1-initial-splitByGrapheme-${splitByGrapheme}.png`,
asturur marked this conversation as resolved.
Show resolved Hide resolved
});
await canvasUtil.click({
position: {
x: 50,
y: 65,
},
delay: 200,
});
await canvasUtil.click({
position: {
x: 50,
y: 65,
},
delay: 200,
});
await page.mouse.down();
await page.mouse.move(65, 120, { steps: 15 });
await page.mouse.up();
await canvasUtil.ctrlC();
await canvasUtil.click({
position: {
x: 176,
y: 65,
},
delay: 200,
});
await canvasUtil.press('Enter');
await canvasUtil.press('Enter');
await canvasUtil.press('a');
await canvasUtil.press('b');
await canvasUtil.press('c');
await canvasUtil.press('Enter');
await canvasUtil.press('Enter');
await expect(await canvasUtil.screenshot()).toMatchSnapshot({
name: `2-before-pasting-splitByGrapheme-${splitByGrapheme}.png`,
});
await canvasUtil.ctrlV();
await expect(await canvasUtil.screenshot()).toMatchSnapshot({
name: `3-after-pasting-splitByGrapheme-${splitByGrapheme}.png`,
maxDiffPixelRatio: 0.03,
});
// NOTE: Here is clear that there style bug of #9028 is visible splitbygrapheme true only
// to be triggered the copy paste has to happen across lines
await canvasUtil.click({
position: {
x: 176,
y: 152,
},
delay: 200,
});
await canvasUtil.press('Enter');
await canvasUtil.press('Enter');
await expect(await canvasUtil.screenshot()).toMatchSnapshot({
name: `4-after-adding-more-lines-splitByGrapheme-${splitByGrapheme}.png`,
maxDiffPixelRatio: 0.03,
});
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions e2e/tests/text/adding-text/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Textbox, util } from 'fabric';
import { beforeAll } from '../../test';

beforeAll(
(canvas) => {
const textValue = 'fabric.js sandbox';
const text = new Textbox(textValue, {
originX: 'center',
objectCaching: false,
splitByGrapheme: true,
width: 200,
top: 20,
styles: util.stylesFromArray(
[
{
style: {
fontWeight: 'bold',
fontSize: 64,
},
start: 0,
end: 9,
},
],
textValue
),
});
canvas.add(text);
canvas.centerObjectH(text);
return { text };
},
{ width: 300, height: 700 }
);
17 changes: 17 additions & 0 deletions e2e/utils/CanvasUtil.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { type LocatorScreenshotOptions, type Page } from '@playwright/test';
import type { Canvas } from 'fabric';
import os from 'node:os';

export class CanvasUtil {
constructor(readonly page: Page, readonly selector = '#canvas') {}
Expand All @@ -8,6 +9,22 @@ export class CanvasUtil {
return this.page.click(`canvas_top=${this.selector}`, clickProperties);
}

press(keyPressed: Parameters<Page['press']>[1]) {
return this.page.keyboard.press(keyPressed, { delay: 200 });
}

ctrlC(): Promise<void> {
const isMac = os.platform() === 'darwin';
const modifier = isMac ? 'Meta' : 'Control';
return this.page.keyboard.press(`${modifier}+KeyC`);
}

ctrlV(): Promise<void> {
const isMac = os.platform() === 'darwin';
const modifier = isMac ? 'Meta' : 'Control';
return this.page.keyboard.press(`${modifier}+KeyV`);
}

screenshot(options: LocatorScreenshotOptions = {}) {
return this.page
.locator(`canvas_wrapper=${this.selector}`)
Expand Down
4 changes: 2 additions & 2 deletions src/shapes/Textbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ export class Textbox<
: this.wordSplit(line);

if (wordsOrGraphemes.length === 0) {
return [];
return [{ word: [], width: 0 }];
}

return wordsOrGraphemes.map((word: string) => {
Expand All @@ -349,7 +349,7 @@ export class Textbox<
: this.graphemeSplit(word);
const width = this._measureWord(graphemeArray, lineIndex, offset);
largestWordWidth = Math.max(width, largestWordWidth);
offset += word.length + infix.length;
offset += graphemeArray.length + infix.length;
return { word: graphemeArray, width };
});
});
Expand Down
Loading