refactor(#9498): refactor endCurrentTransform

[ShaMan123]

Motivation

closes #9498

Description

I encountered an exception thrown by fabric when using _discardActiveObject => endCurrentTransform w/o passing the e param which was typed as optional in _discardActiveObject but was not in endCurrentTransform/modified event.
Types were wrong, that is why we missed it.
I had to refactor some code and that made some cleanup possible.

Changes

• squashed _finalizeCurrentTransform => endCurrentTransform and moved from Canvas to SelectableCanvas
• exposed FabricObject#isTransforming(action)
• rm _scaling flag: blame point to 20a87c6#diff-8eca4d2c9c7324551d9e6a7ce3817a987c9d598332b3f2f6ab02d2f1c98cde4fR1033 - not used by fabric => isTransforming('scale') can be used instead
• rm isMoving => isTransforming('drag') is used instead
• refactor _handleEvent to support passing a custom object needed for mouse up after _currentTransform has been nullified

Gist

In Action

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	907.037 (-0.524)	304.937 (+0.231)

[ShaMan123]

This looks good to me, let's see what tests have to say

[ShaMan123]

this is a change done when sqaushing _finalizeCurrentTransform requiring a bit more work to adapt mouse up logic

[asturur]

Somehow it was working in javascript before.
Can we make the 'e' part of the event optional without any refactor?
I understand how the bug was introduced, but i need to dig deeper on what are the side effects.

[asturur]

Where is the exception thrown?
My understanding is that e will be undefined in the event handler, and so developer code will eventually thrown.
Or is fabric throwing by itself?

[ShaMan123]

You understand correctly

[ShaMan123]

stack call drill down:
_discardActiveObject
endCurrentTransform
_finalizeCurrentTransform
fire modified event - throws e is not defined

I can do a patch that is not as clean as this and PR this on top. Should I?

[ShaMan123]

Actually I think not because the only change needed is in types making e optional in the modified event

[asturur]

Sorry i completely forgot about this on.
I wanted exactly to ask if we can just make the type optional and just write over it that the only case we know in which the event is missing is when we call endTransform by code and not by user actions?

[ShaMan123]

Sorry i completely forgot about this on. I wanted exactly to ask if we can just make the type optional and just write over it that the only case we know in which the event is missing is when we call endTransform by code and not by user actions?

I don't understand what you suggest. Isn't that this PR? Are you missing a comment in the code?

[github-actions]

Coverage after merging fix-9498 into master will be

82.80%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	7.69%	100%	0%	14.29%	17, 20, 23, 35, 38, 41
src
ClassRegistry.ts	100%	100%	100%	100%
Collection.ts	94.85%	94.83%	87.10%	97.14%	109, 112, 215–216, 241–242
CommonMethods.ts	96.55%	87.50%	100%	100%	9
Intersection.ts	100%	100%	100%	100%
Observable.ts	87.76%	85.29%	87.50%	89.58%	134–135, 160–161, 32–33, 41, 50, 78, 89
Point.ts	100%	100%	100%	100%
Shadow.ts	98.36%	95.65%	100%	100%	178
cache.ts	97.06%	90%	100%	100%	57
config.ts	74.14%	65%	66.67%	82.76%	131, 139, 139, 139, 139, 139–141, 152–154, 30
constants.ts	100%	100%	100%	100%
src/LayoutManager
LayoutManager.ts	87.50%	89.29%	76.92%	90%	133–134, 136–137, 137, 137, 137, 137, 231, 289–290, 301, 46
constants.ts	100%	100%	100%	100%
src/LayoutManager/LayoutStrategies
ClipPathLayout.ts	82.05%	72.22%	100%	88.89%	30–31, 42, 54, 57–58, 65
FitContentLayout.ts	100%	100%	100%	100%
FixedLayout.ts	20%	0%	0%	40%	20–21, 23, 23, 23, 23, 23
LayoutStrategy.ts	86%	64.71%	100%	96.15%	36, 43, 43, 43, 51, 71–72
utils.ts	91.67%	85.71%	100%	100%	28, 34
src/Pattern
Pattern.ts	90.54%	93.94%	80%	90.32%	119, 130, 139, 32, 36
src/brushes
BaseBrush.ts	100%	100%	100%	100%
CircleBrush.ts	0%	0%	0%	0%	108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 13, 136, 138, 23–24, 32–36, 40–44, 51–54, 62–66, 68, 76, 76, 76, 76, 76–77, 79, 79, 79–82, 84, 92–93, 95, 97–99
PatternBrush.ts	97.06%	87.50%	100%	100%	21
PencilBrush.ts	91.06%	82.35%	100%	93.81%	122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
SprayBrush.ts	0%	0%	0%	0%	107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 148, 148, 148–151, 153–156, 160–161, 163, 165–168, 17, 171, 178–179, 18, 181, 183–184, 186, 193–194, 196–197, 20, 200, 200, 207, 207, 21, 211, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
Canvas.ts	78.67%	77.09%	82.46%	79.29%	1011–1013, 1016–1017, 1021–1022, 1133–1135, 1138–1139, 1212, 1324, 1444, 158, 183, 289, 289–294, 299, 322–323, 328–333, 353, 353, 353–354, 354, 354–355, 363, 368–369, 369, 369–370, 372, 381, 387–388, 388, 388, 42, 431, 439, 443, 443, 443–444, 446, 46, 528–529, 529, 529–530, 536, 536, 536–538, 558, 560, 560, 560–561, 561, 561, 564, 564, 564–565, 568, 577–578, 580–581, 583, 583–584, 586–587, 599–600, 600, 600–601, 603–608, 614, 621, 658, 658, 658, 660, 662–667, 673, 679, 679, 679–680, 682, 685, 690, 703, 731, 783–784, 784, 784, 784, 784, 784, 787–788, 791, 791–793, 796–797, 877, 884, 884, 884, 894, 923, 938–939, 939, 939–941, 944–945, 945, 945, 947, 955, 955, 955–957, 957, 957, 964–965, 973–974, 974, 974–975, 981, 983
CanvasOptions.ts	100%	100%	100%	100%
SelectableCanvas.ts	93.17%	91.27%	94.44%	94.44%	1033, 1041, 1152, 1154, 1156–1157, 1226–1227, 461–462, 464–465, 465, 465, 514–515, 591, 596, 646, 648–649, 695, 700–701, 728–729, 789–790, 795–799, 801, 960, 960–961, 964, 984, 984
StaticCanvas.ts	96.53%	93.09%	98.92%	98.29%	1019, 1029, 1080–1082, 1085, 1120–1121, 1197, 1206, 1206, 1210, 1210, 1257–1258, 178–179, 195, 559, 571–572, 902–903, 903, 903–904
StaticCanvasOptions.ts	100%	100%	100%	100%
TextEditingManager.ts	86%	71.43%	91.67%	91.67%	17, 17, 17–18, 18, 18
src/canvas/DOMManagers
CanvasDOMManager.ts	95.52%	70%	100%	100%	21–22, 29
StaticCanvasDOMManager.ts	97.50%	88.89%	100%	100%	33
util.ts	86.67%	80.56%	83.33%	93.94%	14, 26, 63–64, 67, 67, 74, 93–94
src/color
Color.ts	94.96%	91.67%	96.30%	96.05%	233, 258–259, 267–268, 48
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	85.71%	76.92%	100%	89.74%	55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
Control.ts	94.44%	93.10%	91.67%	96.77%	183, 249, 354
changeWidth.ts	100%	100%	100%	100%
commonControls.ts	100%	100%	100%	100%
controlRendering.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
drag.ts	100%	100%	100%	100%
fireEvent.ts	88.89%	75%	100%	100%	13
polyControl.ts	10.87%	0%	0%	16.13%	114, 114, 114, 114, 114, 116–119, 119, 122, 129, 24–26, 50–52, 58–59, 61, 71, 77–79, 79, 81, 84, 86, 90–92, 94, 99
rotate.ts	19.57%	12.50%	50%	21.43%	41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73,

[asturur]

we fix the type issue with this: #9596
Then when you can expand on why the other refactors are necessary or better or needed for some other bug we can evaluate the other changes as well