Skip to content

Commit ba4e54a

Browse files
authored
WFPREV-827 Prevent stuck map pin when popup is displayed and filter change removes project from list (#1064)
1 parent 75f8f94 commit ba4e54a

File tree

2 files changed

+169
-2
lines changed

2 files changed

+169
-2
lines changed

client/wfprev-war/src/main/angular/src/app/components/map/map.component.spec.ts

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ describe('MapComponent', () => {
131131
getCenter: jasmine.createSpy('getCenter'),
132132
setView: jasmine.createSpy('setView'),
133133
fitBounds: jasmine.createSpy('fitBounds'),
134+
closePopup: jasmine.createSpy('closePopup'),
134135
controls: { bottomleft: { addTo: jasmine.createSpy('addTo') } },
135136
},
136137
layerIds: ['a', 'b'],
@@ -147,6 +148,7 @@ describe('MapComponent', () => {
147148
(L as any).markerClusterGroup = () => ({
148149
addLayer: jasmine.createSpy('addLayer'),
149150
clearLayers: jasmine.createSpy('clearLayers'),
151+
getLayers: jasmine.createSpy('getLayers').and.returnValue([]),
150152
});
151153
spyOn(L.Control.prototype, 'addTo').and.callFake(function (this: any) {
152154
return this;
@@ -557,11 +559,13 @@ describe('MapComponent', () => {
557559
mockMap = {
558560
addLayer: jasmine.createSpy(),
559561
removeLayer: jasmine.createSpy(),
562+
closePopup: jasmine.createSpy(),
560563
};
561564
mapServiceMock.getSMKInstance.and.returnValue({ $viewer: { map: mockMap } });
562565
(component as any).markersClusterGroup = {
563566
clearLayers: jasmine.createSpy(),
564567
addLayer: jasmine.createSpy(),
568+
getLayers: jasmine.createSpy().and.returnValue([]),
565569
};
566570
spyOn(console, 'log');
567571
spyOn(console, 'error');
@@ -657,6 +661,7 @@ describe('MapComponent', () => {
657661
addLayer: jasmine.createSpy('addLayer'),
658662
removeLayer: jasmine.createSpy('removeLayer'),
659663
hasLayer: jasmine.createSpy('hasLayer').and.returnValue(false),
664+
closePopup: jasmine.createSpy('closePopup')
660665
};
661666
// make SMK return our mock map
662667
mapServiceMock.getSMKInstance.and.returnValue({ $viewer: { map: mockMap } });
@@ -665,6 +670,8 @@ describe('MapComponent', () => {
665670
(component as any).markersClusterGroup = {
666671
clearLayers: jasmine.createSpy('clearLayers'),
667672
addLayer: jasmine.createSpy('addLayer'),
673+
closePopup: jasmine.createSpy('closePopup'),
674+
getLayers: jasmine.createSpy('getLayers').and.returnValue([]),
668675
};
669676

670677

@@ -676,7 +683,7 @@ describe('MapComponent', () => {
676683
it('creates project & activity layers with unique projectGuids and adds them to map', () => {
677684
const locs = [
678685
{ projectGuid: 'a', latitude: 1, longitude: 2 },
679-
{ projectGuid: 'a', latitude: 3, longitude: 4 },
686+
{ projectGuid: 'a', latitude: 3, longitude: 4 },
680687
{ projectGuid: 'b', latitude: 5, longitude: 6 },
681688
{ projectGuid: undefined, latitude: 7, longitude: 8 }, // invalid -> filtered out
682689
];
@@ -749,5 +756,130 @@ describe('MapComponent', () => {
749756
});
750757
});
751758

759+
describe('teardownActiveUI()', () => {
760+
let mapMock: any;
761+
762+
beforeEach(() => {
763+
mapMock = { closePopup: jasmine.createSpy('closePopup') };
764+
mapServiceMock.getSMKInstance.and.returnValue({ $viewer: { map: mapMock } });
765+
});
766+
767+
it('closes global popup and removes active marker via cluster group when present', () => {
768+
const activeMarkerSpy = jasmine.createSpyObj<L.Marker>('Marker', [
769+
'unbindPopup', 'remove'
770+
]);
771+
Object.setPrototypeOf(activeMarkerSpy, L.Marker.prototype);
772+
773+
const clusterMock = {
774+
addLayer: jasmine.createSpy('addLayer'),
775+
clearLayers: jasmine.createSpy('clearLayers'),
776+
removeLayer: jasmine.createSpy('removeLayer'),
777+
hasLayer: jasmine.createSpy('hasLayer').and.returnValue(true),
778+
getLayers: jasmine.createSpy('getLayers').and.returnValue([]),
779+
};
780+
781+
(component as any).markersClusterGroup = clusterMock as any;
782+
(component as any).activeMarker = activeMarkerSpy;
783+
784+
(component as any).teardownActiveUI();
785+
786+
expect(mapMock.closePopup).toHaveBeenCalled();
787+
expect(activeMarkerSpy.unbindPopup).toHaveBeenCalled();
788+
expect(clusterMock.removeLayer).toHaveBeenCalledWith(activeMarkerSpy);
789+
expect((component as any).activeMarker).toBeNull();
790+
});
791+
792+
it('closes global popup and removes active marker directly when not in cluster', () => {
793+
const activeMarkerSpy = jasmine.createSpyObj<L.Marker>('Marker', [
794+
'unbindPopup', 'remove'
795+
]);
796+
Object.setPrototypeOf(activeMarkerSpy, L.Marker.prototype);
797+
798+
const clusterMock = {
799+
addLayer: jasmine.createSpy('addLayer'),
800+
clearLayers: jasmine.createSpy('clearLayers'),
801+
removeLayer: jasmine.createSpy('removeLayer'),
802+
hasLayer: jasmine.createSpy('hasLayer').and.returnValue(false),
803+
getLayers: jasmine.createSpy('getLayers').and.returnValue([]),
804+
};
805+
806+
(component as any).markersClusterGroup = clusterMock as any;
807+
(component as any).activeMarker = activeMarkerSpy;
808+
809+
(component as any).teardownActiveUI();
810+
811+
expect(mapMock.closePopup).toHaveBeenCalled();
812+
expect(activeMarkerSpy.unbindPopup).toHaveBeenCalled();
813+
expect(activeMarkerSpy.remove).toHaveBeenCalled();
814+
expect(clusterMock.removeLayer).not.toHaveBeenCalled();
815+
expect((component as any).activeMarker).toBeNull();
816+
});
817+
});
818+
819+
describe('updateProjectMarkersFromLocations() teardown before clear', () => {
820+
it('unbinds/closes popups and removes listeners for each marker before clearing cluster', () => {
821+
const mapMock = { addLayer: () => { }, removeLayer: () => { }, closePopup: () => { } };
822+
mapServiceMock.getSMKInstance.and.returnValue({ $viewer: { map: mapMock } });
823+
824+
const m1 = jasmine.createSpyObj<L.Marker>('Marker', ['unbindPopup', 'closePopup', 'off']);
825+
const m2 = jasmine.createSpyObj<L.Marker>('Marker', ['unbindPopup', 'closePopup', 'off']);
826+
Object.setPrototypeOf(m1, L.Marker.prototype);
827+
Object.setPrototypeOf(m2, L.Marker.prototype);
828+
829+
(component as any).markersClusterGroup = {
830+
addLayer: jasmine.createSpy('addLayer'),
831+
clearLayers: jasmine.createSpy('clearLayers'),
832+
removeLayer: jasmine.createSpy('removeLayer'),
833+
hasLayer: jasmine.createSpy('hasLayer').and.returnValue(false),
834+
getLayers: jasmine.createSpy('getLayers').and.returnValue([m1, m2]),
835+
} as any;
836+
837+
(component as any).updateProjectMarkersFromLocations([]);
838+
839+
expect(m1.unbindPopup).toHaveBeenCalled();
840+
expect(m1.closePopup).toHaveBeenCalled();
841+
expect(m1.off).toHaveBeenCalled();
842+
843+
expect(m2.unbindPopup).toHaveBeenCalled();
844+
expect(m2.closePopup).toHaveBeenCalled();
845+
expect(m2.off).toHaveBeenCalled();
846+
847+
expect((component as any).markersClusterGroup.clearLayers).toHaveBeenCalled();
848+
});
849+
});
850+
851+
describe('filtering removes selected/active project', () => {
852+
it('tears down active marker and closes map popup when selected project is no longer in results', () => {
853+
const mapMock = {
854+
addLayer: jasmine.createSpy('addLayer'),
855+
removeLayer: jasmine.createSpy('removeLayer'),
856+
closePopup: jasmine.createSpy('closePopup'),
857+
};
858+
mapServiceMock.getSMKInstance.and.returnValue({ $viewer: { map: mapMock } });
859+
860+
const activeMarkerSpy = jasmine.createSpyObj<L.Marker>('Marker', ['unbindPopup', 'remove']);
861+
Object.setPrototypeOf(activeMarkerSpy, L.Marker.prototype);
862+
863+
const clusterMock = {
864+
addLayer: jasmine.createSpy('addLayer'),
865+
clearLayers: jasmine.createSpy('clearLayers'),
866+
removeLayer: jasmine.createSpy('removeLayer'),
867+
hasLayer: jasmine.createSpy('hasLayer').and.returnValue(true),
868+
getLayers: jasmine.createSpy('getLayers').and.returnValue([]),
869+
};
870+
871+
(component as any).markersClusterGroup = clusterMock as any;
872+
(component as any).activeMarker = activeMarkerSpy;
873+
(component as any).selectedProject = { projectGuid: 'to-remove' };
874+
875+
(component as any).updateProjectMarkersFromLocations([
876+
{ projectGuid: 'test-guid', latitude: 1, longitude: 2 },
877+
]);
878+
879+
expect(mapMock.closePopup).toHaveBeenCalled();
880+
expect(clusterMock.removeLayer).toHaveBeenCalledWith(activeMarkerSpy);
881+
expect((component as any).activeMarker).toBeNull();
882+
});
883+
});
752884

753885
});

client/wfprev-war/src/main/angular/src/app/components/map/map.component.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ export class MapComponent implements AfterViewInit, OnDestroy {
267267
}
268268

269269
private updateProjectMarkersFromLocations(locations: ProjectLocation[]): void {
270+
this.teardownActiveUI();
271+
270272
const smk = this.mapService.getSMKInstance();
271273
const map = smk?.$viewer?.map;
272274

@@ -275,7 +277,16 @@ export class MapComponent implements AfterViewInit, OnDestroy {
275277
return;
276278
}
277279

278-
this.markersClusterGroup.clearLayers();
280+
if (this.markersClusterGroup) {
281+
this.markersClusterGroup.getLayers().forEach(l => {
282+
if (l instanceof L.Marker) {
283+
l.closePopup();
284+
l.unbindPopup();
285+
l.off();
286+
}
287+
});
288+
this.markersClusterGroup.clearLayers();
289+
}
279290
this.projectMarkerMap.clear();
280291
this.activeMarker = null;
281292

@@ -429,4 +440,28 @@ export class MapComponent implements AfterViewInit, OnDestroy {
429440
});
430441
}
431442

443+
// Close any open popup and fully remove the active marker so no DOM is orphaned
444+
private teardownActiveUI(): void {
445+
const smk = this.mapService.getSMKInstance();
446+
const map: L.Map | undefined = smk?.$viewer?.map;
447+
448+
map?.closePopup();
449+
450+
// Remove the active marker and unbind popup to drop DOM from panes
451+
try {
452+
if (this.activeMarker) {
453+
this.activeMarker.unbindPopup();
454+
if (this.markersClusterGroup?.hasLayer(this.activeMarker)) {
455+
this.markersClusterGroup.removeLayer(this.activeMarker);
456+
} else {
457+
this.activeMarker.remove();
458+
}
459+
}
460+
} catch (err) {
461+
console.error('Error during teardown:', err);
462+
} finally {
463+
this.activeMarker = null;
464+
}
465+
}
466+
432467
}

0 commit comments

Comments
 (0)