Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(TS): finalize interactivity #8404

Merged
merged 39 commits into from Jan 16, 2023
Merged

chore(TS): finalize interactivity #8404

merged 39 commits into from Jan 16, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Oct 31, 2022

Motivation

Description

Finalize migrating interactivity and controls since we did it in parallel
#8400 #8401

Changes

_setCornerCoords => _calcCornerCoords
__corner type is not 0 anymore, string or undefined

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2022

Build Stats

file / KB (diff) bundled minified
fabric 937.760 (+0.144) 295.748 (-0.035)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2022

Coverage after merging finalize-interactivity into master will be

83.17%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.ts100%100%100%100%
src
   cache.ts96.97%90%100%100%56
   config.ts77.27%66.67%66.67%84.62%130, 138–140, 151–153
   constants.ts100%100%100%100%
   env.ts72.73%53.33%100%79.17%22, 22–23, 23, 23, 25, 25, 27, 29, 31–32, 62
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.19%85.71%100%96.30%118, 124, 135, 144, 96
   point.class.ts100%100%100%100%
   shadow.class.ts98.48%96%100%100%156
   typedefs.ts100%100%100%100%
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts0%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.ts97.06%87.50%100%100%21
   pencil_brush.class.ts91.81%85.42%100%93.52%122–123, 152, 152–154, 276, 280, 285–286, 68–69, 84–85
   spray_brush.class.ts0%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.ts100%100%100%100%
   canvas.class.ts93.68%90.42%94%96.05%1137, 1137–1138, 1141, 1161, 1161, 1220, 1270–1271, 1292, 1300, 1413–1414, 1416–1417, 1437–1438, 556–557, 562, 572, 705–706, 708–709, 709, 709, 755–756, 817–818, 871–873, 903, 908–909, 938–939
   canvas_events.ts78.83%76.63%83.33%79.69%1012–1013, 1013, 1013–1015, 1017–1018, 1018, 1018, 1020, 1028, 1028, 1028–1030, 1030, 1030, 1036–1037, 1045–1046, 1046, 1046–1047, 1052, 1054, 1085–1087, 1090–1091, 1095–1096, 1213–1215, 1218–1219, 1292, 1412, 1506–1507, 1513, 1517–1518, 1534, 1556, 1603, 1608, 1656, 175, 200, 309–310, 313–317, 322, 345–346, 351–356, 376, 376, 376–377, 377, 377–378, 386, 391–392, 392, 392–393, 395, 404, 410–411, 411, 411, 454, 462, 466, 466, 466, 466, 466–467, 469, 551–552, 552, 552–553, 559, 559, 559–561, 581, 583, 583, 583–584, 584, 584, 587, 587, 587–588, 591, 600–601, 603–604, 606, 606–607, 609–610, 622–623, 623, 623–624, 626–630, 636, 643, 683, 683, 683, 685, 687–691, 697, 703, 703, 703–704, 706, 709, 714, 727, 754, 811–812, 812, 812–813, 815, 818–819, 819, 819–820, 822–823, 826, 826–828, 831–832, 902, 914, 921, 942, 974, 995–996
   static_canvas.class.ts94.80%89.88%97.92%97.01%1089–1090, 1090, 1090–1091, 1211, 1221, 1275–1276, 1279, 1314–1315, 1393, 1402, 1407, 1456–1457, 1685, 1685–1686, 1735, 1738, 1741, 1741, 1741, 1744, 1747, 1747, 1747, 286–287, 384–385, 387–388, 763, 763–764, 849
src/color
   color.class.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   changeWidth.ts100%100%100%100%
   control.class.ts93.90%88.89%90.91%97.73%235, 319, 319, 354
   controls.render.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   default_controls.ts86.67%66.67%100%100%122, 129
   drag.ts100%100%100%100%
   polyControl.ts6.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.ts20%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.ts93.41%92.68%100%93.59%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts78.79%64.29%100%85.71%27, 29, 29, 29, 31, 33, 35
   skew.ts91.03%79.31%100%97.67%130–131, 162–163, 170, 176, 178
   util.ts100%100%100%100%
   wrapWithFireEvent.ts100%100%100%100%
   wrapWithFixedAnchor.ts100%100%100%100%
src/filters
   2d_backend.class.ts92%83.33%100%93.75%35–36
   FilterBackend.ts88.89%88.89%100%85.71%15–16
   WebGLProbe.ts37.14%40%60%30%28–30, 30, 30–31, 33–35, 43, 46–48, 48, 48–51, 53, 58
   base_filter.class.ts20.83%20.83%33.33%18.18%

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READY

src/mixins/object_interactivity.mixin.ts Outdated Show resolved Hide resolved
fabricObject: FabricObject,
currentControl: Control
) {
positionHandler(dim: Point, finalMatrix: TMat2D, fabricObject: FabricObject) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see prev comment

* @todo evaluate simplification of code switching to circle interaction area at runtime
* @private
*/
_setCornerCoords(): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored into _calcCornerCoords() in a revertable commit 5afe4c6
reasons:

  • cleaner/readable code
  • better typed
  • perf (one less iteration, kept in a contained method since I saw you wrote it might be removed)
  • since it is private, considered not breaking

* @todo evaluate simplification of code switching to circle interaction area at runtime
* @private
*/
private _calcCornerCoords(control: Control, position: Point) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see prev comment


type TOCoord = IPoint & {
type TOCoord = Point & {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure we use it as a Point in fabric

@ShaMan123
Copy link
Contributor Author

ready

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asturur this is waiting too long

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated from master

);
this.forEachControl((control, key) => {
const position = control.positionHandler(dim, finalMatrix, this, control);
coords[key] = Object.assign(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using assign since I think we use it as Point as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will clarify this with a comment. so it doesn't get swapped with a spread operator

@asturur asturur merged commit a3631b6 into master Jan 16, 2023
@ShaMan123 ShaMan123 deleted the finalize-interactivity branch January 16, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants