Repository: fabricjs/fabric.js
Pull request #10190 (2024-09-29 07:06:51)

update(IText): Add method _enterEditing
Add method _enterEditing to prevent triggering text:editing:entered.

https://github.com/fabricjs/fabric.js/issues/10187

codesandbox[bot]: #### Review or Edit in CodeSandbox

Open the branch in <a href="https://codesandbox.io/p/github/zhe-he/fabric.js/patch-i-text?mode=review&utm_source=gh_app">Web Editor</a> &bull; <a href="https://codesandbox.io/p/vscode?owner=zhe-he&repo=fabric.js&branch=patch-i-text&utm_source=gh_app">VS Code</a> &bull; <a href="https://codesandbox.io/p/vscode?owner=zhe-he&repo=fabric.js&branch=patch-i-text&insiders=true&utm_source=gh_app">Insiders</a><br>
Open <a href="https://codesandbox.io/p/devtool/preview/zhe-he/fabric.js/patch-i-text?task=preview&port=51423&redirect=true&utm_source=gh_app">Preview</a>

<!-- open-in-codesandbox:complete -->

asturur: Also please divide eventual tests for enterEditing for the 2 respective methods, under JEST.
If you don't know how to do that i can provide guidance.
zhe-he: Test cases have been added, but I cannot test them locally. The reasons are as follows:
When I executed `./node_modules/.bin/jest ./src/shapes/IText/ITextBehavior.test.ts` before adding the test cases, the previous test cases reported errors. This issue may need your attention.
asturur: Hopefully i didn't break anything.
I left _exitEditing to avoid breaking changes and introduced anyway exitEditingImpl
asturur: > Test cases have been added, but I cannot test them locally. The reasons are as follows: When I executed `./node_modules/.bin/jest ./src/shapes/IText/ITextBehavior.test.ts` before adding the test cases, the previous test cases reported errors. This issue may need your attention.

There was an unrestored mock in the previous tests

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 195b25b79..4d1efa904 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## [next]
 
+- update(IText): Add method enterEditingImpl/exitEditingImpl that executes the logic of enterEditing/exitEditing without events [#10187](https://github.com/fabricjs/fabric.js/issues/10187)
 - fix(FabricObject): Fix clipPath blurryness with scale [#9774](https://github.com/fabricjs/fabric.js/pull/9774)
 
 ## [6.4.3]
diff --git a/src/shapes/IText/IText.ts b/src/shapes/IText/IText.ts
index 32ef1d8ed..aba9905e8 100644
--- a/src/shapes/IText/IText.ts
+++ b/src/shapes/IText/IText.ts
@@ -692,7 +692,7 @@ export class IText<
   }
 
   dispose() {
-    this._exitEditing();
+    this.exitEditingImpl();
     this.draggableTextDelegate.dispose();
     super.dispose();
   }
diff --git a/src/shapes/IText/ITextBehavior.ts b/src/shapes/IText/ITextBehavior.ts
index cb77c7720..ad22afab3 100644
--- a/src/shapes/IText/ITextBehavior.ts
+++ b/src/shapes/IText/ITextBehavior.ts
@@ -387,6 +387,22 @@ export abstract class ITextBehavior<
     if (this.isEditing || !this.editable) {
       return;
     }
+    this.enterEditingImpl();
+    this.fire('editing:entered', e ? { e } : undefined);
+    this._fireSelectionChanged();
+    if (this.canvas) {
+      this.canvas.fire('text:editing:entered', {
+        target: this as unknown as IText,
+        e,
+      });
+      this.canvas.requestRenderAll();
+    }
+  }
+
+  /**
+   * runs the actual logic that enter from editing state, see {@link enterEditing}
+   */
+  enterEditingImpl() {
     if (this.canvas) {
       this.canvas.calcOffset();
       this.canvas.textEditingManager.exitTextEditing();
@@ -403,15 +419,6 @@ export abstract class ITextBehavior<
     this._textBeforeEdit = this.text;
 
     this._tick();
-    this.fire('editing:entered', e ? { e } : undefined);
-    this._fireSelectionChanged();
-    if (this.canvas) {
-      this.canvas.fire('text:editing:entered', {
-        target: this as unknown as IText,
-        e,
-      });
-      this.canvas.requestRenderAll();
-    }
   }
 
   /**
@@ -669,6 +676,9 @@ export abstract class ITextBehavior<
 
   /**
    * runs the actual logic that exits from editing state, see {@link exitEditing}
+   * Please use exitEditingImpl, this function was kept to avoid breaking changes.
+   * Will be removed in fabric 7.0
+   * @deprecated use "exitEditingImpl"
    */
   protected _exitEditing() {
     const hiddenTextarea = this.hiddenTextarea;
@@ -686,10 +696,10 @@ export abstract class ITextBehavior<
   }
 
   /**
-   * Exits from editing state and fires relevant events
+   * runs the actual logic that exits from editing state, see {@link exitEditing}
+   * But it does not fire events
    */
-  exitEditing() {
-    const isTextChanged = this._textBeforeEdit !== this.text;
+  exitEditingImpl() {
     this._exitEditing();
     this.selectionEnd = this.selectionStart;
     this._restoreEditingProps();
@@ -697,6 +707,15 @@ export abstract class ITextBehavior<
       this.initDimensions();
       this.setCoords();
     }
+  }
+
+  /**
+   * Exits from editing state and fires relevant events
+   */
+  exitEditing() {
+    const isTextChanged = this._textBeforeEdit !== this.text;
+    this.exitEditingImpl();
+
     this.fire('editing:exited');
     isTextChanged && this.fire(MODIFIED);
     if (this.canvas) {


Test patch:
diff --git a/src/shapes/IText/ITextBehavior.test.ts b/src/shapes/IText/ITextBehavior.test.ts
index 42c99c088..b54514c95 100644
--- a/src/shapes/IText/ITextBehavior.test.ts
+++ b/src/shapes/IText/ITextBehavior.test.ts
@@ -106,6 +106,9 @@ describe('IText cursor animation snapshot', () => {
       });
     jest.useFakeTimers();
   });
+  afterAll(() => {
+    ValueAnimation.prototype.calculate = origCalculate;
+  });
   beforeEach(() => {
     jest.runAllTimers();
     currentAnimation = [];
@@ -193,3 +196,32 @@ describe('IText _tick', () => {
     expect(_tickMock).toHaveBeenCalledWith(0);
   });
 });
+
+describe('Itext enterEditing and exitEditing', () => {
+  const enterMock = jest.fn();
+  const exitMock = jest.fn();
+
+  afterEach(() => {
+    enterMock.mockClear();
+    exitMock.mockClear();
+  });
+
+  test('Entering and leaving edit triggers the listener', () => {
+    const iText = new IText('some word');
+    iText.on('editing:entered', enterMock);
+    iText.on('editing:exited', exitMock);
+    iText.enterEditing();
+    expect(enterMock).toHaveBeenCalledTimes(1);
+    iText.exitEditing();
+    expect(exitMock).toHaveBeenCalledTimes(1);
+  });
+  test('Entering and leaving edit does not trigger the listener', () => {
+    const iText = new IText('some word');
+    iText.on('editing:entered', enterMock);
+    iText.on('editing:exited', exitMock);
+    iText.enterEditingImpl();
+    expect(enterMock).toHaveBeenCalledTimes(0);
+    iText.exitEditingImpl();
+    expect(exitMock).toHaveBeenCalledTimes(0);
+  });
+});