From 54d4c78c2a8e5a722b1d2daf952dd53095494a23 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Thu, 8 Oct 2026 10:31:39 -0600 Subject: [PATCH 1/3] feat(clustering): propagate contentDescription to marker options for accessibility Support setting accessibility contentDescription on clustered map markers, resolving accessibility issues where TalkBack and UIAutomator announce generic "Map Marker" instead of meaningful pin information. - Add contentDescription to ClusteringMarkerProperties and its helper composable. - Propagate custom contentDescription or ClusterItem.title to MarkerOptions in ComposeUiClusterRenderer. - Add regression tests in GoogleMapViewClusteringTests verifying both default item title fallback and explicit custom contentDescription. Fixes #683, #706 --- .../compose/GoogleMapViewClusteringTests.kt | 86 +++++++++++++++++++ .../compose/clustering/ClusterRenderer.kt | 24 +++++- .../android/compose/clustering/Clustering.kt | 6 ++ 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt b/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt index c374b453a..2e0386f72 100644 --- a/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt +++ b/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt @@ -30,7 +30,9 @@ import androidx.compose.ui.unit.dp import com.google.android.gms.maps.model.CameraPosition import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.Marker +import com.google.android.gms.maps.model.MarkerOptions import com.google.common.truth.Truth.assertThat +import com.google.maps.android.clustering.ClusterItem import com.google.maps.android.clustering.ClusterManager import com.google.maps.android.compose.clustering.Clustering import com.google.maps.android.compose.clustering.ClusteringMarkerProperties @@ -198,4 +200,88 @@ class GoogleMapViewClusteringTests { assertThat(marker.isVisible).isTrue() } } + + private fun MarkerOptions.getContentDescription(): String? { + val field = MarkerOptions::class.java.declaredFields.firstOrNull { + it.type == String::class.java && it.name != "zzb" && it.name != "zzc" + } ?: MarkerOptions::class.java.getDeclaredField("zzr") + field.isAccessible = true + return field.get(this) as? String + } + + @OptIn(MapsComposeExperimentalApi::class) + @Test + fun testClusteringPropagatesItemTitleToMarkerContentDescription() { + val clusterManagerHolder = arrayOfNulls>(1) + val items = listOf(MyItem(startingPosition, "Store Location 42", "Snippet", 0f)) + + initMapAndGetMarker(clusterManagerHolder) { + Clustering( + items = items, + onClusterManager = { cm -> + clusterManagerHolder[0] = cm + } + ) + } + + composeTestRule.runOnUiThread { + val cm = clusterManagerHolder[0]!! + val renderer = cm.renderer + val method = renderer.javaClass.methods.firstOrNull { + it.name == "onBeforeClusterItemRendered" && it.parameterTypes.size == 2 + } ?: renderer.javaClass.getDeclaredMethod( + "onBeforeClusterItemRendered", + ClusterItem::class.java, + MarkerOptions::class.java + ).apply { isAccessible = true } + + val markerOptions = MarkerOptions() + method.invoke(renderer, items.first(), markerOptions) + + // Regression test for #683 / #706: Clustered pins currently do not set contentDescription, + // resulting in TalkBack announcing generic "Map Marker" instead of the pin title. + assertThat(markerOptions.getContentDescription()).isEqualTo("Store Location 42") + } + } + + @OptIn(MapsComposeExperimentalApi::class) + @Test + fun testClusteringMarkerPropertiesCustomContentDescription() { + val clusterManagerHolder = arrayOfNulls>(1) + val items = listOf(MyItem(startingPosition, "Store Location 42", "Snippet", 0f)) + + initMapAndGetMarker(clusterManagerHolder) { + Clustering( + items = items, + clusterItemContent = { + ClusteringMarkerProperties( + contentDescription = "Custom Accessibility Pin Description" + ) + Surface(modifier = Modifier.size(20.dp)) { + Text("X") + } + }, + onClusterManager = { cm -> + clusterManagerHolder[0] = cm + } + ) + } + + composeTestRule.runOnUiThread { + val cm = clusterManagerHolder[0]!! + val renderer = cm.renderer + val method = renderer.javaClass.methods.firstOrNull { + it.name == "onBeforeClusterItemRendered" && it.parameterTypes.size == 2 + } ?: renderer.javaClass.getDeclaredMethod( + "onBeforeClusterItemRendered", + ClusterItem::class.java, + MarkerOptions::class.java + ).apply { isAccessible = true } + + val markerOptions = MarkerOptions() + method.invoke(renderer, items.first(), markerOptions) + + assertThat(markerOptions.getContentDescription()).isEqualTo("Custom Accessibility Pin Description") + } + } } diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt index cf37d5174..5ab62bef3 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt @@ -300,6 +300,11 @@ internal class ComposeUiClusterRenderer( markerOptions.anchor(anchor.x, anchor.y) markerOptions.zIndex(props?.zIndex ?: clusterContentZIndexState.value) markerOptions.rotation(props?.rotation ?: clusterContentRotationState.value) + props?.contentDescription?.let { + if (it.isNotEmpty()) { + markerOptions.contentDescription(it) + } + } } } @@ -324,7 +329,7 @@ internal class ComposeUiClusterRenderer( super.onBeforeClusterItemRendered(item, markerOptions) if (!scope.isActive) return - if (clusterItemContentState.value != null) { + val contentDesc = if (clusterItemContentState.value != null) { val viewInfo = keysToViews[ViewKey.Item(item)] ?: createAndAddView(ViewKey.Item(item)) markerOptions.icon(renderViewToBitmapDescriptor(viewInfo.view)) @@ -333,6 +338,13 @@ internal class ComposeUiClusterRenderer( markerOptions.anchor(anchor.x, anchor.y) markerOptions.zIndex(props.zIndex ?: clusterItemContentZIndexState.value) markerOptions.rotation(props.rotation ?: clusterItemContentRotationState.value) + props.contentDescription ?: item.title + } else { + item.title + } + + if (!contentDesc.isNullOrEmpty()) { + markerOptions.contentDescription(contentDesc) } } @@ -394,7 +406,15 @@ internal class ComposeUiClusterRenderer( val rotation = getRotationOverride() val anchor = getAnchor() val zIndex = getZIndex() - LaunchedEffect(properties.anchor, properties.zIndex, properties.rotation, rotation, anchor, zIndex) { + LaunchedEffect( + properties.anchor, + properties.zIndex, + properties.rotation, + properties.contentDescription, + rotation, + anchor, + zIndex + ) { invalidate() } CompositionLocalProvider( diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt index f65dbdf1a..40cf8c494 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt @@ -60,6 +60,8 @@ public class ClusteringMarkerProperties { internal set public var rotation: Float? by mutableStateOf(null) internal set + public var contentDescription: String? by mutableStateOf(null) + internal set } /** @@ -78,18 +80,22 @@ public val LocalClusteringMarkerProperties: androidx.compose.runtime.ProvidableC * will be used. * @param rotation the rotation of the marker in degrees clockwise about the marker's anchor point. * If null, the default rotation specified in [Clustering] will be used. + * @param contentDescription the content description for accessibility purposes. If null, the title + * of the [ClusterItem] will be used as a fallback for cluster items. */ @Composable public fun ClusteringMarkerProperties( anchor: Offset? = null, zIndex: Float? = null, rotation: Float? = null, + contentDescription: String? = null, ) { val properties = LocalClusteringMarkerProperties.current SideEffect { properties.anchor = anchor properties.zIndex = zIndex properties.rotation = rotation + properties.contentDescription = contentDescription } } From 3144c9516e25bb9b5fe182e05b5bc141380603f8 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:29:23 -0600 Subject: [PATCH 2/3] fix(clustering): address reviewer feedback for binary compatibility and test robustness - Retain 3-parameter ClusteringMarkerProperties overload marked @Deprecated(level = DeprecationLevel.HIDDEN) for binary compatibility. - Extract description resolution logic to internal resolveItemContentDescription and resolveClusterContentDescription functions with dedicated unit tests. - Remove obfuscated Proguard field references in GoogleMapViewClusteringTests and ensure distinct title vs description values. --- .../compose/GoogleMapViewClusteringTests.kt | 48 ++++++++--- .../compose/clustering/ClusterRenderer.kt | 29 +++++-- .../android/compose/clustering/Clustering.kt | 23 ++++++ .../ClusterRendererContentDescriptionTest.kt | 80 +++++++++++++++++++ 4 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt diff --git a/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt b/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt index 2e0386f72..017864bae 100644 --- a/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt +++ b/maps-app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewClusteringTests.kt @@ -201,12 +201,34 @@ class GoogleMapViewClusteringTests { } } - private fun MarkerOptions.getContentDescription(): String? { - val field = MarkerOptions::class.java.declaredFields.firstOrNull { - it.type == String::class.java && it.name != "zzb" && it.name != "zzc" - } ?: MarkerOptions::class.java.getDeclaredField("zzr") - field.isAccessible = true - return field.get(this) as? String + /** + * Inspects non-null String fields on [MarkerOptions] to extract contentDescription + * dynamically without hardcoding obfuscated Play services field names. + */ + private fun MarkerOptions.findCustomContentDescription(): String? { + for (field in MarkerOptions::class.java.declaredFields) { + if (field.type == String::class.java) { + field.isAccessible = true + val value = field.get(this) as? String + if (value != null && value != this.title && value != this.snippet) { + return value + } + } + } + return null + } + + private fun MarkerOptions.countStringFieldOccurrences(target: String): Int { + var count = 0 + for (field in MarkerOptions::class.java.declaredFields) { + if (field.type == String::class.java) { + field.isAccessible = true + if (field.get(this) == target) { + count++ + } + } + } + return count } @OptIn(MapsComposeExperimentalApi::class) @@ -216,7 +238,7 @@ class GoogleMapViewClusteringTests { val items = listOf(MyItem(startingPosition, "Store Location 42", "Snippet", 0f)) initMapAndGetMarker(clusterManagerHolder) { - Clustering( + Clustering( items = items, onClusterManager = { cm -> clusterManagerHolder[0] = cm @@ -238,9 +260,10 @@ class GoogleMapViewClusteringTests { val markerOptions = MarkerOptions() method.invoke(renderer, items.first(), markerOptions) - // Regression test for #683 / #706: Clustered pins currently do not set contentDescription, - // resulting in TalkBack announcing generic "Map Marker" instead of the pin title. - assertThat(markerOptions.getContentDescription()).isEqualTo("Store Location 42") + // When no custom description is supplied, fallback to item.title is populated on MarkerOptions. + // Both title and contentDescription fields hold "Store Location 42" (count == 2), + // whereas before the fix contentDescription remained null (count == 1). + assertThat(markerOptions.countStringFieldOccurrences("Store Location 42")).isEqualTo(2) } } @@ -248,10 +271,11 @@ class GoogleMapViewClusteringTests { @Test fun testClusteringMarkerPropertiesCustomContentDescription() { val clusterManagerHolder = arrayOfNulls>(1) + // Item title is deliberately distinct from the custom contentDescription val items = listOf(MyItem(startingPosition, "Store Location 42", "Snippet", 0f)) initMapAndGetMarker(clusterManagerHolder) { - Clustering( + Clustering( items = items, clusterItemContent = { ClusteringMarkerProperties( @@ -281,7 +305,7 @@ class GoogleMapViewClusteringTests { val markerOptions = MarkerOptions() method.invoke(renderer, items.first(), markerOptions) - assertThat(markerOptions.getContentDescription()).isEqualTo("Custom Accessibility Pin Description") + assertThat(markerOptions.findCustomContentDescription()).isEqualTo("Custom Accessibility Pin Description") } } } diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt index 5ab62bef3..7a12547e1 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt @@ -300,10 +300,9 @@ internal class ComposeUiClusterRenderer( markerOptions.anchor(anchor.x, anchor.y) markerOptions.zIndex(props?.zIndex ?: clusterContentZIndexState.value) markerOptions.rotation(props?.rotation ?: clusterContentRotationState.value) - props?.contentDescription?.let { - if (it.isNotEmpty()) { - markerOptions.contentDescription(it) - } + val contentDesc = resolveClusterContentDescription(cluster, props?.contentDescription) + if (!contentDesc.isNullOrEmpty()) { + markerOptions.contentDescription(contentDesc) } } } @@ -338,9 +337,9 @@ internal class ComposeUiClusterRenderer( markerOptions.anchor(anchor.x, anchor.y) markerOptions.zIndex(props.zIndex ?: clusterItemContentZIndexState.value) markerOptions.rotation(props.rotation ?: clusterItemContentRotationState.value) - props.contentDescription ?: item.title + resolveItemContentDescription(item, props.contentDescription) } else { - item.title + resolveItemContentDescription(item, null) } if (!contentDesc.isNullOrEmpty()) { @@ -434,5 +433,21 @@ internal class ComposeUiClusterRenderer( onInvalidate?.invoke() } } - } + +/** + * Resolves the accessibility content description for an individual [ClusterItem]. + * If a custom [customContentDescription] is provided, it takes precedence; otherwise falls back to [ClusterItem.getTitle]. + */ +internal fun resolveItemContentDescription( + item: T, + customContentDescription: String?, +): String? = customContentDescription ?: item.title + +/** + * Resolves the accessibility content description for a [Cluster]. + */ +internal fun resolveClusterContentDescription( + cluster: Cluster, + customContentDescription: String?, +): String? = customContentDescription diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt index 40cf8c494..6fa5682e8 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt @@ -99,6 +99,29 @@ public fun ClusteringMarkerProperties( } } +/** + * Helper function to specify properties for the marker representing a cluster or cluster item. + * + * Retained for binary compatibility with binaries compiled against earlier versions. + */ +@Deprecated( + message = "Retained for binary compatibility.", + level = DeprecationLevel.HIDDEN, +) +@Composable +public fun ClusteringMarkerProperties( + anchor: Offset? = null, + zIndex: Float? = null, + rotation: Float? = null, +) { + ClusteringMarkerProperties( + anchor = anchor, + zIndex = zIndex, + rotation = rotation, + contentDescription = null, + ) +} + /** * Groups many items on a map based on zoom level. * diff --git a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt new file mode 100644 index 000000000..5a7ed094f --- /dev/null +++ b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt @@ -0,0 +1,80 @@ +/* + * 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.clustering + +import com.google.android.gms.maps.model.LatLng +import com.google.maps.android.clustering.Cluster +import com.google.maps.android.clustering.ClusterItem +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +public class ClusterRendererContentDescriptionTest { + + private class TestClusterItem( + override val position: LatLng, + override val title: String?, + override val snippet: String? = null, + override val zIndex: Float? = null, + ) : ClusterItem + + private class TestCluster( + override val items: Collection, + override val position: LatLng, + ) : Cluster { + override val size: Int get() = items.size + } + + @Test + public fun testResolveItemContentDescriptionFallsBackToItemTitleWhenNull() { + val item = TestClusterItem(LatLng(1.0, 2.0), title = "Store 42") + val result = resolveItemContentDescription(item, customContentDescription = null) + assertEquals("Store 42", result) + } + + @Test + public fun testResolveItemContentDescriptionCustomOverridesItemTitle() { + val item = TestClusterItem(LatLng(1.0, 2.0), title = "Store 42") + val result = resolveItemContentDescription( + item, + customContentDescription = "Custom Accessibility Pin" + ) + assertEquals("Custom Accessibility Pin", result) + } + + @Test + public fun testResolveItemContentDescriptionReturnsNullWhenBothNull() { + val item = TestClusterItem(LatLng(1.0, 2.0), title = null) + val result = resolveItemContentDescription(item, customContentDescription = null) + assertNull(result) + } + + @Test + public fun testResolveClusterContentDescription() { + val item = TestClusterItem(LatLng(1.0, 2.0), title = "Item") + val cluster = TestCluster(listOf(item), LatLng(1.0, 2.0)) + + val customResult = resolveClusterContentDescription( + cluster, + customContentDescription = "Cluster of 1 location" + ) + assertEquals("Cluster of 1 location", customResult) + + val defaultResult = resolveClusterContentDescription(cluster, customContentDescription = null) + assertNull(defaultResult) + } +} From d4dc6b314d4cd36e1fad1f2a85ce2b5a1fb82f52 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:42:43 -0600 Subject: [PATCH 3/3] refactor(clustering): inline cluster contentDescription lookup --- .../compose/clustering/ClusterRenderer.kt | 9 +------ .../ClusterRendererContentDescriptionTest.kt | 25 +------------------ 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt index 7a12547e1..ae42ae417 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt @@ -300,7 +300,7 @@ internal class ComposeUiClusterRenderer( markerOptions.anchor(anchor.x, anchor.y) markerOptions.zIndex(props?.zIndex ?: clusterContentZIndexState.value) markerOptions.rotation(props?.rotation ?: clusterContentRotationState.value) - val contentDesc = resolveClusterContentDescription(cluster, props?.contentDescription) + val contentDesc = props?.contentDescription if (!contentDesc.isNullOrEmpty()) { markerOptions.contentDescription(contentDesc) } @@ -444,10 +444,3 @@ internal fun resolveItemContentDescription( customContentDescription: String?, ): String? = customContentDescription ?: item.title -/** - * Resolves the accessibility content description for a [Cluster]. - */ -internal fun resolveClusterContentDescription( - cluster: Cluster, - customContentDescription: String?, -): String? = customContentDescription diff --git a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt index 5a7ed094f..85fb1a24e 100644 --- a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt +++ b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererContentDescriptionTest.kt @@ -17,7 +17,6 @@ package com.google.maps.android.compose.clustering import com.google.android.gms.maps.model.LatLng -import com.google.maps.android.clustering.Cluster import com.google.maps.android.clustering.ClusterItem import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -32,13 +31,6 @@ public class ClusterRendererContentDescriptionTest { override val zIndex: Float? = null, ) : ClusterItem - private class TestCluster( - override val items: Collection, - override val position: LatLng, - ) : Cluster { - override val size: Int get() = items.size - } - @Test public fun testResolveItemContentDescriptionFallsBackToItemTitleWhenNull() { val item = TestClusterItem(LatLng(1.0, 2.0), title = "Store 42") @@ -51,7 +43,7 @@ public class ClusterRendererContentDescriptionTest { val item = TestClusterItem(LatLng(1.0, 2.0), title = "Store 42") val result = resolveItemContentDescription( item, - customContentDescription = "Custom Accessibility Pin" + customContentDescription = "Custom Accessibility Pin", ) assertEquals("Custom Accessibility Pin", result) } @@ -62,19 +54,4 @@ public class ClusterRendererContentDescriptionTest { val result = resolveItemContentDescription(item, customContentDescription = null) assertNull(result) } - - @Test - public fun testResolveClusterContentDescription() { - val item = TestClusterItem(LatLng(1.0, 2.0), title = "Item") - val cluster = TestCluster(listOf(item), LatLng(1.0, 2.0)) - - val customResult = resolveClusterContentDescription( - cluster, - customContentDescription = "Cluster of 1 location" - ) - assertEquals("Cluster of 1 location", customResult) - - val defaultResult = resolveClusterContentDescription(cluster, customContentDescription = null) - assertNull(defaultResult) - } }