-
Notifications
You must be signed in to change notification settings - Fork 119
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
Location layer asset update #245
Conversation
d6e44b9
to
a2beeb7
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.
looking great overall, found a couple of small nit picks
private void addCameraListener() { | ||
if (locationLayerMode == LocationLayerMode.NAVIGATION) { | ||
mapboxMap.removeOnCameraMoveListener(this); | ||
return; |
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.
it's a bit strange that a method called #addCameraListener is actually removing the listener in some cases. Either rename the method to something fitting or decouple the removal into a separate codeblock
@@ -496,6 +504,8 @@ private void setNavigationLayerVisibility(boolean visible) { | |||
@Override | |||
public void onCameraMove() { | |||
locationLayer.updateAccuracyRadius(location); | |||
locationLayer.updateForegroundOffset(mapboxMap.getCameraPosition().tilt); | |||
locationLayer.updateForegroundBearing((float) mapboxMap.getCameraPosition().bearing); |
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.
reuse the CameraPosition object, now we are calling twice into c++ code while we can get away with just once.
Thanks for the review, made the changes @tobrun |
#This PR optimizes/updates some of the location layer assets. The main difference is now we are adding a more 3D effect to the icons so that the user location pops out more and is easier to find (especially on a white background).
I've also fixed an issue with the accuracy layer getting initialized with the default radius of 5 and then immediately going back to zero since the
location
variable was null.lastLocation
variable has been removed sincelocation
is virtually identical.