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

Improve documentation for map events #8947

Closed
zugaldia opened this issue May 10, 2017 · 3 comments
Closed

Improve documentation for map events #8947

zugaldia opened this issue May 10, 2017 · 3 comments
Labels
Android Mapbox Maps SDK for Android documentation

Comments

@zugaldia
Copy link
Member

We currently provide one line descriptions for map events inside Javadoc, but sometimes it isn't completely clear how events are different from each other, or what's expected for developers to be able to do in response to a specific event.

For example, for WILL_START_RENDERING_FRAME we currently say:

This event is triggered when the map will start rendering a frame.

iOS explains this further on mapViewWillStartRenderingFrame:

Tells the delegate that the map view is about to redraw.

This method is called any time the map view needs to redraw due to a change in the viewpoint or style property transition. This method may be called very frequently, even moreso than -mapViewRegionIsChanging:. Therefore, your implementation of this method should be as lightweight as possible to avoid affecting performance.

We should clarify our current Javadoc and provide some more background on map events. Current iOS docs (here, here) are a great starting point for this.

cc: @langsmith @cammace

@zugaldia zugaldia added Android Mapbox Maps SDK for Android documentation labels May 10, 2017
@tobrun
Copy link
Member

tobrun commented May 15, 2017

FWIW: the core names sometimes make it a bit more clear what these events actually mean.
You can find them in map_observer.hpp, though they're undocumented.

    virtual void onCameraWillChange(CameraChangeMode) {}
    virtual void onCameraIsChanging() {}
    virtual void onCameraDidChange(CameraChangeMode) {}
    virtual void onWillStartLoadingMap() {}
    virtual void onDidFinishLoadingMap() {}
    virtual void onDidFailLoadingMap(std::exception_ptr) {}
    virtual void onWillStartRenderingFrame() {}
    virtual void onDidFinishRenderingFrame(RenderMode) {}
    virtual void onWillStartRenderingMap() {}
    virtual void onDidFinishRenderingMap(RenderMode) {}
    virtual void onDidFinishLoadingStyle() {}
    virtual void onSourceChanged(style::Source&) {}

When implementing #8389, I'm thinking of changing the name of our public API to match the one exposed by core. AFAIK the naming as-is, was mostly copied from iOS which matches Mapkit.

@zugaldia
Copy link
Member Author

When implementing #8389, I'm thinking of changing the name of our public API to match the one exposed by core. AFAIK the naming as-is, was mostly copied from iOS which matches Mapkit.

👍 Agreed that matching core's naming makes more sense here than matching MapKit's.

@tobrun
Copy link
Member

tobrun commented Aug 17, 2017

With #9498 we are introducing dedicated callbacks that will match the core api fully in terms of naming. All these callbacks with be accompanied with javadoc.

@tobrun tobrun closed this as completed Aug 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android documentation
Projects
None yet
Development

No branches or pull requests

2 participants