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

[android] - add style loading callback #8291

Merged
merged 2 commits into from
Mar 10, 2017

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Mar 6, 2017

Closes #8262.

This PR adds a style loading callback that is invoked when the style has finished loading.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Mar 6, 2017
@tobrun tobrun added this to the android-v5.0.0 milestone Mar 6, 2017
@tobrun tobrun self-assigned this Mar 6, 2017
@tobrun tobrun requested a review from ivovandongen March 6, 2017 19:06
@mention-bot
Copy link

@tobrun, thanks for your PR! By analyzing this pull request, we identified @incanus to be potential reviewers.

}
}
});
}
nativeMapView.setStyleUrl(url);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not call this class's setStyleUrl() in order to gate the native call through fewer (i.e. a single) places?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@incanus That would be a circular reference and would cause a stack overflow

* <li>{@code null}: loads the default {@link Style#MAPBOX_STREETS} style.</li>
* </ul>
* <p>
* This method is asynchronous and will return immediately before the style finishes loading.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and the similar comment(s) above) could be worded better—makes it sound like immediately before instead of what actually happens, which is immediately and before the style finishes loading.

* </ul>
* <p>
* This method is asynchronous and will return immediately before the style finishes loading.
* If you wish to wait for the map to finish loading listen for the {@link MapView#DID_FINISH_LOADING_MAP} event.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a , after loading.

public void setStyleUrl(@NonNull final String url, @Nullable final OnStyleLoadedListener callback) {
if (callback != null) {
nativeMapView.addOnMapChangedListener(new MapView.OnMapChangedListener() {
@Override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be my Android newbness speaking, but do you need to call the superclass implementation if you are overriding here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@incanus It's an interface, not a class

Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@incanus
Copy link
Contributor

incanus commented Mar 6, 2017

Just my minor comment docs changes above, but this is good to ship.

@tobrun tobrun merged commit d1e50b3 into release-ios-v3.5.0-android-v5.0.0 Mar 10, 2017
@tobrun tobrun deleted the 8262-style-loaded-callback branch March 10, 2017 01:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants