Repository: fabricjs/fabric.js
Pull request #9341 (2023-09-14 13:35:38)

fix(IText): cursor width under group
<!--
        Hi there!
        Thanks for taking the time and putting the effort into making fabric better! 💖
        Take a look at /CONTRIBUTING.md for crucial instructions regarding local setup, testing etc.
        https://github.com/fabricjs/fabric.js/blob/master/CONTRIBUTING.md

        Adding tests that verify your fix and safeguard it from unwanted loss and changes is a MUST.

        Pull Requests are not always simple. Don't hesitate to ask for help (beware of [gotchas](http://fabricjs.com/fabric-gotchas) 😓).
        We appreciate your effort and would like the process to be productive and enjoyable.
        A strong community means a strong and better product for everyone.
-->

## Motivation

@jiayihu 

Found while editing text in a scaled group

<!-- Why you are proposing -->
<!-- You can use the @closes notation to mark issues that will be resolved by this PR -->



## Description

<!-- What you are proposing -->

Text cursor width doesn't take into account group transform.
Another miss of the group rewrite.
I saw that draggable text has the same issue

## Changes

<!-- before the fix vs. after -->

## Gist

<!-- Technical stuff if necessary -->

## In Action

<!-- Show case your accomplishment -->
<!-- Upload screenshots, screencasts and live examples showing your fix in contrast to the current state -->


github-actions[bot]: <!-- BUILD STATS COMMENT -->
**Build Stats**

 | file / KB (diff) | bundled | minified | 
 | --- | --- | --- | 
 | fabric | 915.669 (**+0.014**) | 305.483 (**+0.014**) | 

asturur: Those are all the artifacts of interactive group that insist was a bad idea and should be marked beta also with 6.0 release.
asturur: The fix is totally fine, i want to think of a nicer way to test this, also with more cases.
This should be quick and is a bug, so i ll bump it up in my list
ShaMan123: > Those are all the artifacts of interactive group that insist was a bad idea and should be marked beta also with 6.0 release.

@jiayihu can maybe share what v6 is doing to the project we are working on.
You should have seen the amount of impossible workarounds removed due to interactive groups.
I can't defend it more than I have.
jiayihu: Group subselection is indeed a common feature of canvas applications, think about draw.io, Miro, tldraw.com etc where you can group objects but still want to subselect. Think about photoshop/sketch/Figma layers as well, all groups. 
The way I see a Group is basically like a DIV, you have many reasons to use it as wrapper while still wanting the items to be selectable/interactive.
asturur: pulling in and out object in and from groups was easy enough after the first tweak @ShaMan123 did to the group that persisted the group transform when adding/removing.
All the rest is superflous
asturur: now that i added the angles and i don't see the results i expect, i want to see it in action
asturur: It behaves mostly ok also with weird situations of angle and scale. It never look broken

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ba6f0da53..6a3656a54 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,9 +2,10 @@
 
 ## [next]
 
+- fix(IText): cursor width under group [#9341](https://github.com/fabricjs/fabric.js/pull/9341)
 - TS(Canvas): constructor optional el [#9348](https://github.com/fabricjs/fabric.js/pull/9348)
 
-## [6.0.0-b3]
+## [6.0.0-b13]
 
 - fix(Textbox): implemente a fix for the style shifting issues on new lines [#9197](https://github.com/fabricjs/fabric.js/pull/9197)
 - Fix(Control) fix a regression in `wrap with fixed anchor`, regression from #8400 [#9326](https://github.com/fabricjs/fabric.js/pull/9326)
diff --git a/src/shapes/IText/IText.ts b/src/shapes/IText/IText.ts
index 117c2d883..4a2ef312e 100644
--- a/src/shapes/IText/IText.ts
+++ b/src/shapes/IText/IText.ts
@@ -500,7 +500,7 @@ export class IText<
       charIndex =
         cursorLocation.charIndex > 0 ? cursorLocation.charIndex - 1 : 0,
       charHeight = this.getValueOfPropertyAt(lineIndex, charIndex, 'fontSize'),
-      multiplier = this.scaleX * this.canvas!.getZoom(),
+      multiplier = this.getObjectScaling().x * this.canvas!.getZoom(),
       cursorWidth = this.cursorWidth / multiplier,
       dy = this.getValueOfPropertyAt(lineIndex, charIndex, 'deltaY'),
       topOffset =


Test patch:
diff --git a/src/shapes/IText/IText.test.ts b/src/shapes/IText/IText.test.ts
new file mode 100644
index 000000000..6317dfa0b
--- /dev/null
+++ b/src/shapes/IText/IText.test.ts
@@ -0,0 +1,42 @@
+import '../../../jest.extend';
+import { Group } from '../Group';
+import { IText } from './IText';
+
+describe('IText', () => {
+  describe('cursor drawing width', () => {
+    test.each([
+      { scale: 1, zoom: 1, textScale: 1, angle: 0, textAngle: 0 },
+      { scale: 1, zoom: 50, textScale: 2, angle: 0, textAngle: 0 },
+      { scale: 200, zoom: 1, textScale: 2, angle: 45, textAngle: 0 },
+      { scale: 200, zoom: 1, textScale: 1, angle: 0, textAngle: 0 },
+      { scale: 200, zoom: 50, textScale: 1, angle: 30, textAngle: 30 },
+      { scale: 200, zoom: 1 / 200, textScale: 1, angle: 0, textAngle: 0 },
+      { scale: 200, zoom: 1 / 200, textScale: 2, angle: 0, textAngle: 90 },
+    ])(
+      'group scaled by $scale and rotated by $angle , text scaled by $textScale and rotated by $textAngle, and canvas zoomed by $zoom',
+      ({ scale, zoom, textScale, angle, textAngle }) => {
+        const text = new IText('testing', {
+          cursorWidth: 100,
+          angle: textAngle,
+          scaleX: textScale,
+          scaleY: textScale,
+        });
+        const group = new Group([text]);
+        group.set({ scaleX: scale, scaleY: scale, angle });
+        group.setCoords();
+        const fillRect = jest.fn();
+        const getZoom = jest.fn().mockReturnValue(zoom);
+        const mockContext = { fillRect };
+        const mockCanvas = { contextTop: mockContext, getZoom };
+        jest.replaceProperty(text, 'canvas', mockCanvas);
+
+        text.renderCursorAt(1);
+        const call = fillRect.mock.calls[0];
+        expect({ width: call[2], height: call[3] }).toMatchSnapshot({
+          cloneDeepWith: (value) =>
+            typeof value === 'number' ? value.toFixed(3) : undefined,
+        });
+      }
+    );
+  });
+});
diff --git a/src/shapes/IText/__snapshots__/IText.test.ts.snap b/src/shapes/IText/__snapshots__/IText.test.ts.snap
new file mode 100644
index 000000000..1f1b2ccc9
--- /dev/null
+++ b/src/shapes/IText/__snapshots__/IText.test.ts.snap
@@ -0,0 +1,50 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`IText cursor drawing width group scaled by 1 and rotated by 0 , text scaled by 1 and rotated by 0, and canvas zoomed by 1 1`] = `
+{
+  "height": "40.000",
+  "width": "100.000",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 1 and rotated by 0 , text scaled by 2 and rotated by 0, and canvas zoomed by 50 1`] = `
+{
+  "height": "40.000",
+  "width": "1.000",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 200 and rotated by 0 , text scaled by 1 and rotated by 0, and canvas zoomed by 0.005 1`] = `
+{
+  "height": "40.000",
+  "width": "100.000",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 200 and rotated by 0 , text scaled by 1 and rotated by 0, and canvas zoomed by 1 1`] = `
+{
+  "height": "40.000",
+  "width": "0.500",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 200 and rotated by 0 , text scaled by 2 and rotated by 90, and canvas zoomed by 0.005 1`] = `
+{
+  "height": "40.000",
+  "width": "50.000",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 200 and rotated by 30 , text scaled by 1 and rotated by 30, and canvas zoomed by 50 1`] = `
+{
+  "height": "40.000",
+  "width": "0.010",
+}
+`;
+
+exports[`IText cursor drawing width group scaled by 200 and rotated by 45 , text scaled by 2 and rotated by 0, and canvas zoomed by 1 1`] = `
+{
+  "height": "40.000",
+  "width": "0.250",
+}
+`;