-
Notifications
You must be signed in to change notification settings - Fork 20
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
Restore zoom + camera position state when returning to map #472
Conversation
24f3266
to
e938b2d
Compare
val target = if (mapViewModel.mapCenter.value.isOrigin()) { | ||
latestReportPosition.value | ||
} else { | ||
mapViewModel.mapCenter.value | ||
} | ||
map.cameraPosition = CameraPosition.Builder() | ||
.target(target?.asMapLibreLatLng()) | ||
.zoom(mapViewModel.zoom.value) | ||
.build() |
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 has to be done in the update()
function, because the latest report position is loaded asynchronously and it might not be available when the view is initialized
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.
In my testing, this was never a problem, the latestReportPosition was always available. But sure, I’ll change it back to update().
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.
Thanks!
For the default map zoom level, I'm not sure what would be a good value. It depends a lot on your use case. For example, personally I usually use the map to get an overview of the areas where I have not yet collected data and in that case a good default would be to zoom to the city level. But I'll test how this looks and maybe change it depending on how it feels
This is why I only increased it to 12. There, it still comfortably fits 100-500K person cities (e.g. Zürich) on my screen, while also showing just a bit more street details. Of course, it will also vary by screen size and resolution. It’s not a radical change, but I find it a bit easier. |
Previously, when switching between tabs and returning to the map, the state would be reset to the latestReportPosition. Instead, restore it to the last position and zoom that the user navigated to.
Previously, when switching between tabs and returning to the map, the state would be reset to the latestReportPosition.
(Personally, especially the zoom being reset annoys me, since when mapping in a city I zoom in much more than the default of 10. Even the 12 that I now changed the default to is still quite far out for city/street-level.)
Instead, restore it to the last position and zoom that the user navigated to.
Also move this logic from update() to factory(), since it only needs to run once.