-
Notifications
You must be signed in to change notification settings - Fork 131
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
feat: Add the reason for map camera movement #154
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.
Change generally LGTM although I think this is a good opportunity to improve the type safety around reason
and use an enum instead. A new type can also be created for the initial case. Note that this is a pattern already used in the library for MapType
.
@arriolac Good idea! I converted to an enum in 819179e. I wasn't sure how to handle the case where a new Maps SDK constant is passed back from the |
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! Just a small nit about imports.
Re: using -2 for an unknown reason. The other option is to initialize cameraMoveStartedReason
as null which would represent NO_MOVEMENT_YET
. It's nice to have properties as non-optional though so I think your current approach is better.
import kotlinx.coroutines.cancel | ||
import kotlinx.coroutines.currentCoroutineContext | ||
import kotlinx.coroutines.suspendCancellableCoroutine | ||
import kotlinx.coroutines.* |
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.
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.
Ah, good catch - I fixed the imports to not use wildcards and updated my AS settings.
# [2.4.0](v2.3.0...v2.4.0) (2022-06-23) ### Features * Add the reason for map camera movement ([#154](#154)) ([202f34f](202f34f))
🎉 This PR is included in version 2.4.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
As discussed in #49, this PR exposes the
reason
value passed back inGoogleMap.OnCameraMoveStartedListener(int reason)
callback so that developers can tell the reason that the camera started moving (e.g., user gesture, developer animation, API animation):https://developers.google.com/android/reference/com/google/android/gms/maps/GoogleMap.OnCameraMoveStartedListener#onCameraMoveStarted(int)
For constant values see:
https://developers.google.com/android/reference/com/google/android/gms/maps/GoogleMap.OnCameraMoveStartedListener#constants
The PR also adds demo code to
LocationTrackingActivity
to print the reason that the camera moves, as well as assertions to test behavior in an existing unit test.Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #49 🦕