Skip to content

Commit 33af59b

Browse files
pengdevgithub-actions[bot]
authored andcommitted
Fix attribution state updates to cover runtime added sources. (#8613)
This PR improves the attribution refresh mechanism by fixing when and how attributions are updated, moving shared logic to common modules, and ensuring proper ordering of attribution items. ## Attribution Refresh Logic Improvements ### When attributions are refreshed: - Style data loaded - Refreshes attributions when the map style is loaded/changed - Source added and source metadata loaded - Updates when new sources are added at runtime and their metadata becomes available - Source removed - Refreshes when sources are removed from the map - Initial load - Shows Mapbox telemetry/feedback attributions even without custom sources ## API Changes - Attribution.isMapboxFeedback() extension function moved from extension-compose to sdk-base - Now available to all modules, not just Compose ## Attribution ordering for both Compose and non-compose users - Mapbox special attributions (telemetry, privacy, feedback, geofencing) now appear at the bottom of the list - Custom source attributions appear first, following natural priority More context in https://mapbox.slack.com/archives/CFN66JMMK/p1764944165105729?thread_ts=1764765065.988139&cid=CFN66JMMK cc @mapbox/maps-android GitOrigin-RevId: 88272fe77be539ef42996e6a9f59c5ef34eafa46
1 parent fb55bd1 commit 33af59b

File tree

9 files changed

+183
-60
lines changed

9 files changed

+183
-60
lines changed

compose-app/src/main/java/com/mapbox/maps/compose/testapp/examples/ornaments/CustomAttributionActivity.kt

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,18 @@ import com.mapbox.maps.compose.testapp.ui.theme.MapboxMapComposeTheme
4545
import com.mapbox.maps.extension.compose.MapboxMap
4646
import com.mapbox.maps.extension.compose.animation.viewport.rememberMapViewportState
4747
import com.mapbox.maps.extension.compose.ornaments.attribution.MapAttributionScope
48-
import com.mapbox.maps.extension.compose.ornaments.attribution.isMapboxFeedback
48+
import com.mapbox.maps.extension.compose.style.DoubleListValue
49+
import com.mapbox.maps.extension.compose.style.DoubleValue
50+
import com.mapbox.maps.extension.compose.style.LongValue
51+
import com.mapbox.maps.extension.compose.style.StringListValue
52+
import com.mapbox.maps.extension.compose.style.StringValue
53+
import com.mapbox.maps.extension.compose.style.layers.generated.RasterLayer
54+
import com.mapbox.maps.extension.compose.style.sources.generated.SchemeValue
55+
import com.mapbox.maps.extension.compose.style.sources.generated.rememberRasterSourceState
4956
import com.mapbox.maps.extension.compose.style.standard.MapboxStandardSatelliteStyle
5057
import com.mapbox.maps.extension.compose.style.standard.MapboxStandardStyle
5158
import com.mapbox.maps.plugin.attribution.Attribution
59+
import com.mapbox.maps.plugin.attribution.isMapboxFeedback
5260
import kotlinx.coroutines.launch
5361

5462
/**
@@ -73,6 +81,9 @@ public class CustomAttributionActivity : ComponentActivity() {
7381
var showSatelliteStyle: Boolean by remember {
7482
mutableStateOf(false)
7583
}
84+
var showOSMLayer: Boolean by remember {
85+
mutableStateOf(false)
86+
}
7687
val coroutineScope = rememberCoroutineScope()
7788

7889
var userConsentState: MapAttributionScope.UserConsentState? by remember {
@@ -121,6 +132,19 @@ public class CustomAttributionActivity : ComponentActivity() {
121132
text = if (showSatelliteStyle) "Show Satellite Style" else "Show Standard Style"
122133
)
123134
}
135+
FloatingActionButton(
136+
modifier = Modifier
137+
.padding(bottom = 10.dp),
138+
onClick = {
139+
showOSMLayer = !showOSMLayer
140+
},
141+
shape = RoundedCornerShape(16.dp),
142+
) {
143+
Text(
144+
modifier = Modifier.padding(10.dp),
145+
text = if (!showOSMLayer) "Show OSM layer" else "Hide OSM layer"
146+
)
147+
}
124148
}
125149
},
126150
floatingActionButtonPosition = FabPosition.End
@@ -158,7 +182,22 @@ public class CustomAttributionActivity : ComponentActivity() {
158182
MapboxStandardStyle()
159183
}
160184
}
161-
)
185+
) {
186+
if (showOSMLayer) {
187+
val source = rememberRasterSourceState {
188+
tileSize = LongValue(RASTER_TILE_SIZE_PIXELS)
189+
tiles = StringListValue(listOf(OSM_RASTER_TILE_URL))
190+
attribution = StringValue(TILE_JSON_ATTRIBUTION)
191+
scheme = SchemeValue.XYZ
192+
minZoom = LongValue(TILE_JSON_MIN_ZOOM)
193+
maxZoom = LongValue(TILE_JSON_MAX_ZOOM)
194+
bounds = DoubleListValue(MERCATOR_BOUNDS)
195+
}
196+
RasterLayer(source) {
197+
rasterOpacity = DoubleValue(0.7)
198+
}
199+
}
200+
}
162201
}
163202
}
164203
}
@@ -330,5 +369,16 @@ public class CustomAttributionActivity : ComponentActivity() {
330369

331370
private companion object {
332371
const val ZOOM: Double = 9.0
372+
const val TILE_JSON_VERSION = "2.0.0"
373+
const val TILE_JSON_NAME = "OpenStreetMap"
374+
const val TILE_JSON_DESCRIPTION = "A free editable map of the whole world."
375+
const val TILE_JSON_ATTRIBUTION = "© OpenStreetMap contributors, CC-BY-SA"
376+
const val TILE_JSON_MIN_ZOOM = 0L
377+
const val TILE_JSON_MAX_ZOOM = 18L
378+
379+
const val OSM_RASTER_TILE_URL = "https://tile.openstreetmap.org/{z}/{x}/{y}.png"
380+
const val RASTER_TILE_SIZE_PIXELS = 256L
381+
val MERCATOR_BOUNDS = listOf(-180.0, -85.0, 180.0, 85.0)
382+
val CENTER_MAP_LOCATION = listOf(11.9, 57.7, 8.0)
333383
}
334384
}

extension-compose/api/Release/metalava.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,10 +1213,6 @@ package com.mapbox.maps.extension.compose.ornaments.attribution {
12131213
property public final boolean telemetryEnableState;
12141214
}
12151215

1216-
public final class MapAttributionScopeKt {
1217-
method @com.mapbox.annotation.MapboxExperimental public static boolean isMapboxFeedback(com.mapbox.maps.plugin.attribution.Attribution);
1218-
}
1219-
12201216
}
12211217

12221218
package com.mapbox.maps.extension.compose.ornaments.compass {

extension-compose/api/extension-compose.api

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -953,10 +953,6 @@ public final class com/mapbox/maps/extension/compose/ornaments/attribution/MapAt
953953
public final synthetic fun setTelemetryEnableState (Z)V
954954
}
955955

956-
public final class com/mapbox/maps/extension/compose/ornaments/attribution/MapAttributionScopeKt {
957-
public static final fun isMapboxFeedback (Lcom/mapbox/maps/plugin/attribution/Attribution;)Z
958-
}
959-
960956
public final class com/mapbox/maps/extension/compose/ornaments/compass/ComposableSingletons$MapCompassScopeKt {
961957
public static final field INSTANCE Lcom/mapbox/maps/extension/compose/ornaments/compass/ComposableSingletons$MapCompassScopeKt;
962958
public static field lambda-1 Lkotlin/jvm/functions/Function2;

extension-compose/src/main/java/com/mapbox/maps/extension/compose/ornaments/attribution/MapAttributionScope.kt

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ import androidx.compose.ui.unit.sp
4848
import androidx.compose.ui.window.DialogProperties
4949
import com.mapbox.annotation.MapboxDelicateApi
5050
import com.mapbox.annotation.MapboxExperimental
51+
import com.mapbox.common.Cancelable
5152
import com.mapbox.common.geofencing.GeofencingError
5253
import com.mapbox.maps.MapView
53-
import com.mapbox.maps.coroutine.styleDataLoadedEvents
54+
import com.mapbox.maps.SourceDataLoadedType
5455
import com.mapbox.maps.extension.compose.MapboxMapScopeMarker
5556
import com.mapbox.maps.extension.compose.R
5657
import com.mapbox.maps.extension.compose.ornaments.attribution.internal.AttributionComposePlugin
@@ -60,6 +61,7 @@ import com.mapbox.maps.logW
6061
import com.mapbox.maps.plugin.Plugin
6162
import com.mapbox.maps.plugin.attribution.Attribution
6263
import com.mapbox.maps.plugin.attribution.AttributionParserConfig
64+
import com.mapbox.maps.plugin.attribution.isMapboxFeedback
6365
import kotlinx.coroutines.flow.MutableStateFlow
6466
import kotlinx.coroutines.flow.filterNotNull
6567
import kotlinx.coroutines.flow.first
@@ -636,36 +638,93 @@ public class MapAttributionScope internal constructor(
636638

637639
@Composable
638640
private fun UpdateAttributionState(mapView: MapView, plugin: AttributionComposePlugin?) {
641+
// Update user consent state for the first time
639642
LaunchedEffect(plugin) {
640643
if (plugin != null) {
641-
// Update user consent state for the first time
642644
_userConsentState.value = UserConsentState.Builder()
643645
.setTelemetryEnableState(plugin.mapAttributionDelegate.telemetry().userTelemetryRequestState)
644646
.setGeofencingUserConsentState(
645647
plugin.mapAttributionDelegate.geofencingConsent().getUserConsent()
646648
)
647649
.build()
648-
logD(TAG, "${plugin.hashCode()} Load initial user consent state: $userConsentState ")
649-
650-
// Update attributions when style data is added
651-
mapView.mapboxMap.styleDataLoadedEvents.collect { event ->
652-
// TO BE FIXED by Native, the SourceDataLoadedEvents with metadata type was never emitted
653-
// mapView.mapboxMap.sourceDataLoadedEvents.collect { event ->
654-
// if (event.type == SourceDataLoadedType.METADATA) {
655-
logD(TAG, "${plugin.hashCode()} style data loaded, refresh attributions")
656-
_attributions.value = plugin.mapAttributionDelegate.parseAttributions(
657-
mapView.context,
658-
AttributionParserConfig()
659-
).toMutableList().apply {
660-
find { it.isMapboxFeedback() }?.let {
661-
remove(it)
662-
add(it.copy(url = plugin.mapAttributionDelegate.buildMapBoxFeedbackUrl(mapView.context)))
663-
}
664-
}.toList()
665-
// }
650+
logD(
651+
TAG,
652+
"AttributionComposePlugin(${plugin.hashCode()}) Load initial user consent state: $userConsentState "
653+
)
654+
}
655+
}
656+
657+
// Refresh the attributions when style changes, as for Standard style, the source events are hidden
658+
DisposableEffect(plugin) {
659+
var styleDataLoadedCancelable: Cancelable? = null
660+
if (plugin != null) {
661+
// Refresh the attribution initially to show the Mapbox telemetry, feedback attributions etc, even without source attributions
662+
updateAttributionState(mapView, plugin)
663+
// Update attributions when style data is loaded
664+
styleDataLoadedCancelable = mapView.mapboxMap.subscribeStyleDataLoaded {
665+
logD(
666+
TAG,
667+
"AttributionComposePlugin(${plugin.hashCode()}) style loaded, refresh attributions"
668+
)
669+
updateAttributionState(mapView, plugin)
666670
}
667671
}
672+
onDispose {
673+
styleDataLoadedCancelable?.cancel()
674+
styleDataLoadedCancelable = null
675+
}
668676
}
677+
678+
// Refresh the attributions when source is added or removed in the runtime
679+
DisposableEffect(plugin) {
680+
var sourceAddedCancellable: Cancelable? = null
681+
var sourceRemovedCancellable: Cancelable? = null
682+
val sourceDataLoadedCancellable = hashMapOf<String, Cancelable>()
683+
if (plugin != null) {
684+
mapView.mapboxMap.apply {
685+
// Update attributions when new source is added in the runtime
686+
sourceAddedCancellable = subscribeSourceAdded { sourceAdded ->
687+
sourceDataLoadedCancellable[sourceAdded.sourceId] =
688+
subscribeSourceDataLoaded {
689+
// refresh the attribution only when the source metadata is loaded, otherwise the new attribution is not yet parsed
690+
if (it.type == SourceDataLoadedType.METADATA && it.sourceId == sourceAdded.sourceId) {
691+
logD(
692+
TAG,
693+
"AttributionComposePlugin(${plugin.hashCode()}) source metadata loaded, refresh attributions"
694+
)
695+
updateAttributionState(mapView, plugin)
696+
sourceDataLoadedCancellable.remove(sourceAdded.sourceId)?.cancel()
697+
}
698+
}
699+
}
700+
// Update attribution when the source is removed
701+
sourceRemovedCancellable = subscribeSourceRemoved {
702+
updateAttributionState(mapView, plugin)
703+
}
704+
}
705+
}
706+
onDispose {
707+
sourceAddedCancellable?.cancel()
708+
sourceAddedCancellable = null
709+
sourceRemovedCancellable?.cancel()
710+
sourceRemovedCancellable = null
711+
sourceDataLoadedCancellable.values.forEach { it.cancel() }
712+
sourceDataLoadedCancellable.clear()
713+
}
714+
}
715+
}
716+
717+
private fun updateAttributionState(mapView: MapView, plugin: AttributionComposePlugin) {
718+
_attributions.value = plugin.mapAttributionDelegate.parseAttributions(
719+
mapView.context,
720+
AttributionParserConfig()
721+
).toMutableList().apply {
722+
find { it.isMapboxFeedback() }?.let {
723+
remove(it)
724+
add(it.copy(url = plugin.mapAttributionDelegate.buildMapBoxFeedbackUrl(mapView.context)))
725+
}
726+
}
727+
.toList()
669728
}
670729

671730
@Composable
@@ -874,24 +933,4 @@ public class MapAttributionScope internal constructor(
874933
return "$PLUGIN_ID-${INSTANCE_COUNT++}"
875934
}
876935
}
877-
}
878-
879-
/**
880-
* Determines if this attribution entry represents a Mapbox feedback link.
881-
*
882-
* Mapbox feedback attributions are special entries that allow users to provide feedback
883-
* about map data and require dynamic URL generation with current map state parameters.
884-
*
885-
* ## Implementation Notes
886-
* This function checks for Mapbox domain and feedback-related keywords in the attribution URL.
887-
* When true, the URL should be built using [AttributionState.buildMapboxFeedbackUrl()] rather
888-
* than using the attribution's static URL directly.
889-
*
890-
* @return true if this attribution represents a Mapbox feedback link that requires
891-
* dynamic URL generation, false otherwise.
892-
*/
893-
@MapboxExperimental
894-
public fun Attribution.isMapboxFeedback(): Boolean {
895-
return url.startsWith("https://mapbox.com/") &&
896-
(url.contains("/feedback") || url.contains("/contribute"))
897936
}

maps-sdk/src/main/java/com/mapbox/maps/plugin/MapAttributionDelegateImpl.kt

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.mapbox.maps.geofencing.MapGeofencingConsent
1111
import com.mapbox.maps.module.MapTelemetry
1212
import com.mapbox.maps.plugin.attribution.Attribution
1313
import com.mapbox.maps.plugin.attribution.AttributionParserConfig
14+
import com.mapbox.maps.plugin.attribution.isMapboxFeedback
1415
import com.mapbox.maps.plugin.delegates.MapAttributionDelegate
1516
import java.util.regex.Matcher
1617
import java.util.regex.Pattern
@@ -52,7 +53,7 @@ internal class MapAttributionDelegateImpl @OptIn(MapboxExperimental::class) cons
5253
*
5354
* @return the parsed attributions
5455
*/
55-
@OptIn(MapboxExperimental::class)
56+
@OptIn(MapboxExperimental::class, com.mapbox.annotation.MapboxExperimental::class)
5657
override fun parseAttributions(
5758
context: Context,
5859
config: AttributionParserConfig
@@ -68,7 +69,18 @@ internal class MapAttributionDelegateImpl @OptIn(MapboxExperimental::class) cons
6869
.withMapboxGeofencingConsent(config.withMapboxGeofencingConsent)
6970
.withAttributionData(*attributionsArray)
7071
.withExtraAttributions(extraAttributions)
71-
.build().getAttributions().toList()
72+
.build()
73+
.getAttributions()
74+
.sortedByDescending { attr ->
75+
// Sort attribution by putting Mapbox related special attributions at the bottom of the list
76+
when {
77+
attr.url == Attribution.ABOUT_TELEMETRY_URL -> -1
78+
attr.url == Attribution.GEOFENCING_URL_MARKER -> -2
79+
attr.url == Attribution.PRIVACY_POLICY_URL -> -3
80+
attr.isMapboxFeedback() -> -4
81+
else -> 0
82+
}
83+
}
7284
}
7385

7486
/**

maps-sdk/src/test/java/com/mapbox/maps/attribution/MapAttributionDelegateImplTest.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,41 +210,41 @@ class MapAttributionDelegateImplTest {
210210
}
211211
2 -> {
212212
assertEquals(
213-
"URL improve map should match", "https://www.mapbox.com/map-feedback/",
213+
"Telemetry URL should match", "https://www.mapbox.com/telemetry/",
214214
url
215215
)
216216
assertEquals(
217-
"Title improve map should match", "Improve This Map",
217+
"Telemetry title should match", "Mapbox Telemetry",
218218
title
219219
)
220220
}
221221
3 -> {
222222
assertEquals(
223-
"Telemetry URL should match", "https://www.mapbox.com/telemetry/",
223+
"Geofencing consent URL should match", Attribution.GEOFENCING_URL_MARKER,
224224
url
225225
)
226226
assertEquals(
227-
"Telemetry title should match", "Mapbox Telemetry",
227+
"Geofencing consent title should match", Attribution.GEOFENCING,
228228
title
229229
)
230230
}
231231
4 -> {
232232
assertEquals(
233-
"Geofencing consent URL should match", Attribution.GEOFENCING_URL_MARKER,
233+
"Telemetry URL should match", "https://www.mapbox.com/legal/privacy#product-privacy-policy/",
234234
url
235235
)
236236
assertEquals(
237-
"Geofencing consent title should match", Attribution.GEOFENCING,
237+
"Telemetry title should match", "Mapbox Privacy Policy",
238238
title
239239
)
240240
}
241241
5 -> {
242242
assertEquals(
243-
"Telemetry URL should match", "https://www.mapbox.com/legal/privacy#product-privacy-policy/",
243+
"URL improve map should match", "https://www.mapbox.com/map-feedback/",
244244
url
245245
)
246246
assertEquals(
247-
"Telemetry title should match", "Mapbox Privacy Policy",
247+
"Title improve map should match", "Improve This Map",
248248
title
249249
)
250250
}

sdk-base/api/Release/metalava.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,6 +1796,10 @@ package com.mapbox.maps.plugin.attribution {
17961796
method public void showAttribution(com.mapbox.maps.plugin.delegates.MapAttributionDelegate mapAttributionDelegate);
17971797
}
17981798

1799+
public final class AttributionKt {
1800+
method @com.mapbox.annotation.MapboxExperimental public static boolean isMapboxFeedback(com.mapbox.maps.plugin.attribution.Attribution);
1801+
}
1802+
17991803
public final class AttributionParserConfig {
18001804
ctor public AttributionParserConfig(boolean withImproveMap = true, boolean withCopyrightSign = true, boolean withTelemetryAttribution = true, boolean withMapboxAttribution = true, boolean withMapboxPrivacyPolicy = true, boolean withMapboxGeofencingConsent = true);
18011805
ctor public AttributionParserConfig(boolean withImproveMap = true, boolean withCopyrightSign = true, boolean withTelemetryAttribution = true, boolean withMapboxAttribution = true, boolean withMapboxPrivacyPolicy = true);

sdk-base/api/sdk-base.api

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,6 +1862,10 @@ public abstract interface class com/mapbox/maps/plugin/attribution/AttributionDi
18621862
public abstract fun showAttribution (Lcom/mapbox/maps/plugin/delegates/MapAttributionDelegate;)V
18631863
}
18641864

1865+
public final class com/mapbox/maps/plugin/attribution/AttributionKt {
1866+
public static final fun isMapboxFeedback (Lcom/mapbox/maps/plugin/attribution/Attribution;)Z
1867+
}
1868+
18651869
public final class com/mapbox/maps/plugin/attribution/AttributionParserConfig {
18661870
public fun <init> ()V
18671871
public fun <init> (Z)V

0 commit comments

Comments
 (0)