Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Several methods assume style object is created #4527

Closed
tmpsantos opened this issue Mar 30, 2016 · 4 comments
Closed

[core] Several methods assume style object is created #4527

tmpsantos opened this issue Mar 30, 2016 · 4 comments
Assignees

Comments

@tmpsantos
Copy link
Contributor

We have several methods that are currently crashing because map.setStyleURL is async. For instance addLayer, addClass, isLoaded and several others. We should have a signal (MapChangeDidFinishLoadingStyle?) emitted right after MapContext::loadStyleJSON.

On Android for instance MapChangeDidFinishLoadingMap is used to indicate that the style was loaded as a whole but we need something to indicate that the style JSON arrived (and the style object was created). Otherwise we might ended up rendering a partial state and later add a custom layer or class.

This new signal would be better implemented after we get rid of the context thread (#2909) because we can safely call view.notifyMapChange from MapContext::loadStyleJSON after that.

Finally, we should maybe throw an runtime exception when calling the methods that depend on a style when the style json is not yet loaded.

/cc @brunoabinader @jfirebaugh

@brunoabinader
Copy link
Member

Nice catch @tmpsantos - I think it would be useful if we can also add a onStyleLoaded observer that could e.g. trigger a callback on the client.

@1ec5
Copy link
Contributor

1ec5 commented Sep 19, 2016

@friedbunny, for iOS/macOS, how does this relate to #2775? Do we need to add a new delegate method, or is an existing one suitable?

@friedbunny
Copy link
Contributor

for iOS/macOS, how does this relate to #2775? Do we need to add a new delegate method, or is an existing one suitable?

There isn’t currently a suitable delegate method. MapKit doesn’t care to inform you of style loading events, so there’s no analogue to draw upon.

@jfirebaugh
Copy link
Contributor

I agree with the utility of a notification for this event -- it's style.load in gl-js. I'm less keen on expanding the MapChange enumeration, for reasons detailed in #6383, though I wouldn't block a PR for this on the full refactor suggested there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants