-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[android] Adding pulsing circle to LocationComponent options #13942
[android] Adding pulsing circle to LocationComponent options #13942
Conversation
Still definitely a work in progress but opening this pr to start discussion and review from @mapbox/maps-android . Not sure whether my implementation is correct. Once things are 👌 on the back end, I can finish the test app example, write tests, finish Javadocs, etc. Also, as you'll see in the fade enabled GIF above, there's a weird "hitch" when a pulse is finished and a new one is about to begin. I assume it probably has to do with how I'm tracking things in the |
2bdda13
to
5eddbb6
Compare
I've got https://github.com/mapbox/mapbox-gl-native/pull/13942/files#diff-e6b14cce649c4c464b5c466cbda57938R76 , which is used here and then ultimately here. Doesn't seem to be working though. If you open the pulsing circle test app activity and then go back, it'll crash the app:
Thoughts on how I might manage lifecycle with this background runnable and all?... |
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.
sub module pins were updated but not required for this PR?
might be good to refactor all the pulsing fields into a new object?
...ndroid/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java
Outdated
Show resolved
Hide resolved
...MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponentOptions.java
Outdated
Show resolved
Hide resolved
.../MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java
Outdated
Show resolved
Hide resolved
5eddbb6
to
55a49db
Compare
Yes, they snuck in here 😢 |
Yea, wasn't sure whether we should have completely separate builder or not for these pulsing options. I didn't make one as a start, 'cause it might be too many nested builders into one another. 3 levels deep: |
Note: @LukasPaczos followed up offline and confirmed that the current setup is fine. Pulsing options can be in |
7846ce9
to
2547b7d
Compare
eaadbd0
to
7834827
Compare
0878689
to
365fb1f
Compare
2a4f7ab
to
9b877fa
Compare
9a44ddf
to
15f8bf2
Compare
54145bb
to
ca202fc
Compare
ca202fc
to
dceb95e
Compare
71d3a2e
to
1a885b1
Compare
Ok @LukasPaczos , things are looking good visually and all tests are passing. This is ready for yet another round of review when you have the time and are in the mood. |
a50f17c
to
3f1abeb
Compare
…inator class and test for pulsing circle
3f1abeb
to
3829e79
Compare
Looks good! Looking forward to use this. Any updates when this PR will be merged? 👍 |
We are removing platform code from gl-native in #15970. If you want to see this PR merged, it will require re-implementation in https://github.com/mapbox/mapbox-gl-native-android. |
@malwoodsantoro When can we get this re implemented? |
For anyone following this pr and/or interested in using this feature, you should know that this feature has been added to the Mapbox Maps SDK for Android at mapbox/mapbox-gl-native-android#172. The current Mapbox Maps SDK for Android is in the https://github.com/mapbox/mapbox-gl-native-android repository because the Mapbox team decided to move the Maps SDK for Android to a standalone repo: #15971 |
When the new version 9.2.0. will be officially announced? I see that on the Mapbox page stays still 9.1.0. |
Hey @Cloud9Clone, https://docs.mapbox.com/android/maps/overview/ and other docs pages just haven't been updated yet. Docs about pulsing |
This pr adds a pulsing circle functionality to the Android
LocationComponent
.The following options/methods have been added to
LocationComponentOptions
for styling the pulsing circle:Fade enabled:
Fade disabled: