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

Fix NG0953 caused by event errors #208

Merged
merged 3 commits into from
Nov 4, 2024
Merged

Fix NG0953 caused by event errors #208

merged 3 commits into from
Nov 4, 2024

Conversation

HarelM
Copy link
Collaborator

@HarelM HarelM commented Nov 3, 2024

Fixes #193 by adding a subscription to all the events and unsubscribing when removing the relevant object.
I think this might cause some issue with the popup events, but I'm not sure which scenario is the right one and how it should behave.
In any case I think it's better than failing to present a popup.

@HarelM HarelM changed the title Fix event errors Fix NG0953 caused by event errors Nov 4, 2024
@HarelM HarelM merged commit f4cc209 into main Nov 4, 2024
2 checks passed
@HarelM HarelM deleted the fix-event-errors branch November 4, 2024 21:28
@marcjulian
Copy link
Collaborator

marcjulian commented Nov 6, 2024

I testet the latest release 18.1.3 with this fix. In my app the error #193 is still present. I will try to provide a reproduction, that we can fix it.

CleanShot 2024-11-06 at 10 31 24
CleanShot 2024-11-06 at 10 31 17

@HarelM
Copy link
Collaborator Author

HarelM commented Nov 6, 2024

It might be the issue that I saw with the destroy with the marker and popup.
The main issue I solved is around layers, I think.
Can you check if you align the ngDestory stuff this issue is resolved?
Also, I think there is a situation I was able to reproduce where both "if"s in the popup destroy are false and the popup unregistering is not called, see here - (the attached marker is null, and feature and lnglat are not defined since the popup is attached to a marker):

removePopupFromMarker() {
if (this.popupInstance) {
const markerInstance = this.marker()?.markerInstance();
if (this.lngLat() || this.feature()) {
this.mapService.removePopupFromMap(this.popupInstance);
} else if (markerInstance) {
this.mapService.removePopupFromMarker(markerInstance);
}
}
this.popupInstance = null;

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.

Popup click changing routes results in NG0953
2 participants