Repository navigation
Conversation
- The datastore could answer before onMapReady; the saved position was then silently dropped and the map opened at the provider's world-tiled default. applyOpeningCameraIfReady waits for both sides and decides exactly once. - With nothing stored: markers that exist keep owning the opening view, otherwise the phone's fix when permission allows, else a country-scale frame for the app language (DACH), else a zoom that shows the world once. - A stored view now wins over the first-marker auto-navigation, which used to stomp the restored position at every open. Co-Authored-By: Devin/SWE-2 <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Location callback ordering, failed location requests, and regionless German locales leave opening-camera behavior incorrect.
3 open findings
What changed in this PR
Improves the map’s opening view by coordinating saved-camera restoration and adding defaults when no position is stored.
Changes:
- Waits for both the map provider and stored camera data.
- Prevents marker auto-navigation from overriding a restored view.
- Adds phone-location, regional, and world-view fallbacks.
| File | Description |
|---|---|
| app/src/main/java/dev/wander/android/opentagviewer/MapsActivity.java | Coordinates camera restoration, marker priority, and opening-view fallbacks. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- onMapReady fires out of initialize() before the MapView's first layout, so the stored-camera restore ran setCenter on a zero-sized view: osmdroid scrolls in pixels and the opening view landed half a screen off to the side - Defer moveCamera/animateCamera via addOnFirstLayoutListener until the view has been measured; calls after layout run immediately Co-Authored-By: Devin/SWE-2 <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- The phone-fix callback rechecks currentMarkers: the request fires
before showLastDeviceLocations runs, and a fix landing afterwards
would stomp the marker view that just claimed the opening
- A failed getLastLocation now falls back to the locale view unless
markers arrived meanwhile - before, the map could stay at the
provider's world default with openingCameraApplied already consumed
- The locale fallback matches the configured locale's language ("de")
instead of Locale.getDefault()'s region: the app's German locale has
no country component, so the DACH frame was never reached
Co-Authored-By: Devin/SWE-2 <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Opening the map could ignore the camera position saved on the previous
visit, and a first-run map opened fully zoomed out with the world tiled
by the provider's default. Two causes, both in
MapsActivity:pos.ifPresent(...)gated onmapProvider != null. When the datastoreanswered before
onMapReady, the saved view was silently dropped.applyOpeningCameraIfReady()now waits until both the provider and theloaded value have arrived and decides exactly once.
showBeaconOnMap()navigated to the first marker on every rebuild(
currentMarkers.size() == 1), stomping a restored view whenever asingle marker existed. A stored camera now wins over marker
auto-navigation.
When nothing is stored, the fallback is: existing markers keep owning
the opening view, then the phone's own fix when permission allows, then
a country-scale frame chosen by the app language (DACH), then a zoom
that shows the world once rather than tiled.
Test plan
comes back
tiled world map
Generated with Devin
Update: a second commit covers the osmdroid provider, where the same
opening move ran on a zero-sized
MapView-initialize()callsonMapReadybefore the first layout, and osmdroid scrolls in pixels,so the restored view opened half a screen off to the side. Camera calls
now wait for the first layout via
addOnFirstLayoutListener.