Skip to content

Commit 2f09c6d

Browse files
arriolacarriolac
authored
fix: Ensure CameraPositionState map reference clears when GoogleMap l… (#109)
* fix: Ensure CameraPositionState map reference clears when GoogleMap leaves composition. Change-Id: Iccf615e1a468ea7fbddb652052cec51b5d98a763 * Adding test for GoogleMap coming in and out of the Composition. Change-Id: I4e4bcdd39561ca17515e4c4891742c50bd14e409 * PR feedback. Change-Id: I4e7ce6dc006cee72f49e29b7eb15303ae71e4654
1 parent 3ce97d8 commit 2f09c6d

File tree

5 files changed

+135
-115
lines changed

5 files changed

+135
-115
lines changed

‎app/src/androidTest/java/com/google/maps/android/compose/GoogleMapViewTests.kt‎

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
package com.google.maps.android.compose
1616

1717
import androidx.compose.foundation.layout.fillMaxSize
18+
import androidx.compose.runtime.Composable
1819
import androidx.compose.ui.Modifier
1920
import androidx.compose.ui.test.assertIsDisplayed
2021
import androidx.compose.ui.test.junit4.createAndroidComposeRule
2122
import androidx.compose.ui.test.onNodeWithTag
2223
import androidx.compose.ui.test.onNodeWithText
2324
import androidx.compose.ui.test.performClick
24-
import androidx.test.annotation.UiThreadTest
2525
import com.google.android.gms.maps.model.CameraPosition
2626
import com.google.android.gms.maps.model.LatLng
2727
import org.junit.Assert.assertEquals
@@ -44,15 +44,7 @@ class GoogleMapViewTests {
4444

4545
private lateinit var cameraPositionState: CameraPositionState
4646

47-
@Before
48-
fun setUp() {
49-
cameraPositionState = CameraPositionState(
50-
position = CameraPosition.fromLatLngZoom(
51-
startingPosition,
52-
startingZoom
53-
)
54-
)
55-
47+
private fun initMap(content: @Composable () -> Unit = {}) {
5648
val countDownLatch = CountDownLatch(1)
5749
composeTestRule.setContent {
5850
GoogleMapView(
@@ -61,19 +53,33 @@ class GoogleMapViewTests {
6153
onMapLoaded = {
6254
countDownLatch.countDown()
6355
}
64-
)
56+
) {
57+
content.invoke()
58+
}
6559
}
6660
val mapLoaded = countDownLatch.await(30, TimeUnit.SECONDS)
6761
assertTrue("Map loaded", mapLoaded)
6862
}
6963

64+
@Before
65+
fun setUp() {
66+
cameraPositionState = CameraPositionState(
67+
position = CameraPosition.fromLatLngZoom(
68+
startingPosition,
69+
startingZoom
70+
)
71+
)
72+
}
73+
7074
@Test
7175
fun testStartingCameraPosition() {
76+
initMap()
7277
startingPosition.assertEquals(cameraPositionState.position.target)
7378
}
7479

7580
@Test
7681
fun testCameraReportsMoving() {
82+
initMap()
7783
zoom(shouldAnimate = true, zoomIn = true) {
7884
composeTestRule.waitUntil(1000) {
7985
cameraPositionState.isMoving
@@ -84,6 +90,7 @@ class GoogleMapViewTests {
8490

8591
@Test
8692
fun testCameraReportsNotMoving() {
93+
initMap()
8794
zoom(shouldAnimate = true, zoomIn = true) {
8895
composeTestRule.waitUntil(1000) {
8996
cameraPositionState.isMoving
@@ -97,6 +104,7 @@ class GoogleMapViewTests {
97104

98105
@Test
99106
fun testCameraZoomInAnimation() {
107+
initMap()
100108
zoom(shouldAnimate = true, zoomIn = true) {
101109
composeTestRule.waitUntil(1000) {
102110
cameraPositionState.isMoving
@@ -114,6 +122,7 @@ class GoogleMapViewTests {
114122

115123
@Test
116124
fun testCameraZoomIn() {
125+
initMap()
117126
zoom(shouldAnimate = false, zoomIn = true) {
118127
composeTestRule.waitUntil(1000) {
119128
cameraPositionState.isMoving
@@ -131,6 +140,7 @@ class GoogleMapViewTests {
131140

132141
@Test
133142
fun testCameraZoomOut() {
143+
initMap()
134144
zoom(shouldAnimate = false, zoomIn = false) {
135145
composeTestRule.waitUntil(1000) {
136146
cameraPositionState.isMoving
@@ -148,6 +158,7 @@ class GoogleMapViewTests {
148158

149159
@Test
150160
fun testCameraZoomOutAnimation() {
161+
initMap()
151162
zoom(shouldAnimate = true, zoomIn = false) {
152163
composeTestRule.waitUntil(1000) {
153164
cameraPositionState.isMoving
@@ -164,48 +175,49 @@ class GoogleMapViewTests {
164175
}
165176

166177
@Test
167-
@UiThreadTest
168178
fun testLatLngInVisibleRegion() {
169-
val projection = cameraPositionState.projection
170-
assertNotNull(projection)
171-
assertTrue(
172-
projection!!.visibleRegion.latLngBounds.contains(startingPosition)
173-
)
179+
initMap()
180+
composeTestRule.runOnUiThread {
181+
val projection = cameraPositionState.projection
182+
assertNotNull(projection)
183+
assertTrue(
184+
projection!!.visibleRegion.latLngBounds.contains(startingPosition)
185+
)
186+
}
174187
}
175188

176189
@Test
177-
@UiThreadTest
178190
fun testLatLngNotInVisibleRegion() {
179-
val projection = cameraPositionState.projection
180-
assertNotNull(projection)
181-
val latLng = LatLng(23.4, 25.6)
182-
assertFalse(
183-
projection!!.visibleRegion.latLngBounds.contains(latLng)
184-
)
191+
initMap()
192+
composeTestRule.runOnUiThread {
193+
val projection = cameraPositionState.projection
194+
assertNotNull(projection)
195+
val latLng = LatLng(23.4, 25.6)
196+
assertFalse(
197+
projection!!.visibleRegion.latLngBounds.contains(latLng)
198+
)
199+
}
185200
}
186201

187202
@Test(expected = IllegalStateException::class)
188203
fun testMarkerStateCannotBeReused() {
189-
val countDownLatch = CountDownLatch(1)
190-
composeTestRule.setContent {
191-
GoogleMap(
192-
modifier = Modifier.fillMaxSize(),
193-
cameraPositionState = cameraPositionState,
194-
onMapLoaded = {
195-
countDownLatch.countDown()
196-
}
197-
) {
198-
val markerState = rememberMarkerState()
199-
Marker(
200-
state = markerState
201-
)
202-
Marker(
203-
state = markerState
204-
)
205-
}
204+
initMap {
205+
val markerState = rememberMarkerState()
206+
Marker(
207+
state = markerState
208+
)
209+
Marker(
210+
state = markerState
211+
)
206212
}
207-
val mapLoaded = countDownLatch.await(30, TimeUnit.SECONDS)
208-
assertTrue(mapLoaded)
213+
}
214+
215+
@Test
216+
fun testCameraPositionStateMapClears() {
217+
initMap()
218+
composeTestRule.onNodeWithTag("toggleMapVisibility")
219+
.performClick()
220+
.performClick()
209221
}
210222

211223
private fun zoom(

‎app/src/main/java/com/google/maps/android/compose/MapSampleActivity.kt‎

Lines changed: 68 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ fun GoogleMapView(
108108
modifier: Modifier,
109109
cameraPositionState: CameraPositionState,
110110
onMapLoaded: () -> Unit,
111+
content: @Composable () -> Unit = {}
111112
) {
112113
val singaporeState = rememberMarkerState(position = singapore)
113114
val singapore2State = rememberMarkerState(position = singapore2)
@@ -123,73 +124,78 @@ fun GoogleMapView(
123124
var mapProperties by remember {
124125
mutableStateOf(MapProperties(mapType = MapType.NORMAL))
125126
}
127+
var mapVisible by remember { mutableStateOf(true) }
126128

127-
GoogleMap(
128-
modifier = modifier,
129-
cameraPositionState = cameraPositionState,
130-
properties = mapProperties,
131-
uiSettings = uiSettings,
132-
onMapLoaded = onMapLoaded,
133-
onPOIClick = {
134-
Log.d(TAG, "POI clicked: ${it.name}")
135-
}
136-
) {
137-
// Drawing on the map is accomplished with a child-based API
138-
val markerClick: (Marker) -> Boolean = {
139-
Log.d(TAG, "${it.title} was clicked")
140-
cameraPositionState.projection?.let { projection ->
141-
Log.d(TAG, "The current projection is: $projection")
129+
if (mapVisible) {
130+
GoogleMap(
131+
modifier = modifier,
132+
cameraPositionState = cameraPositionState,
133+
properties = mapProperties,
134+
uiSettings = uiSettings,
135+
onMapLoaded = onMapLoaded,
136+
onPOIClick = {
137+
Log.d(TAG, "POI clicked: ${it.name}")
142138
}
143-
false
144-
}
145-
MarkerInfoWindowContent(
146-
state = singaporeState,
147-
title = "Zoom in has been tapped $ticker times.",
148-
onClick = markerClick,
149-
draggable = true,
150-
) {
151-
Text(it.title ?: "Title", color = Color.Red)
152-
}
153-
MarkerInfoWindowContent(
154-
state = singapore2State,
155-
title = "Marker with custom info window.\nZoom in has been tapped $ticker times.",
156-
icon = BitmapDescriptorFactory.defaultMarker(BitmapDescriptorFactory.HUE_BLUE),
157-
onClick = markerClick,
158139
) {
159-
Text(it.title ?: "Title", color = Color.Blue)
140+
// Drawing on the map is accomplished with a child-based API
141+
val markerClick: (Marker) -> Boolean = {
142+
Log.d(TAG, "${it.title} was clicked")
143+
cameraPositionState.projection?.let { projection ->
144+
Log.d(TAG, "The current projection is: $projection")
145+
}
146+
false
147+
}
148+
MarkerInfoWindowContent(
149+
state = singaporeState,
150+
title = "Zoom in has been tapped $ticker times.",
151+
onClick = markerClick,
152+
draggable = true,
153+
) {
154+
Text(it.title ?: "Title", color = Color.Red)
155+
}
156+
MarkerInfoWindowContent(
157+
state = singapore2State,
158+
title = "Marker with custom info window.\nZoom in has been tapped $ticker times.",
159+
icon = BitmapDescriptorFactory.defaultMarker(BitmapDescriptorFactory.HUE_BLUE),
160+
onClick = markerClick,
161+
) {
162+
Text(it.title ?: "Title", color = Color.Blue)
163+
}
164+
Marker(
165+
state = singapore3State,
166+
title = "Marker in Singapore",
167+
onClick = markerClick
168+
)
169+
Circle(
170+
center = circleCenter,
171+
fillColor = MaterialTheme.colors.secondary,
172+
strokeColor = MaterialTheme.colors.secondaryVariant,
173+
radius = 1000.0,
174+
)
175+
content()
160176
}
161-
Marker(
162-
state = singapore3State,
163-
title = "Marker in Singapore",
164-
onClick = markerClick
165-
)
166-
Circle(
167-
center = circleCenter,
168-
fillColor = MaterialTheme.colors.secondary,
169-
strokeColor = MaterialTheme.colors.secondaryVariant,
170-
radius = 1000.0,
171-
)
172-
}
173177

178+
}
174179
Column {
175180
MapTypeControls(onMapTypeClick = {
176181
Log.d("GoogleMap", "Selected map type $it")
177182
mapProperties = mapProperties.copy(mapType = it)
178183
})
179-
Button(
180-
modifier = Modifier.padding(4.dp),
181-
colors = ButtonDefaults.buttonColors(
182-
backgroundColor = MaterialTheme.colors.onPrimary,
183-
contentColor = MaterialTheme.colors.primary
184-
),
185-
onClick = {
186-
mapProperties = mapProperties.copy(mapType = MapType.NORMAL)
187-
cameraPositionState.position = defaultCameraPosition
188-
singaporeState.position = singapore
189-
singaporeState.hideInfoWindow()
190-
}
191-
) {
192-
Text(text = "RESET MAP", style = MaterialTheme.typography.body1)
184+
Row {
185+
MapButton(
186+
text = "Reset Map",
187+
onClick = {
188+
mapProperties = mapProperties.copy(mapType = MapType.NORMAL)
189+
cameraPositionState.position = defaultCameraPosition
190+
singaporeState.position = singapore
191+
singaporeState.hideInfoWindow()
192+
}
193+
)
194+
MapButton(
195+
text = "Toggle Map",
196+
onClick = { mapVisible = !mapVisible },
197+
modifier = Modifier.testTag("toggleMapVisibility"),
198+
)
193199
}
194200
val coroutineScope = rememberCoroutineScope()
195201
ZoomControls(
@@ -242,18 +248,8 @@ private fun MapTypeControls(
242248
}
243249

244250
@Composable
245-
private fun MapTypeButton(type: MapType, onClick: () -> Unit) {
246-
Button(
247-
modifier = Modifier.padding(4.dp),
248-
colors = ButtonDefaults.buttonColors(
249-
backgroundColor = MaterialTheme.colors.onPrimary,
250-
contentColor = MaterialTheme.colors.primary
251-
),
252-
onClick = onClick
253-
) {
254-
Text(text = type.toString(), style = MaterialTheme.typography.body1)
255-
}
256-
}
251+
private fun MapTypeButton(type: MapType, onClick: () -> Unit) =
252+
MapButton(text = type.toString(), onClick = onClick)
257253

258254
@Composable
259255
private fun ZoomControls(
@@ -284,16 +280,16 @@ private fun ZoomControls(
284280
}
285281

286282
@Composable
287-
private fun MapButton(text: String, onClick: () -> Unit) {
283+
private fun MapButton(text: String, onClick: () -> Unit, modifier: Modifier = Modifier) {
288284
Button(
289-
modifier = Modifier.padding(8.dp),
285+
modifier = modifier.padding(4.dp),
290286
colors = ButtonDefaults.buttonColors(
291287
backgroundColor = MaterialTheme.colors.onPrimary,
292288
contentColor = MaterialTheme.colors.primary
293289
),
294290
onClick = onClick
295291
) {
296-
Text(text = text, style = MaterialTheme.typography.h5)
292+
Text(text = text, style = MaterialTheme.typography.body1)
297293
}
298294
}
299295

‎maps-compose/src/main/java/com/google/maps/android/compose/MapApplier.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.google.android.gms.maps.model.Polyline
2626
internal interface MapNode {
2727
fun onAttached() {}
2828
fun onRemoved() {}
29+
fun onCleared() {}
2930
}
3031

3132
private object MapNodeRoot : MapNode
@@ -43,6 +44,8 @@ internal class MapApplier(
4344

4445
override fun onClear() {
4546
map.clear()
47+
decorations.forEach { it.onCleared() }
48+
decorations.clear()
4649
}
4750

4851
override fun insertBottomUp(index: Int, instance: MapNode) {

0 commit comments

Comments
 (0)