fix(map): address code review findings — precision, naming, icons, i18n

- Fix Int.toFloat() precision loss in track point filter by storing
  time as string in GeoJSON and using string-based equality comparison
- Rename MapStyle enum values to match actual tile styles: Satellite→Light
  (Positron), Hybrid→RoadMap (Americana), with updated string resources
- Reset bearingUpdate to IGNORE when gesture cancels location tracking
- Use LocationOn icon for ALWAYS_NORTH tracking mode instead of
  misleading LocationDisabled
- Remove dead isOfflineManagerAvailable() expect/actual declarations
- Replace hardcoded English strings in offline map UI with
  stringResource() calls backed by core:resources entries
This commit is contained in:
James Rich 2026-04-13 12:14:17 -05:00
parent 01d2642275
commit 5cae109ec9
10 changed files with 64 additions and 44 deletions

View file

@ -1160,10 +1160,16 @@
<string name="bluetooth_feature_config_description">Wirelessly manage your device settings and channels.</string>
<string name="map_style_selection">Map style selection</string>
<string name="map_style_osm">OpenStreetMap</string>
<string name="map_style_satellite">Satellite</string>
<string name="map_style_light">Light</string>
<string name="map_style_terrain">Terrain</string>
<string name="map_style_hybrid">Hybrid</string>
<string name="map_style_road_map">Road Map</string>
<string name="map_style_dark">Dark</string>
<string name="offline_maps">Offline Maps</string>
<string name="offline_download">Download</string>
<string name="offline_download_visible_region">Download visible region</string>
<string name="offline_saves_tiles">Saves tiles for offline use</string>
<string name="offline_downloaded_regions">Downloaded Regions</string>
<string name="offline_unnamed_region">Unnamed Region</string>
<string name="local_stats_battery">Battery: %1$d%</string>
<string name="local_stats_nodes">Nodes: %1$d online / %2$d total</string>

View file

@ -38,15 +38,22 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import kotlinx.coroutines.launch
import org.jetbrains.compose.resources.stringResource
import org.maplibre.compose.camera.CameraState
import org.maplibre.compose.material3.OfflinePackListItem
import org.maplibre.compose.offline.OfflinePackDefinition
import org.maplibre.compose.offline.rememberOfflineManager
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.done
import org.meshtastic.core.resources.offline_download
import org.meshtastic.core.resources.offline_download_visible_region
import org.meshtastic.core.resources.offline_downloaded_regions
import org.meshtastic.core.resources.offline_maps
import org.meshtastic.core.resources.offline_saves_tiles
import org.meshtastic.core.resources.offline_unnamed_region
import org.meshtastic.core.ui.icon.CloudDownload
import org.meshtastic.core.ui.icon.MeshtasticIcons
@Composable actual fun isOfflineManagerAvailable(): Boolean = true
@Suppress("LongMethod")
@Composable
actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
@ -55,9 +62,10 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
var showDialog by remember { mutableStateOf(false) }
if (showDialog) {
val unnamedRegion = stringResource(Res.string.offline_unnamed_region)
AlertDialog(
onDismissRequest = { showDialog = false },
title = { Text("Offline Maps") },
title = { Text(stringResource(Res.string.offline_maps)) },
text = {
Column(modifier = Modifier.fillMaxWidth()) {
// Download button for current viewport
@ -85,13 +93,16 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
) {
Icon(
imageVector = MeshtasticIcons.CloudDownload,
contentDescription = "Download",
contentDescription = stringResource(Res.string.offline_download),
modifier = Modifier.padding(end = 16.dp),
)
Column {
Text(text = "Download visible region", style = MaterialTheme.typography.bodyLarge)
Text(
text = "Saves tiles for offline use",
text = stringResource(Res.string.offline_download_visible_region),
style = MaterialTheme.typography.bodyLarge,
)
Text(
text = stringResource(Res.string.offline_saves_tiles),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
)
@ -101,27 +112,27 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
// Existing packs
if (offlineManager.packs.isNotEmpty()) {
Text(
text = "Downloaded Regions",
text = stringResource(Res.string.offline_downloaded_regions),
style = MaterialTheme.typography.titleSmall,
modifier = Modifier.padding(top = 16.dp, bottom = 8.dp),
)
offlineManager.packs.toList().forEach { pack ->
key(pack.hashCode()) {
OfflinePackListItem(pack = pack, offlineManager = offlineManager) {
Text(pack.metadata?.decodeToString().orEmpty().ifBlank { "Unnamed Region" })
Text(pack.metadata?.decodeToString().orEmpty().ifBlank { unnamedRegion })
}
}
}
}
}
},
confirmButton = { TextButton(onClick = { showDialog = false }) { Text("Done") } },
confirmButton = { TextButton(onClick = { showDialog = false }) { Text(stringResource(Res.string.done)) } },
)
}
// Expose the toggle via a side effect — the parent screen will call this
// by rendering OfflineMapContent and using the showDialog state
IconButton(onClick = { showDialog = true }) {
Icon(imageVector = MeshtasticIcons.CloudDownload, contentDescription = "Offline Maps")
Icon(imageVector = MeshtasticIcons.CloudDownload, contentDescription = stringResource(Res.string.offline_maps))
}
}

View file

@ -199,6 +199,7 @@ fun MapScreen(
LaunchedEffect(cameraState.moveReason) {
if (cameraState.moveReason == CameraMoveReason.GESTURE && isLocationTrackingEnabled) {
isLocationTrackingEnabled = false
bearingUpdate = BearingUpdate.IGNORE
}
}
}

View file

@ -18,14 +18,6 @@ package org.meshtastic.feature.map
import androidx.compose.runtime.Composable
/**
* Returns `true` if the platform supports offline map tile management.
* - Android: `true` (backed by MapLibre Native).
* - iOS: `true` (backed by MapLibre Native).
* - Desktop/JS: `false` (no offline support).
*/
@Composable expect fun isOfflineManagerAvailable(): Boolean
/**
* Renders platform-specific offline map management UI if the platform supports it. The composable receives the current
* style URI and [cameraState] for downloading the visible region.

View file

@ -34,7 +34,7 @@ import org.meshtastic.core.resources.map_filter
import org.meshtastic.core.resources.orient_north
import org.meshtastic.core.resources.refresh
import org.meshtastic.core.resources.toggle_my_position
import org.meshtastic.core.ui.icon.LocationDisabled
import org.meshtastic.core.ui.icon.LocationOn
import org.meshtastic.core.ui.icon.MapCompass
import org.meshtastic.core.ui.icon.MeshtasticIcons
import org.meshtastic.core.ui.icon.MyLocation
@ -116,13 +116,13 @@ fun MapControlsOverlay(
}
}
// Location tracking button — 3 states: Off (MyLocation), Tracking (LocationDisabled), TrackingBearing (NearMe)
// Location tracking button — 3 states: Off (MyLocation), Tracking (NearMe), TrackingNorth (LocationOn)
MapButton(
icon =
when {
!isLocationTrackingEnabled -> MeshtasticIcons.MyLocation
isTrackingBearing -> MeshtasticIcons.NearMe
else -> MeshtasticIcons.LocationDisabled
else -> MeshtasticIcons.LocationOn
},
contentDescription = stringResource(Res.string.toggle_my_position),
iconTint = if (isLocationTrackingEnabled) MaterialTheme.colorScheme.primary else null,

View file

@ -20,7 +20,8 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import org.maplibre.compose.expressions.dsl.asNumber
import kotlinx.serialization.json.jsonPrimitive
import org.maplibre.compose.expressions.dsl.asString
import org.maplibre.compose.expressions.dsl.const
import org.maplibre.compose.expressions.dsl.eq
import org.maplibre.compose.expressions.dsl.feature
@ -81,7 +82,7 @@ fun NodeTrackLayers(
strokeWidth = const(1.dp),
strokeColor = const(Color.White),
onClick = { features ->
val time = features.firstOrNull()?.properties?.get("time")?.toString()?.toIntOrNull()
val time = features.firstOrNull()?.properties?.get("time")?.jsonPrimitive?.content?.toIntOrNull()
if (time != null && onPositionSelected != null) {
onPositionSelected(time)
ClickResult.Consume
@ -96,7 +97,7 @@ fun NodeTrackLayers(
CircleLayer(
id = "node-track-selected",
source = pointsSource,
filter = feature["time"].asNumber() eq const(selectedPositionTime.toFloat()),
filter = feature["time"].asString() eq const(selectedPositionTime.toString()),
radius = const(10.dp),
color = const(SelectedPointColor), // Red
strokeWidth = const(2.dp),

View file

@ -20,9 +20,9 @@ import org.jetbrains.compose.resources.StringResource
import org.maplibre.compose.style.BaseStyle
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.map_style_dark
import org.meshtastic.core.resources.map_style_hybrid
import org.meshtastic.core.resources.map_style_light
import org.meshtastic.core.resources.map_style_osm
import org.meshtastic.core.resources.map_style_satellite
import org.meshtastic.core.resources.map_style_road_map
import org.meshtastic.core.resources.map_style_terrain
/**
@ -35,15 +35,15 @@ enum class MapStyle(val label: StringResource, val styleUri: String) {
OpenStreetMap(label = Res.string.map_style_osm, styleUri = "https://tiles.openfreemap.org/styles/liberty"),
/** Clean, light cartographic style via OpenFreeMap Positron. */
Satellite(label = Res.string.map_style_satellite, styleUri = "https://tiles.openfreemap.org/styles/positron"),
Light(label = Res.string.map_style_light, styleUri = "https://tiles.openfreemap.org/styles/positron"),
/** Topographic style via OpenFreeMap Bright. */
Terrain(label = Res.string.map_style_terrain, styleUri = "https://tiles.openfreemap.org/styles/bright"),
/** US road-map style via Americana. */
Hybrid(label = Res.string.map_style_hybrid, styleUri = "https://americanamap.org/style.json"),
RoadMap(label = Res.string.map_style_road_map, styleUri = "https://americanamap.org/style.json"),
/** Dark mode style via OpenFreeMap Bright (dark palette). */
/** Dark mode style via OpenFreeMap Fiord. */
Dark(label = Res.string.map_style_dark, styleUri = "https://tiles.openfreemap.org/styles/fiord"),
;

View file

@ -123,7 +123,7 @@ fun positionsToPointFeatures(positions: List<org.meshtastic.proto.Position>): Fe
if (lat == 0.0 && lng == 0.0) return@mapNotNull null
val props = buildJsonObject {
put("time", pos.time ?: 0)
put("time", (pos.time ?: 0).toString())
put("altitude", pos.altitude ?: 0)
put("ground_speed", pos.ground_speed ?: 0)
put("sats_in_view", pos.sats_in_view ?: 0)

View file

@ -38,15 +38,22 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import kotlinx.coroutines.launch
import org.jetbrains.compose.resources.stringResource
import org.maplibre.compose.camera.CameraState
import org.maplibre.compose.material3.OfflinePackListItem
import org.maplibre.compose.offline.OfflinePackDefinition
import org.maplibre.compose.offline.rememberOfflineManager
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.done
import org.meshtastic.core.resources.offline_download
import org.meshtastic.core.resources.offline_download_visible_region
import org.meshtastic.core.resources.offline_downloaded_regions
import org.meshtastic.core.resources.offline_maps
import org.meshtastic.core.resources.offline_saves_tiles
import org.meshtastic.core.resources.offline_unnamed_region
import org.meshtastic.core.ui.icon.CloudDownload
import org.meshtastic.core.ui.icon.MeshtasticIcons
@Composable actual fun isOfflineManagerAvailable(): Boolean = true
@Suppress("LongMethod")
@Composable
actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
@ -55,9 +62,10 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
var showDialog by remember { mutableStateOf(false) }
if (showDialog) {
val unnamedRegion = stringResource(Res.string.offline_unnamed_region)
AlertDialog(
onDismissRequest = { showDialog = false },
title = { Text("Offline Maps") },
title = { Text(stringResource(Res.string.offline_maps)) },
text = {
Column(modifier = Modifier.fillMaxWidth()) {
Row(
@ -84,13 +92,16 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
) {
Icon(
imageVector = MeshtasticIcons.CloudDownload,
contentDescription = "Download",
contentDescription = stringResource(Res.string.offline_download),
modifier = Modifier.padding(end = 16.dp),
)
Column {
Text(text = "Download visible region", style = MaterialTheme.typography.bodyLarge)
Text(
text = "Saves tiles for offline use",
text = stringResource(Res.string.offline_download_visible_region),
style = MaterialTheme.typography.bodyLarge,
)
Text(
text = stringResource(Res.string.offline_saves_tiles),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
)
@ -99,25 +110,25 @@ actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
if (offlineManager.packs.isNotEmpty()) {
Text(
text = "Downloaded Regions",
text = stringResource(Res.string.offline_downloaded_regions),
style = MaterialTheme.typography.titleSmall,
modifier = Modifier.padding(top = 16.dp, bottom = 8.dp),
)
offlineManager.packs.toList().forEach { pack ->
key(pack.hashCode()) {
OfflinePackListItem(pack = pack, offlineManager = offlineManager) {
Text(pack.metadata?.decodeToString().orEmpty().ifBlank { "Unnamed Region" })
Text(pack.metadata?.decodeToString().orEmpty().ifBlank { unnamedRegion })
}
}
}
}
}
},
confirmButton = { TextButton(onClick = { showDialog = false }) { Text("Done") } },
confirmButton = { TextButton(onClick = { showDialog = false }) { Text(stringResource(Res.string.done)) } },
)
}
IconButton(onClick = { showDialog = true }) {
Icon(imageVector = MeshtasticIcons.CloudDownload, contentDescription = "Offline Maps")
Icon(imageVector = MeshtasticIcons.CloudDownload, contentDescription = stringResource(Res.string.offline_maps))
}
}

View file

@ -19,8 +19,6 @@ package org.meshtastic.feature.map
import androidx.compose.runtime.Composable
import org.maplibre.compose.camera.CameraState
@Composable actual fun isOfflineManagerAvailable(): Boolean = false
@Composable
actual fun OfflineMapContent(styleUri: String, cameraState: CameraState) {
// Offline map management is not available on Desktop.