-
Notifications
You must be signed in to change notification settings - Fork 135
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
fix: Set initial map padding (#326) #327
base: main
Are you sure you want to change the base?
Conversation
b3030b1
to
ae012d1
Compare
Thanks for the contribution @kkris ! This fixes the issue. |
@kikoso Thanks for reviewing! Any idea how to fix the
|
Hi @kkris ! Since you are running it from another branch, the instrumented tests are going to fail since you do not have a local API Key. We have thought of a couple of solutions here, but nothing in place so far. What you can do in the meantime is run the instrumentation test locally. Before merging the external contributions we run the test locally too. |
@@ -152,15 +155,7 @@ internal inline fun MapUpdater( | |||
set(mapProperties.maxZoomPreference) { map.setMaxZoomPreference(it) } | |||
set(mapProperties.minZoomPreference) { map.setMinZoomPreference(it) } | |||
set(contentPadding) { |
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.
This needs to change from set()
to update()
. Perhaps could move up to the other update()
calls purely for consistency.
set(contentPadding) { | |
update(contentPadding) { |
applyContentPadding(map, contentPadding) | ||
cameraPositionState.setMap(map) |
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.
Could you add a brief comment here to inform a reader why padding must precede camera position?
applyContentPadding(map, contentPadding) | |
cameraPositionState.setMap(map) | |
applyContentPadding(map, contentPadding) | |
// set camera position after padding for correct centering | |
cameraPositionState.setMap(map) |
Fixes #326 🦕
With this fix applied, the example from #326 now behaves correctly: https://github.com/googlemaps/android-maps-compose/assets/232214/7a07bcbb-c9fc-4924-8131-762926f36e29