Check if this._circleElement exists before trying to change it's style

[pakastin]

Fixes an issue when this._circleElement doesn't exist before trying to update it.

[andrewharvey]

Could you describe what situation lead to this code path being called without this._circleElement being defined?

We also have an assert a few lines up so it should be one or the other, the assert or this check.

[andrewharvey]

cc @Meekohi

[Meekohi]

Yeah if there is some way for this to come up in practice we should address the root issue -- otherwise I was hoping the assert makes it clear that this._circleElement always exists when this is called.

[pakastin]

Well, it just broke aviamaps.com/map immediately when page loaded so 🤷‍♂️

[andrewharvey]

@pakastin When I try a minimal test case it's working as expected, are you doing anything special?

It would be helpful if you could work the error you're seeing down into a minimal test case, so we can work out what's causing this issue.

[pakastin]

Ok I found the cause. I was calling fitBounds before map load event. That fixed the problem. I would make a check though, since this._circleElement is created conditionally:

mapbox-gl-js/src/ui/control/geolocate_control.js

Lines 363 to 372 in 7a4eb62

	if (this.options.showUserLocation) {
	this._dotElement = DOM.create('div', 'mapboxgl-user-location-dot');
	this._userLocationDotMarker = new Marker(this._dotElement);
	this._circleElement = DOM.create('div', 'mapboxgl-user-location-accuracy-circle');
	this._accuracyCircleMarker = new Marker({element: this._circleElement, pitchAlignment: 'map'});
	if (this.options.trackUserLocation) this._watchState = 'OFF';
	}

[andrewharvey]

Oh I see at line

mapbox-gl-js/src/ui/control/geolocate_control.js

Lines 129 to 130 in 7a4eb62

	checkGeolocationSupport(this._setupUI);
	this._map.on('zoom', this._onZoom);

checkGeolocationSupport is async so the zoom event is being registered and may be triggered before setupUI is run.

@pakastin or @Meekohi do you want to fix the root cause here? Should just be a matter of putting this._map.on('zoom', this._onZoom); inside setupUI, and if possible a unit test which would have caught this issue, if it's possible.

[Meekohi]

Yup I think I get the flow -- I'll try to tackle this tonight unless someone can get to it earlier.

[andrewharvey]

Thanks for reporting this bug @pakastin and submitting the PR. I'll close this one in favour of #9288 which addresses the root cause.