Repository navigation
fix(widgets): preserve fractional scale bar distances #1045
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
Changes from all commits
aa0e509
ddeefca
e2b9cba
064e3b5
3af7bf1
ba81967
58ea02c
ee57e4c
adb78fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // Copyright 2026 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.maps.android.compose | ||
|
|
||
| import androidx.compose.foundation.background | ||
| import androidx.compose.foundation.layout.Box | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.material3.MaterialTheme | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.test.assertIsDisplayed | ||
| import androidx.compose.ui.test.junit4.v2.createComposeRule | ||
| import androidx.compose.ui.test.onNodeWithText | ||
| import androidx.compose.ui.unit.dp | ||
| import com.google.maps.android.compose.widgets.ScaleBar | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| class ScaleBarDeviceRenderingTests { | ||
| @get:Rule | ||
| val composeTestRule = createComposeRule() | ||
|
|
||
| @Test | ||
| fun unboundCameraRendersOnLightBackground() { | ||
| assertInitialLabels(dark = false) | ||
| } | ||
|
|
||
| @Test | ||
| fun unboundCameraRendersOnDarkBackground() { | ||
| assertInitialLabels(dark = true) | ||
| } | ||
|
|
||
| private fun assertInitialLabels(dark: Boolean) { | ||
| val camera = CameraPositionState() | ||
| val background = if (dark) Color.Black else Color.White | ||
| val foreground = if (dark) Color.White else Color.Black | ||
| composeTestRule.setContent { | ||
| MaterialTheme { | ||
| Box(Modifier.background(background).padding(16.dp)) { | ||
| ScaleBar( | ||
| cameraPositionState = camera, | ||
| textColor = foreground, | ||
| lineColor = foreground, | ||
| shadowColor = background, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| composeTestRule.onNodeWithText("0 m").assertIsDisplayed() | ||
| composeTestRule.onNodeWithText("0 ft").assertIsDisplayed() | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // Copyright 2026 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.maps.android.compose | ||
|
|
||
| import android.graphics.Point | ||
| import androidx.compose.foundation.background | ||
| import androidx.compose.foundation.layout.Box | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.material3.MaterialTheme | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.test.assertIsDisplayed | ||
| import androidx.compose.ui.test.junit4.v2.createComposeRule | ||
| import androidx.compose.ui.test.onNodeWithText | ||
| import androidx.compose.ui.unit.dp | ||
| import com.google.android.gms.maps.Projection | ||
| import com.google.android.gms.maps.model.CameraPosition | ||
| import com.google.android.gms.maps.model.LatLng | ||
| import com.google.maps.android.compose.widgets.ScaleBar | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import org.robolectric.RobolectricTestRunner | ||
| import org.robolectric.annotation.Config | ||
|
|
||
| @RunWith(RobolectricTestRunner::class) | ||
| @Config(sdk = [29, 35]) | ||
| class ScaleBarRenderingTests { | ||
| @get:Rule | ||
| val composeTestRule = createComposeRule() | ||
|
|
||
| @Test | ||
| fun fractionalLabelsRenderOnLightBackground() { | ||
| assertFractionalLabels(dark = false) | ||
| } | ||
|
|
||
| @Test | ||
| fun fractionalLabelsRenderOnDarkBackground() { | ||
| assertFractionalLabels(dark = true) | ||
| } | ||
|
|
||
| @Test | ||
| fun largeLabelsRenderOnLightBackground() { | ||
| assertFractionalLabels(dark = false, large = true) | ||
| } | ||
|
|
||
| @Test | ||
| fun largeLabelsRenderOnDarkBackground() { | ||
| assertFractionalLabels(dark = true, large = true) | ||
| } | ||
|
|
||
| private fun assertFractionalLabels(dark: Boolean, large: Boolean = false) { | ||
| val projection = mockk<Projection>() | ||
| every { projection.fromScreenLocation(any()) } answers { | ||
| // At the equator 0.02 degrees spans about 2,224 m, or 1,976 m for the 8/9-width bar. | ||
| if (firstArg<Point>().x == 0) LatLng(0.0, 0.0) else LatLng(0.0, if (large) 20.0 else 0.02) | ||
| } | ||
| val camera = mockk<CameraPositionState>() | ||
| every { camera.position } returns CameraPosition.fromLatLngZoom(LatLng(0.0, 0.0), 12f) | ||
| every { camera.projection } returns projection | ||
| val background = if (dark) Color.Black else Color.White | ||
| val foreground = if (dark) Color.White else Color.Black | ||
| composeTestRule.setContent { | ||
| MaterialTheme { | ||
| Box(Modifier.background(background).padding(16.dp)) { | ||
| ScaleBar( | ||
| cameraPositionState = camera, | ||
| textColor = foreground, | ||
| lineColor = foreground, | ||
| shadowColor = background, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| composeTestRule.onNodeWithText(if (large) "1977 km" else "2 km").assertIsDisplayed() | ||
| composeTestRule.onNodeWithText(if (large) "1228 mi" else "1.2 mi").assertIsDisplayed() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,6 +51,7 @@ import com.google.android.gms.maps.Projection | |||||||||||||||||||||||||||||||||||||||||
| import com.google.maps.android.compose.CameraPositionState | ||||||||||||||||||||||||||||||||||||||||||
| import com.google.maps.android.sphericalDistance | ||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.delay | ||||||||||||||||||||||||||||||||||||||||||
| import kotlin.math.roundToInt | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| internal fun calculateDistance(projection: Projection, width: Dp, density: Density): Int { | ||||||||||||||||||||||||||||||||||||||||||
| val widthInPixels = with(density) { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -177,43 +178,47 @@ public fun ScaleBar( | |||||||||||||||||||||||||||||||||||||||||
| modifier = Modifier.fillMaxSize(), | ||||||||||||||||||||||||||||||||||||||||||
| verticalArrangement = Arrangement.SpaceAround, | ||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||
| // Here, we determine the appropriate units (meters/kilometers and feet/miles) | ||||||||||||||||||||||||||||||||||||||||||
| // based on the calculated distance in meters. | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| var metricUnits = "m" | ||||||||||||||||||||||||||||||||||||||||||
| var metricDistance = horizontalLineWidthMeters | ||||||||||||||||||||||||||||||||||||||||||
| if (horizontalLineWidthMeters > METERS_IN_KILOMETER) { | ||||||||||||||||||||||||||||||||||||||||||
| // Switch from meters to kilometers as unit | ||||||||||||||||||||||||||||||||||||||||||
| metricUnits = "km" | ||||||||||||||||||||||||||||||||||||||||||
| metricDistance /= METERS_IN_KILOMETER.toInt() | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| var imperialUnits = "ft" | ||||||||||||||||||||||||||||||||||||||||||
| var imperialDistance = horizontalLineWidthMeters.toDouble().toFeet() | ||||||||||||||||||||||||||||||||||||||||||
| if (imperialDistance > FEET_IN_MILE) { | ||||||||||||||||||||||||||||||||||||||||||
| // Switch from ft to miles as unit | ||||||||||||||||||||||||||||||||||||||||||
| imperialUnits = "mi" | ||||||||||||||||||||||||||||||||||||||||||
| imperialDistance = imperialDistance.toMiles() | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // We display the calculated distances in two Text composables, one for imperial | ||||||||||||||||||||||||||||||||||||||||||
| // and one for metric units. | ||||||||||||||||||||||||||||||||||||||||||
| ScaleText( | ||||||||||||||||||||||||||||||||||||||||||
| modifier = Modifier.align(End), | ||||||||||||||||||||||||||||||||||||||||||
| textColor = textColor, | ||||||||||||||||||||||||||||||||||||||||||
| shadowColor = shadowColor, | ||||||||||||||||||||||||||||||||||||||||||
| text = "${imperialDistance.toInt()} $imperialUnits", | ||||||||||||||||||||||||||||||||||||||||||
| text = formatImperialDistance(horizontalLineWidthMeters), | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| ScaleText( | ||||||||||||||||||||||||||||||||||||||||||
| modifier = Modifier.align(End), | ||||||||||||||||||||||||||||||||||||||||||
| textColor = textColor, | ||||||||||||||||||||||||||||||||||||||||||
| shadowColor = shadowColor, | ||||||||||||||||||||||||||||||||||||||||||
| text = "$metricDistance $metricUnits", | ||||||||||||||||||||||||||||||||||||||||||
| text = formatMetricDistance(horizontalLineWidthMeters), | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| internal fun formatMetricDistance(meters: Int): String = if (meters >= METERS_IN_KILOMETER) { | ||||||||||||||||||||||||||||||||||||||||||
| "${formatScaleDistance(meters / METERS_IN_KILOMETER)} km" | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| "$meters m" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| internal fun formatImperialDistance(meters: Int): String { | ||||||||||||||||||||||||||||||||||||||||||
| val feet = meters.toDouble().toFeet() | ||||||||||||||||||||||||||||||||||||||||||
| return if (feet >= FEET_IN_MILE) { | ||||||||||||||||||||||||||||||||||||||||||
| "${formatScaleDistance(feet.toMiles())} mi" | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| "${feet.roundToInt()} ft" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| private fun formatScaleDistance(distance: Double): String { | ||||||||||||||||||||||||||||||||||||||||||
| if (distance >= 10.0) { | ||||||||||||||||||||||||||||||||||||||||||
| return distance.roundToInt().toString() | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| val scaled = (distance * 10.0).roundToInt() | ||||||||||||||||||||||||||||||||||||||||||
| return if (scaled % 10 == 0) "${scaled / 10}" else "${scaled / 10}.${scaled % 10}" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+214
to
+220
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As @bubenheimer mentioned, two decimals are a bit too much for someone reading a map ("1.76 mi"). What about rounding to one decimal, which also matches what we are doing in the utils Units API? The tests would need an update (e.g.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applied the one-decimal suggestion and updated the tests: 1.01 km becomes 1 km, and 1.24 mi becomes 1.2 mi. The description now uses Refs #233 so the variable-length bar work stays open. |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+201
to
222
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are working toward making this codebase Kotlin Multiplatform (KMP) friendly, we'd love to avoid introducing Also, when zoomed far out (e.g., continental/world zoom levels where the bar spans hundreds or thousands of kilometers/miles), formatting with 2 decimal places produces long labels like We can handle both cleanly in pure Kotlin using import kotlin.math.roundToInt
internal fun formatMetricDistance(meters: Int): String = if (meters >= METERS_IN_KILOMETER) {
"${formatScaleDistance(meters / METERS_IN_KILOMETER)} km"
} else {
"$meters m"
}
internal fun formatImperialDistance(meters: Int): String {
val feet = meters.toDouble().toFeet()
return if (feet >= FEET_IN_MILE) {
"${formatScaleDistance(feet.toMiles())} mi"
} else {
"${feet.roundToInt()} ft"
}
}
private fun formatScaleDistance(distance: Double): String {
if (distance >= 10.0) {
return distance.roundToInt().toString()
}
val scaled = (distance * 100.0).roundToInt()
val whole = scaled / 100
val remainder = scaled % 100
return when {
remainder == 0 -> "$whole"
remainder % 10 == 0 -> "$whole.${remainder / 10}"
else -> "$whole.${remainder.toString().padStart(2, '0')}"
}
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This now uses pure Kotlin math and strings, with no java.* imports in the widget module. Values of 10 km/mi and above round to whole units; smaller values use one decimal after the precision follow-up. |
||||||||||||||||||||||||||||||||||||||||||
| * An animated scale bar that appears when the zoom level of the map changes, and then disappears | ||||||||||||||||||||||||||||||||||||||||||
| * after [visibilityDurationMillis]. This composable wraps [ScaleBar] with visibility animations. | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
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.
Calling
captureToImage()and writing PNGs togetExternalFilesDir(null)was a great way to visually verify the rendering during development! For the checked-in test suite, could we remove the bitmap capture and file write (along with thejava.io.Fileandjava.util.Localeimports) so the test stays fast, self-contained, and avoids unasserted disk I/O in CI?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.
Removed bitmap capture, file writes and locale mutation. In ee57e4c I moved the controlled-projection cases to Robolectric on API 29/35 because MockK’s Android agent crashes during API 29 activity cleanup. All eight cases pass. The device rendering checks now use a real, unbound camera state and pass on API 29.