Skip to content
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

chore: Change the confusing parameter position to a clear name. #637 #638

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class AccessibilityActivity : ComponentActivity() {
super.onCreate(savedInstanceState)
enableEdgeToEdge()
setContent {
val singaporeState = rememberMarkerState(position = singapore)
val singaporeState = rememberMarkerState(initialPosition = singapore)
val cameraPositionState = rememberCameraPositionState {
position = defaultCameraPosition
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ fun GoogleMapView(
mapColorScheme: ComposeMapColorScheme = ComposeMapColorScheme.FOLLOW_SYSTEM,
content: @Composable () -> Unit = {}
) {
val singaporeState = rememberMarkerState(position = singapore)
val singapore2State = rememberMarkerState(position = singapore2)
val singapore3State = rememberMarkerState(position = singapore3)
val singapore4State = rememberMarkerState(position = singapore4)
val singapore5State = rememberMarkerState(position = singapore5)
val singaporeState = rememberMarkerState(initialPosition = singapore)
val singapore2State = rememberMarkerState(initialPosition = singapore2)
val singapore3State = rememberMarkerState(initialPosition = singapore3)
val singapore4State = rememberMarkerState(initialPosition = singapore4)
val singapore5State = rememberMarkerState(initialPosition = singapore5)

var circleCenter by remember { mutableStateOf(singapore) }
if (!singaporeState.isDragging) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ private fun GoogleMapViewInColumn(
cameraPositionState: CameraPositionState,
onMapLoaded: () -> Unit,
) {
val singaporeState = rememberMarkerState(position = singapore)
val singaporeState = rememberMarkerState(initialPosition = singapore)

var uiSettings by remember { mutableStateOf(MapUiSettings(compassEnabled = false)) }
var mapProperties by remember {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class RecompositionActivity : ComponentActivity() {
cameraPositionState: CameraPositionState = rememberCameraPositionState(),
content: @Composable () -> Unit = {},
) {
val markerState = rememberMarkerState(position = singapore)
val markerState = rememberMarkerState(initialPosition = singapore)

val uiSettings by remember { mutableStateOf(MapUiSettings(compassEnabled = false)) }
val mapProperties by remember {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ class AdvancedMarkersActivity : ComponentActivity(), OnMapsSdkInitializedCallbac
val mapProperties by remember {
mutableStateOf(MapProperties(mapType = MapType.NORMAL))
}
val marker1State = rememberMarkerState(position = santiago)
val marker2State = rememberMarkerState(position = bogota)
val marker3State = rememberMarkerState(position = lima)
val marker4State = rememberMarkerState(position = salvador)
val marker1State = rememberMarkerState(initialPosition = santiago)
val marker2State = rememberMarkerState(initialPosition = bogota)
val marker3State = rememberMarkerState(initialPosition = lima)
val marker4State = rememberMarkerState(initialPosition = salvador)

// Drawing on the map is accomplished with a child-based API
val markerClick: (Marker) -> Boolean = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ fun GoogleMapClustering(items: List<MyItem>) {
}

MarkerInfoWindow(
state = rememberMarkerState(position = singapore),
state = rememberMarkerState(initialPosition = singapore),
onClick = {
Log.d(TAG, "Non-cluster marker clicked! $it")
true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private fun DraggableMarker(
onDrag: (LatLng) -> Unit = {},
onDragEnd: () -> Unit = {}
) {
val markerState = rememberMarkerState(position = singapore)
val markerState = rememberMarkerState(initialPosition = singapore)

Marker(
state = markerState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,6 @@ fun rememberUpdatedMarkerState(position: LatLng): MarkerState =
// rememberUpdatedState() uses MutableState, we use MarkerState.
// This is more efficient than updating position in an effect,
// as we avoid an additional recomposition.
remember { MarkerState(position = position) }.also {
remember { MarkerState(initialPosition = position) }.also {
it.position = position
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ public class MarkerState private constructor(position: LatLng) {
/**
* Creates a new [MarkerState] object
*
* @param position the initial marker position
* @param initialPosition the initial marker position
*/
@StateFactoryMarker
public operator fun invoke(
position: LatLng = LatLng(0.0, 0.0)
): MarkerState = MarkerState(position)
initialPosition: LatLng = LatLng(0.0, 0.0)
Copy link
Contributor

@bubenheimer bubenheimer Dec 3, 2024

Choose a reason for hiding this comment

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

I don't see the rationale for changing the parameter name here. Makes sense for rememberMarkerState, but not here. If you end up recomposing MarkerState.invoke(position) you will get a new MarkerState object.

It may look tempting because of the "initial marker position" KDoc verbiage, but that's how State objects generally work, including MutableState. It's mutableStateOf(value="bla").

): MarkerState = MarkerState(initialPosition)

/**
* The default saver implementation for [MarkerState]
Expand All @@ -182,13 +182,15 @@ public class MarkerState private constructor(position: LatLng) {
* Other use cases may be better served syncing [MarkerState.position] with a data model.
*
* This cannot be used to preserve info window visibility across configuration changes.
*
* @param initialPosition the initial marker position
*/
@Composable
public fun rememberMarkerState(
key: String? = null,
position: LatLng = LatLng(0.0, 0.0)
initialPosition: LatLng = LatLng(0.0, 0.0)
): MarkerState = rememberSaveable(key = key, saver = MarkerState.Saver) {
MarkerState(position)
MarkerState(initialPosition)
}

/**
Expand Down