-
Notifications
You must be signed in to change notification settings - Fork 140
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
Revisit MapboxMap plugin related extension functions to not use singletons #75
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate the thinking outside the box. Moving the extension functions to SDK-base makes them always part of the SDK public API. We will conditionally execute APIs if the plugin dependency has been met. If we want to moe forward to such a setup, we can also replace the extension functions from sdk-base and add them directly on MapboxMap.
Note how this is conceptually different as the initial idea behind it. We only wanted to populate the MapboxMap API with new APIs if a plugin dependency was met. If you add animations plugin dependency, "magically" MapboxMap was able to use flyTo etc. without it, it wasn't.
@tobrun - thanks a lot for such detailed write-up! I actually did not realize that bringing extension functions to |
4d9b0f0
to
78aee15
Compare
78aee15
to
aa22e25
Compare
aa22e25
to
f686302
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super clean, I like it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Great work! can we add tests to these functions? |
Yep, working on that right now 👍 |
PRs must be submitted under the terms of our Contributor License Agreement CLA.
Fixes: #73
<changelog>Revisit MapboxMap plugin related extension functions to not use singletons</changelog>
Pull request checklist:
mapbox-maps-android
changelog:<changelog></changelog>
.Summary of changes
Fix extension function usage in case of several instances of
MapboxMap
objects if using plugins.User impact (optional)