fix(Canvas): toDataURL and toCanvasElement do not draw controls by default (skipControlsDrawing)

[asturur]

Description

close #9846

Add an override for the canvas to hide the active object controls
Left as an option.

This will be completely useless when controls will be moved on the upper canvas.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

• Run Next Template
• Run Node Template
• Run Vanilla Template
• Run Vue Template

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	919.447 (+0.484)	306.395 (+0.159)

[jiayihu]

This assumes that only the activeObj has active controls, which is true for vanilla fabric but easily not-so in apps. And it feels hacky anyway. The way instead we did was to use the now-removed interactive property when exporting the canvas and skip control rendering if true. For instance skipping canvas.drawControls().

[asturur]

but this.interactive does not exist anymore.
The issue is in the renderCanvas method that because render the controls in 2 moments of the function is not easily divisible in the static vs non static part.

I could also just add interactive back, it would be as hacky has this but less code probably

[jiayihu]

I don't think it's hacky as ultimately you need a condition to tell if the rendering is happening because of export and differentiate based on that. It can be called interactive but also exporting/static whatever

[asturur]

or could be called protected shouldRenderControls since is being built just for that

[asturur]

It is a workaround as the other because the real issue is that the StaticCanvas shouldn't have an empty drawControls method to call because renderCanvas isn't correctly split between the 2 classes. If it was i could just call the part that does not render the controls

We do not need a boolean to render controls if not for this issue here. In that sense are both workarounds

[jiayihu]

We can also say that it should be left to the app deciding what to do. If you're using SelectableCanvas and do not want to render controls, then just discard the activeObj before toCanvasElement().

I think determining when to render controls in general is highly app-specific.

[asturur]

Yes all good points, but when i have a sort of regression, i patch the issue, i take notes for improvement, but i don't like to do both at once. In this case the boolean will take care of both, but is a new feature introduced for a fix, and that is smells of problems to me, always.

[github-actions]

Coverage after merging toCanvasElement-controls into master will be

84.51%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	14.29%	100%	0%	25%	23, 26, 29, 41, 44, 47
src
ClassRegistry.ts	90.32%	61.54%	100%	97.73%	31, 52
Collection.ts	78.47%	42.62%	87.10%	85.82%	130, 138, 153, 155–157, 159, 169–170, 181, 197, 215, 217, 228, 243, 254, 265, 270, 279, 281, 286–287, 302, 304, 309–310, 329, 333–334, 338–344, 346–348, 350
CommonMethods.ts	91.43%	71.43%	100%	96%	50, 52
Intersection.ts	85.25%	48.91%	100%	97.30%	184–188, 190, 228, 237, 239, 289, 297, 297
Observable.ts	79.89%	54.55%	93.75%	87.10%	136, 145, 148, 160, 162, 167, 68–70, 72, 76, 80, 84–85, 87–91
Point.ts	90.27%	61.22%	100%	93.60%	104, 117, 148, 157, 179, 197, 206, 216, 225, 236–239, 259, 285, 297, 317, 328, 341, 349, 359, 95
Shadow.ts	87.85%	78.26%	100%	88.37%	147, 150, 152–157, 166, 203, 206, 213, 230–237, 241–242, 38–41
cache.ts	84.88%	45.45%	100%	90.14%	57, 59, 71–72, 74–77
config.ts	87.73%	55%	66.67%	94.03%	132, 134–137, 139, 142–143, 147, 152
constants.ts	100%	100%	100%	100%
src/LayoutManager
ActiveSelectionLayoutManager.ts	93.33%	76.92%	85.71%	100%
LayoutManager.ts	90.54%	65.06%	76.92%	99.29%	269, 333, 344
constants.ts	100%	100%	100%	100%
index.ts	48.57%	37.50%	80%	66.67%	1, 1, 1–2, 2, 2–3, 3, 3–4, 4, 4–5, 5, 5, 5–6
types.ts	100%	100%	100%	100%
src/LayoutManager/LayoutStrategies
ClipPathLayout.ts	73.08%	50%	100%	78.95%	39, 41–44, 46–48, 57–58, 66–69
FitContentLayout.ts	100%	100%	100%	100%
FixedLayout.ts	85.71%	20%	100%	100%	23, 23
LayoutStrategy.ts	87.60%	55.56%	100%	96.55%	46, 54, 72, 72, 74
utils.ts	72.58%	50%	100%	78.72%	29–32, 34–35, 40–44
src/Pattern
Pattern.ts	70.18%	90.91%	80%	65.95%	105–107, 114, 118–119, 119–122, 130–138, 140–141, 143, 153–164, 174, 176–181, 183–188, 190–199, 204–205, 207–209, 211, 33, 37
src/brushes
BaseBrush.ts	89.33%	91.67%	100%	88.55%	110, 115, 124–125, 130, 135, 143, 146, 155–160, 99
CircleBrush.ts	52.10%	12.50%	12.50%	58.25%	100–108, 108–118, 122, 130–139, 55, 67, 69, 76, 76, 78–79, 79, 83, 85–86, 92–98
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	75.49%	39.76%	92.86%	84.92%	1004, 1007–1008, 1012–1013, 1018, 1063–1068, 1105, 1126, 1128, 1130, 1137, 1140, 1203–1207, 1286–1290, 1320, 1337, 1383–1400, 1406–1411, 1414–1415, 1417, 1421, 1423–1424, 1426–1428, 1432, 1434, 1436–1438, 1441–1446, 1449–1451, 1454, 1456, 1470, 1477, 1479–1490, 1492–1495, 1495, 1497, 1501–1502, 1505–1506, 1509–1511, 1514, 1522, 354, 369, 388, 443, 559–563, 566–567, 569, 579, 582–583, 585, 588–590, 602, 609–613, 615–620, 622–626, 659, 661, 668–672, 674–679, 681, 683–684, 686–689, 691–692, 747, 783–786, 789, 791, 794, 796, 798, 824, 886–887, 932–933, 935–936, 938, 947–953, 956, 963–964, 966–970, 972, 974, 995–996, 999
CanvasOptions.ts	100%	100%	100%	100%
SelectableCanvas.ts	85.35%	53.54%	100%	94.34%	1012, 1021, 1089–1093, 1139–1140, 1142–1143, 1145, 1172, 1174–1175, 1175, 1177, 1216, 1218, 1220, 1236, 1238, 1242, 1245, 1266, 1269, 1288, 1290, 1294, 1312–1319, 1322, 1325, 1334–1335, 1339–1348, 1352, 1354–1355, 1361–1366, 1370, 362, 384, 462, 513, 515, 591, 596, 673, 701, 992, 994–995
StaticCanvas.ts	75.88%	70.19%	98.91%	75.36%	1008, 1013, 1019, 1022, 1024, 1026, 1034, 1036–1042, 1052, 1055, 1058–1064, 1066–1067, 1069–1090, 1097–1099, 1101, 1109, 1115, 1117, 1117–1118, 1118–1119, 1121, 1121–1126, 1139, 1144–1146, 1150, 1154, 1156–1158, 1163–1167, 1169, 1171–1174, 1177, 1179, 1188, 1190–1191, 1198–1201, 1203, 1209–1212, 1216–1217, 1227, 1233–1235, 1237–1242, 1242, 1242–1246, 1246, 1246–1251, 1253–1260, 1289–1290, 1292–1293, 1295–1297, 1299–1305, 1309, 1311–1324, 1332–1333, 1343, 1354, 1395–1399, 1401, 1403, 1405–1409, 1428, 1444, 1461, 1481, 1514,