chore(TS): IText types

[ShaMan123]

Motivation

part of removing ts-nocheck effort

Description

remove ts-nocheck for itext_behavior, itext_click_behavior, IText

Changes

smashed click behavior init methods into initBehavior
BREAKING: changed getLocalPointer signature

Gist

first merge #8598

In Action

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	938.467 (-0.405)	295.964 (-0.140)

[ShaMan123]

done

[github-actions]

Coverage after merging more-text-types into master will be

82.94%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.ts	100%	100%	100%	100%
src
cache.ts	96.97%	90%	100%	100%	56
config.ts	77.27%	66.67%	66.67%	84.62%	130, 138–140, 151–153
constants.ts	100%	100%	100%	100%
env.ts	72.73%	53.33%	100%	79.17%	22, 22–23, 23, 23, 25, 25, 27, 29, 31–32, 62
intersection.class.ts	100%	100%	100%	100%
pattern.class.ts	93.85%	90.32%	100%	96.15%	100, 124, 135, 144
point.class.ts	100%	100%	100%	100%
shadow.class.ts	98.48%	96%	100%	100%	156
typedefs.ts	100%	100%	100%	100%
src/brushes
base_brush.class.ts	100%	100%	100%	100%
circle_brush.class.ts	0%	0%	0%	0%	100, 102–104, 113, 113, 113, 115, 117, 119–121, 123–126, 134, 141, 143, 28–29, 37–41, 45–49, 56–59, 67–71, 73, 81, 81, 81, 81, 81–82, 84, 84, 84–87, 89, 97–98
pattern_brush.class.ts	97.06%	87.50%	100%	100%	21
pencil_brush.class.ts	91.81%	85.42%	100%	93.52%	122–123, 152, 152–154, 276, 280, 285–286, 68–69, 84–85
spray_brush.class.ts	0%	0%	0%	0%	100–101, 103–104, 112, 112, 112, 112, 112–113, 115–116, 123–124, 126, 128–132, 141, 145–146, 146, 154, 154, 154–157, 159–162, 166–167, 169, 171–174, 177, 184–185, 187, 189–190, 192, 199–200, 202–203, 206, 206, 213, 213, 217, 22–23, 25–27, 27, 27–29, 33, 42, 49, 56, 63, 70, 89–91, 99
src/canvas
TextEditingManager.ts	100%	100%	100%	100%
canvas.class.ts	93.75%	90.49%	94.12%	96.09%	1143, 1143–1144, 1147, 1167, 1167, 1226, 1276–1277, 1298, 1306, 1419–1420, 1422–1423, 1443–1444, 562–563, 568, 578, 711–712, 714–715, 715, 715, 761–762, 823–824, 877–879, 909, 914–915, 944–945
canvas_events.ts	78.85%	76.85%	83.33%	79.58%	1010–1011, 1011, 1011–1013, 1015–1016, 1016, 1016, 1018, 1026, 1026, 1026–1028, 1028, 1028, 1034–1035, 1043–1044, 1044, 1044–1045, 1050, 1052, 1083–1085, 1088–1089, 1093–1094, 1211–1213, 1216–1217, 1290, 1410, 1504–1505, 1511, 1515–1516, 1532, 1554, 1601, 1606, 1654, 172, 197, 306–307, 310–314, 319, 342–343, 348–353, 373, 373, 373–374, 374, 374–375, 383, 388–389, 389, 389–390, 392, 401, 407–408, 408, 408, 451, 459, 463, 463, 463–464, 466, 548–549, 549, 549–550, 556, 556, 556–558, 578, 580, 580, 580–581, 581, 581, 584, 584, 584–585, 588, 597–598, 600–601, 603, 603–604, 606–607, 619–620, 620, 620–621, 623–627, 633, 640, 680, 680, 680, 682, 684–688, 694, 700, 700, 700–701, 703, 706, 711, 724, 751, 751, 809–810, 810, 810–811, 813, 816–817, 817, 817–818, 820–821, 824, 824–826, 829–830, 900, 912, 919, 940, 972, 993–994
static_canvas.class.ts	94.47%	88.89%	97.96%	96.96%	1086–1087, 1087, 1087–1088, 1208, 1218, 1272–1273, 1276, 1311–1312, 1389, 1398, 1398, 1402, 1402, 1449–1450, 1667, 1667–1668, 1717, 1720, 1723, 1723, 1723, 1726, 1729, 1729, 1729, 285–286, 383–384, 386–387, 748, 760–761, 846
src/color
color.class.ts	92.16%	86.49%	100%	94.29%	330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	100%	100%	100%	100%
src/controls
changeWidth.ts	100%	100%	100%	100%
control.class.ts	93.90%	88.89%	90.91%	97.73%	235, 319, 319, 354
controls.render.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
default_controls.ts	86.67%	66.67%	100%	100%	122, 129
drag.ts	100%	100%	100%	100%
polyControl.ts	6.35%	0%	0%	11.43%	100, 105, 119, 121–124, 124, 127, 134, 17, 25–28, 30, 30, 30, 30, 30, 30, 30, 30, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 83, 83–84, 88–90, 90, 90, 90, 90, 92
rotate.ts	20%	12.50%	50%	22.22%	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, 73–74, 76, 78, 80–81
scale.ts	93.41%	92.68%	100%	93.59%	129–130, 132–134, 148–149, 181–183, 42
scaleSkew.ts	78.79%	64.29%	100%	85.71%	27, 29, 29, 29, 31, 33, 35
skew.ts	91.03%	79.31%	100%	97.67%	130–131, 162–163, 170, 176, 178
util.ts	100%	100%	100%	100%
wrapWithFireEvent.ts	100%	100%	100%	100%
wrapWithFixedAnchor.ts	100%	100%	100%	100%
src/filters
2d_backend.class.ts	92%	83.33%	100%	93.75%	35–36
FilterBackend.ts	88.89%	88.89%	100%	85.71%	15–16
WebGLProbe.ts	37.14%	40%	60%	30%	28–30, 30, 30–31, 33–35, 43, 46–48, 48, 48–51, 53, 58
base_filter.class.ts	19.88%	20.83%	30%	17.35%

[asturur]

to add context around this, i think is because we mutate more state other than isEditing when going editing mode.
I ll open a task to fix it seems it seems just wrong as a pattern.

[ShaMan123]

I agree

[asturur]

in canvas file we define this function

function checkClick(e: TPointerEvent, value: number) {
  return !!(e as MouseEvent).button && (e as MouseEvent).button === value - 1;
}

So on top of reusing it, i wonder why there we use value - 1 and here were are using value directly.

[asturur]

Answer i found later: because here we are not inspecting e.button directly.

[ShaMan123]

it was previously e.button.
checkClick is used on the native event before the synthetic event is fired by fabric. The synthetic event has the button attr set and is e.button+1. It is far from ideal that that values aren't identical. But that is what it is.
I thought we should use the button attr of the synthetic event or use checkClick and made it clear we are checking
to avoid a right click.

[asturur]

Ok somethin weird is going on here.
The old code was introduced 6 years ago to guard against NON LEFT CLICKS.
That was the intention.
The code seems wrong in that regard since the value of button for left clicks is 0 and so checking for !==1 was plain wrong.
On top of that UTs have mocked events so not really reliable.
To avoid mixing types with changes, i just kept the exact code that was there before, but what this code is doing should be investigated.

[asturur]

here the context:
https://github.com/fabricjs/fabric.js/pull/3888/files#diff-ca85fc40679e7fc083b650fbb7e7851f09d84a83da4401ad2ec326f57e31febaR86

[asturur]

i see 2 things here ( and on _mouseDownHandlerBefore ):

• we move to evaluate the button option rather than the event.button.
• we changed from a not a left click to a is a right click that leaves the middle click and other possible clicks in an new state ( i would definitely avoid that ).

On top of that it looks like we expose on the event this button value that is not the real button value from the event but more a fabricjs custom value. I wonder if we should remove that value completely and let people look at the event itself

[asturur]

ok seems like we used to look at button because buttons wasn't on IE. But that is a topic or another day.
I think the only important thing here is to change this isRightClick with a !isLeftClick as it was before so we don't change behaviour

[ShaMan123]

I didn't mean to change behavior. It was unclear to me

[ShaMan123]

(e.button && e.button !== 1) means to me:
value is not 0 (not left click) and not middle click => is right click
Why middle click is alright I didn't understand.

[ShaMan123]

I think it is best you do it

[asturur]

I just don't want to chane the behavioru in a typing PR.
I think the reason of being not a left click is because by default fabric wasn't doing anything else with other clicks ( other than allowing for event firing ). So as long as it matters to us, right, middle and the other 2 clicks ( called 4 and 5 that are in some custom mouse ) we just don't do anythin.

[asturur]

Let me know if you have time to address that rightclick vs not left click, so that then i can merge.
If you don't have time for it i can do it tomorrow.

[ShaMan123]

BTW I wanted to move all IText classes under a folder same as Object, makes sense IMO
Text (with text styles) to a folder of itself.
In general are we going to rename files before beta? I think we should.
itext_click_behavior.mixin => ITextClickBehavior

[asturur]

Renaming files or moving them is not a huge issue for me at this point in changes