Skip to content

add support for attribution text #187

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Nov 9, 2023
Merged

add support for attribution text #187

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c8c491b
add support for attribution text
puneet-pdx Nov 3, 2023
adfad3d
combine attribution bar properties/events in a state object
puneet-pdx Nov 8, 2023
ec1d693
refactor mapview parameter list
puneet-pdx Nov 8, 2023
5e1c9fb
update collect lambda
puneet-pdx Nov 8, 2023
d5ef515
Update doc
puneet-pdx Nov 9, 2023
92a3831
Merge branch 'feature-branches/geo-compose' into puneet/2960_attribution
puneet-pdx Nov 9, 2023
d55816b
update doc
puneet-pdx Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions toolkit/geo-compose/src/main/java/com/arcgismaps/toolkit/geocompose/MapView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ import kotlinx.coroutines.launch
* @param viewLabelProperties the [ViewLabelProperties] used by the composable [com.arcgismaps.toolkit.geocompose.MapView]
* @param selectionProperties the [SelectionProperties] used by the composable [com.arcgismaps.toolkit.geocompose.MapView]
* @param wrapAroundMode the [WrapAroundMode] to specify whether continuous panning across the international date line is enabled
* @param attributionBarVisible specifies the visibility of the attribution bar
* @param onViewpointChanged lambda invoked when the viewpoint of the composable MapView has changed
* @param onAttributionTextChanged lambda invoked when the attribution text for the data that is currently displayed in the MapView has changed
* @param onInteractingChanged lambda invoked when the user starts and ends interacting with the composable MapView
* @param onRotate lambda invoked when a user performs a rotation gesture on the composable MapView
* @param onScale lambda invoked when a user performs a pinch gesture on the composable MapView
Expand All @@ -85,11 +87,13 @@ public fun MapView(
graphicsOverlays: GraphicsOverlayCollection = rememberGraphicsOverlayCollection(),
locationDisplay: LocationDisplay = rememberLocationDisplay(),
wrapAroundMode: WrapAroundMode = WrapAroundMode.EnabledWhenSupported,
attributionBarVisible: Boolean = true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the parameter be suffixed with "is"?

Suggested change
attributionBarVisible: Boolean = true,
isAttributionBarVisible: Boolean = true,

geometryEditor: GeometryEditor? = null,
mapViewInteractionOptions: MapViewInteractionOptions = MapViewInteractionOptions(),
viewLabelProperties: ViewLabelProperties = ViewLabelProperties(),
selectionProperties: SelectionProperties = SelectionProperties(),
onViewpointChanged: (() -> Unit)? = null,
onAttributionTextChanged: ((String) -> Unit)? = null,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a general question, what was the reasoning behind using null rather than {} for the default values of event params?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment could be helpful: #159 (comment)

Defaulting the parameter to null is the correct thing to do. It expresses to the user that the parameter is "absent". See Compose API guidelines.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I suppose the difference between these lambdas and Button's onClick is that a Button must have an onClick event, whereas we don't expect users to always do something when these events are received.

onInteractingChanged: ((isInteracting: Boolean) -> Unit)? = null,
onRotate: ((RotationChangeEvent) -> Unit)? = null,
onScale: ((ScaleChangeEvent) -> Unit)? = null,
Expand Down Expand Up @@ -131,9 +135,16 @@ public fun MapView(
}
}

val currentAttributionBarVisible by rememberUpdatedState(attributionBarVisible)
LaunchedEffect(Unit) {
// isAttributionBarVisible does not take effect if applied in the AndroidView update callback
mapView.isAttributionBarVisible = currentAttributionBarVisible
}

MapViewEventHandler(
mapView,
onViewpointChanged,
onAttributionTextChanged,
onInteractingChanged,
onRotate,
onScale,
Expand All @@ -156,6 +167,7 @@ public fun MapView(
private fun MapViewEventHandler(
mapView: MapView,
onViewpointChanged: (() -> Unit)?,
onAttributionTextChanged: ((String) -> Unit)?,
onInteractingChanged: ((isInteracting: Boolean) -> Unit)?,
onRotate: ((RotationChangeEvent) -> Unit)?,
onScale: ((ScaleChangeEvent) -> Unit)?,
Expand All @@ -168,6 +180,7 @@ private fun MapViewEventHandler(
onPan: ((PanChangeEvent) -> Unit)?
) {
val currentViewPointChanged by rememberUpdatedState(onViewpointChanged)
val currentOnAttributionTextChanged by rememberUpdatedState(onAttributionTextChanged)
val currentOnInteractingChanged by rememberUpdatedState(onInteractingChanged)
val currentOnRotate by rememberUpdatedState(onRotate)
val currentOnScale by rememberUpdatedState(onScale)
Expand All @@ -187,6 +200,13 @@ private fun MapViewEventHandler(
}
}
}
launch {
mapView.attributionText.collect { attributionText ->
currentOnAttributionTextChanged?.let {
it(attributionText)
}
}
}
launch(Dispatchers.Main.immediate) {
mapView.isInteracting.collect { isInteracting ->
currentOnInteractingChanged?.let {
Expand Down