Skip to content

Commit fe81169

Browse files
authored
Merge pull request #781 from WHOIM1205/fix/event-listener-cleanup
Fix: Prevent Event Listener Leaks and Duplicate Callbacks in ThreeManager
2 parents 7a528d6 + 46eaff2 commit fe81169

File tree

1 file changed

+49
-13
lines changed
  • packages/phoenix-event-display/src/managers/three-manager

1 file changed

+49
-13
lines changed

packages/phoenix-event-display/src/managers/three-manager/index.ts

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,12 @@ export class ThreeManager {
9494
elem: Intersection<Object3D<Object3DEventMap>>,
9595
) => boolean;
9696
/** 'click' event listener callback to show 3D coordinates of the clicked point */
97-
private show3DPointsCallback: (event: MouseEvent) => void = () => {};
97+
private show3DPointsCallback: ((event: MouseEvent) => void) | null = null;
9898
/** 'click' event listener callback to shift the cartesian grid at the clicked point */
99-
private shiftCartesianGridCallback: (event: MouseEvent) => void = () => {};
99+
private shiftCartesianGridCallback: ((event: MouseEvent) => void) | null =
100+
null;
100101
/** 'click' event listener callback to show 3D distance between two clicked points */
101-
private show3DDistanceCallback: (event: MouseEvent) => void = () => {};
102+
private show3DDistanceCallback: ((event: MouseEvent) => void) | null = null;
102103

103104
/** Origin of the cartesian grid w.r.t. world origin */
104105
public origin: Vector3 = new Vector3(0, 0, 0);
@@ -123,7 +124,7 @@ export class ThreeManager {
123124
/** Color of the text to be displayed as per dark theme */
124125
private displayColor: string = 'black';
125126
/** Mousemove callback to draw dynamic distance line */
126-
private mousemoveCallback: (event: MouseEvent) => void = () => {};
127+
private mousemoveCallback: ((event: MouseEvent) => void) | null = null;
127128
/** Stored keydown handler for cleanup. */
128129
private keydownHandler: ((e: KeyboardEvent) => void) | null = null;
129130
/** Emitting that a new 3D coordinate has been clicked upon */
@@ -503,9 +504,9 @@ export class ThreeManager {
503504
};
504505
}
505506

506-
if (show) {
507+
if (show && this.show3DPointsCallback) {
507508
window.addEventListener('click', this.show3DPointsCallback);
508-
} else {
509+
} else if (this.show3DPointsCallback) {
509510
window.removeEventListener('click', this.show3DPointsCallback);
510511
}
511512
}
@@ -566,9 +567,13 @@ export class ThreeManager {
566567
ctx.fill();
567568
}
568569

569-
window.addEventListener('mousemove', this.mousemoveCallback);
570+
if (this.mousemoveCallback) {
571+
window.addEventListener('mousemove', this.mousemoveCallback);
572+
}
570573
} else {
571-
window.removeEventListener('mousemove', this.mousemoveCallback);
574+
if (this.mousemoveCallback) {
575+
window.removeEventListener('mousemove', this.mousemoveCallback);
576+
}
572577
const distance =
573578
mainIntersect.point.distanceTo(this.prev3DCoord) / 10;
574579

@@ -642,11 +647,15 @@ export class ThreeManager {
642647
};
643648
}
644649

645-
if (show) {
650+
if (show && this.show3DDistanceCallback) {
646651
window.addEventListener('click', this.show3DDistanceCallback);
647652
} else {
648-
window.removeEventListener('click', this.show3DDistanceCallback);
649-
window.removeEventListener('mousemove', this.mousemoveCallback);
653+
if (this.show3DDistanceCallback) {
654+
window.removeEventListener('click', this.show3DDistanceCallback);
655+
}
656+
if (this.mousemoveCallback) {
657+
window.removeEventListener('mousemove', this.mousemoveCallback);
658+
}
650659
if (document.getElementById('3Ddistance') != null) {
651660
document.getElementById('3Ddistance')?.remove();
652661
}
@@ -705,13 +714,17 @@ export class ThreeManager {
705714
}
706715

707716
const rightClickCallback = (_event: any) => {
708-
window.removeEventListener('click', this.shiftCartesianGridCallback);
717+
if (this.shiftCartesianGridCallback) {
718+
window.removeEventListener('click', this.shiftCartesianGridCallback);
719+
}
709720
this.stopShifting.emit(true);
710721
this.shiftGrid = false;
711722
window.removeEventListener('contextmenu', rightClickCallback);
712723
};
713724

714-
window.addEventListener('click', this.shiftCartesianGridCallback);
725+
if (this.shiftCartesianGridCallback) {
726+
window.addEventListener('click', this.shiftCartesianGridCallback);
727+
}
715728
window.addEventListener('contextmenu', rightClickCallback);
716729
}
717730

@@ -1730,6 +1743,29 @@ export class ThreeManager {
17301743
this.keydownHandler = null;
17311744
}
17321745

1746+
// Remove window event listeners for interactive features
1747+
if (this.show3DPointsCallback) {
1748+
window.removeEventListener('click', this.show3DPointsCallback);
1749+
this.show3DPointsCallback = null;
1750+
}
1751+
if (this.show3DDistanceCallback) {
1752+
window.removeEventListener('click', this.show3DDistanceCallback);
1753+
this.show3DDistanceCallback = null;
1754+
}
1755+
if (this.mousemoveCallback) {
1756+
window.removeEventListener('mousemove', this.mousemoveCallback);
1757+
this.mousemoveCallback = null;
1758+
}
1759+
if (this.shiftCartesianGridCallback) {
1760+
window.removeEventListener('click', this.shiftCartesianGridCallback);
1761+
this.shiftCartesianGridCallback = null;
1762+
}
1763+
1764+
// Clean up any dangling DOM elements from interactive features
1765+
document.getElementById('3dcoordinates')?.remove();
1766+
document.getElementById('circledDot')?.remove();
1767+
document.getElementById('3Ddistance')?.remove();
1768+
17331769
// Cleanup sub-managers
17341770
if (this.rendererManager) {
17351771
this.rendererManager.cleanup();

0 commit comments

Comments
 (0)