Skip to content

Commit 9905db4

Browse files
author
Łukasz Paczos
committed
review updates
1 parent d6b3815 commit 9905db4

6 files changed

Lines changed: 110 additions & 190 deletions

File tree

‎changelog/unreleased/bugfixes/6793.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
- Updated behavior of `MapboxRouteLineView::initializeLayers` to always reset the route line related layers with the options provided even if it means removing existing layers in order to apply the options. Also new instances of `MapboxRouteLineView` will result in an initialization of the layers upon the first render call.
1+
- :warning: If you're recreating the `MapboxRouteLineView` instance, for example to change the `MapboxRouteLineOptions`, make sure that your first interaction restores the state and re-applies the options by calling `MapboxRouteLineApi.getRouteDrawData` and passing the result to `MapboxRouteLineView.renderRouteDrawData`. This is a necessary change to fix an issue where `MapboxRouteLineOptions` provided in a new instance of `MapboxRouteLineView` were ignored if the style object used with `render` functions already had route line layers initialized.

‎libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineView.kt‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ import org.jetbrains.annotations.TestOnly
7474
* Many of the method calls execute tasks on a background thread. A cancel method is provided
7575
* in this class which will cancel the background tasks.
7676
*
77+
* If you're recreating the [MapboxRouteLineView] instance, for example to change the
78+
* [MapboxRouteLineOptions], make sure that your first interaction restores the state and re-applies
79+
* the options by calling [MapboxRouteLineApi.getRouteDrawData] and passing the result to [MapboxRouteLineView.renderRouteDrawData].
80+
*
7781
* @param options resource options used rendering the route line on the map
7882
*/
7983
@UiThread
@@ -91,7 +95,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
9195
var options: MapboxRouteLineOptions = options
9296
@Deprecated(
9397
message = "Avoid using this setter as it will not correctly " +
94-
"re-apply all mutated parameters."
98+
"re-apply all mutated parameters. " +
99+
"Recreate the instance and provide options via constructor instead."
95100
)
96101
set
97102

@@ -145,32 +150,14 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
145150
* the layers if they have not yet been initialized. If you have a use case for initializing
146151
* the layers in advance of any API calls this method may be used.
147152
*
148-
* If the layers already exist they will be removed and re-initialized with the options provided.
149-
*
150153
* Each [Layer] added to the map by this class is a persistent layer - it will survive style changes.
151154
* This means that if the data has not changed, it does not have to be manually redrawn after a style change.
152155
* See [Style.addPersistentStyleLayer].
153156
*
154157
* @param style a valid [Style] instance
155158
*/
156159
fun initializeLayers(style: Style) {
157-
resetLayers(style)
158-
}
159-
160-
/**
161-
* Updates the [MapboxRouteLineOptions] used for the route line related layers and initializes the layers.
162-
* If the layers already exist they will be removed and re-initialized with the options provided.
163-
*
164-
* Each [Layer] added to the map by this class is a persistent layer - it will survive style changes.
165-
* This means that if the data has not changed, it does not have to be manually redrawn after a style change.
166-
* See [Style.addPersistentStyleLayer].
167-
*
168-
* @param style a valid [Style] instance
169-
* @param options used for the route line related layers.
170-
*/
171-
fun initializeLayers(style: Style, options: MapboxRouteLineOptions) {
172-
this.options = options
173-
resetLayers(style)
160+
rebuildSourcesAndLayersIfNeeded(style)
174161
}
175162

176163
/**
@@ -255,7 +242,9 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
255242
sourceLayerMap
256243
)
257244
}
258-
} else { {} }
245+
} else {
246+
{}
247+
}
259248
listOf(layerMoveCommand).plus(trimOffsetCommands).plus(gradientCommands)
260249
} ?: listOf()
261250
}
@@ -858,7 +847,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
858847
}
859848
else -> null
860849
}
861-
} else -> {
850+
}
851+
else -> {
862852
when (it) {
863853
in casingLayerIds -> {
864854
options.resourceProvider.alternativeRouteCasingLineScaleExpression

‎libnavui-maps/src/test/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineViewTest.kt‎

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -263,123 +263,6 @@ class MapboxRouteLineViewTest {
263263
unmockkObject(MapboxRouteLineUtils)
264264
}
265265

266-
@Test
267-
fun initializeLayers_withOptions() {
268-
mockkObject(MapboxRouteLineUtils)
269-
val options = MapboxRouteLineOptions.Builder(ctx).build()
270-
val options2 = MapboxRouteLineOptions.Builder(ctx).withTolerance(59.0).build()
271-
val style = mockk<Style> {
272-
every {
273-
setStyleSourceProperty(LAYER_GROUP_1_SOURCE_ID, any(), any())
274-
} returns ExpectedFactory.createNone()
275-
every {
276-
setStyleSourceProperty(LAYER_GROUP_2_SOURCE_ID, any(), any())
277-
} returns ExpectedFactory.createNone()
278-
every {
279-
setStyleSourceProperty(LAYER_GROUP_3_SOURCE_ID, any(), any())
280-
} returns ExpectedFactory.createNone()
281-
every {
282-
setStyleSourceProperty(WAYPOINT_SOURCE_ID, any(), any())
283-
} returns ExpectedFactory.createNone()
284-
every { getSource(LAYER_GROUP_1_SOURCE_ID) } returns null
285-
every { getSource(LAYER_GROUP_2_SOURCE_ID) } returns null
286-
every { getSource(LAYER_GROUP_3_SOURCE_ID) } returns null
287-
every { getSource(WAYPOINT_SOURCE_ID) } returns null
288-
every {
289-
removeStyleSource(LAYER_GROUP_1_SOURCE_ID)
290-
} returns ExpectedFactory.createNone()
291-
every {
292-
removeStyleSource(LAYER_GROUP_2_SOURCE_ID)
293-
} returns ExpectedFactory.createNone()
294-
every {
295-
removeStyleSource(LAYER_GROUP_3_SOURCE_ID)
296-
} returns ExpectedFactory.createNone()
297-
every {
298-
removeStyleSource(WAYPOINT_SOURCE_ID)
299-
} returns ExpectedFactory.createNone()
300-
every {
301-
removeStyleSource(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
302-
} returns ExpectedFactory.createNone()
303-
every {
304-
removeStyleSource(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
305-
} returns ExpectedFactory.createNone()
306-
every {
307-
removeStyleLayer(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
308-
} returns ExpectedFactory.createNone()
309-
every {
310-
removeStyleLayer(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
311-
} returns ExpectedFactory.createNone()
312-
every {
313-
removeStyleLayer(LAYER_GROUP_1_TRAIL_CASING)
314-
} returns ExpectedFactory.createNone()
315-
every {
316-
removeStyleLayer(LAYER_GROUP_1_TRAIL)
317-
} returns ExpectedFactory.createNone()
318-
every {
319-
removeStyleLayer(LAYER_GROUP_1_CASING)
320-
} returns ExpectedFactory.createNone()
321-
every {
322-
removeStyleLayer(LAYER_GROUP_1_MAIN)
323-
} returns ExpectedFactory.createNone()
324-
every {
325-
removeStyleLayer(LAYER_GROUP_1_TRAFFIC)
326-
} returns ExpectedFactory.createNone()
327-
every {
328-
removeStyleLayer(LAYER_GROUP_1_RESTRICTED)
329-
} returns ExpectedFactory.createNone()
330-
every {
331-
removeStyleLayer(LAYER_GROUP_2_TRAIL_CASING)
332-
} returns ExpectedFactory.createNone()
333-
every {
334-
removeStyleLayer(LAYER_GROUP_2_TRAIL)
335-
} returns ExpectedFactory.createNone()
336-
every {
337-
removeStyleLayer(LAYER_GROUP_2_CASING)
338-
} returns ExpectedFactory.createNone()
339-
every {
340-
removeStyleLayer(LAYER_GROUP_2_MAIN)
341-
} returns ExpectedFactory.createNone()
342-
every {
343-
removeStyleLayer(LAYER_GROUP_2_TRAFFIC)
344-
} returns ExpectedFactory.createNone()
345-
every {
346-
removeStyleLayer(LAYER_GROUP_2_RESTRICTED)
347-
} returns ExpectedFactory.createNone()
348-
every {
349-
removeStyleLayer(LAYER_GROUP_3_TRAIL_CASING)
350-
} returns ExpectedFactory.createNone()
351-
every {
352-
removeStyleLayer(LAYER_GROUP_3_TRAIL)
353-
} returns ExpectedFactory.createNone()
354-
every {
355-
removeStyleLayer(LAYER_GROUP_3_CASING)
356-
} returns ExpectedFactory.createNone()
357-
every {
358-
removeStyleLayer(LAYER_GROUP_3_MAIN)
359-
} returns ExpectedFactory.createNone()
360-
every {
361-
removeStyleLayer(LAYER_GROUP_3_TRAFFIC)
362-
} returns ExpectedFactory.createNone()
363-
every {
364-
removeStyleLayer(LAYER_GROUP_3_RESTRICTED)
365-
} returns ExpectedFactory.createNone()
366-
}.also {
367-
mockCheckForLayerInitialization(it)
368-
}
369-
val view = MapboxRouteLineView(options)
370-
371-
view.initializeLayers(style, options2)
372-
373-
verify { MapboxRouteLineUtils.removeLayersAndSources(style) }
374-
verify { MapboxRouteLineUtils.initializeLayers(style, options2) }
375-
verify { style.getSource(LAYER_GROUP_1_SOURCE_ID) }
376-
verify { style.getSource(LAYER_GROUP_2_SOURCE_ID) }
377-
verify { style.getSource(LAYER_GROUP_3_SOURCE_ID) }
378-
verify { style.getSource(WAYPOINT_SOURCE_ID) }
379-
assertEquals(view.options, options2)
380-
unmockkObject(MapboxRouteLineUtils)
381-
}
382-
383266
@Test
384267
fun renderClearRouteLineValue() = coroutineRule.runBlockingTest {
385268
mockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")

‎qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/CustomAlternativeRouteColoringActivity.kt‎

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import android.annotation.SuppressLint
44
import android.graphics.Color
55
import android.os.Bundle
66
import android.util.Log
7+
import androidx.annotation.ColorInt
78
import androidx.appcompat.app.AppCompatActivity
89
import com.mapbox.api.directions.v5.models.DirectionsRoute
910
import com.mapbox.maps.CameraOptions
@@ -33,64 +34,42 @@ class CustomAlternativeRouteColoringActivity : AppCompatActivity() {
3334
viewBinding.mapView.getMapboxMap()
3435
}
3536

36-
private val routeLineColorResources by lazy {
37-
RouteLineColorResources.Builder().build()
38-
}
37+
private val styles = listOf(
38+
NavigationStyles.NAVIGATION_DAY_STYLE,
39+
NavigationStyles.NAVIGATION_NIGHT_STYLE,
40+
Style.LIGHT,
41+
)
3942

40-
private val routeLineResources: RouteLineResources by lazy {
41-
RouteLineResources.Builder().routeLineColorResources(routeLineColorResources).build()
42-
}
43+
private val colors = listOf(
44+
RouteLineColorResources.Builder().build().routeDefaultColor,
45+
Color.GREEN,
46+
)
4347

44-
private val routeLineOptions: MapboxRouteLineOptions by lazy {
45-
MapboxRouteLineOptions.Builder(this)
46-
.withRouteLineResources(routeLineResources)
47-
.withRouteLineBelowLayerId("road-label-navigation")
48-
.withVanishingRouteLineEnabled(true)
49-
.withRouteStyleDescriptors(
50-
listOf(
51-
RouteStyleDescriptor("altRoute1", Color.YELLOW, Color.GREEN),
52-
RouteStyleDescriptor("altRoute2", Color.CYAN, Color.MAGENTA)
53-
)
54-
)
55-
.build()
56-
}
57-
58-
private val routeLineView by lazy {
59-
MapboxRouteLineView(routeLineOptions)
60-
}
61-
62-
private val routeLineApi: MapboxRouteLineApi by lazy {
63-
MapboxRouteLineApi(routeLineOptions)
64-
}
48+
private var routeColor = colors.first()
49+
private var styleId = styles.first()
6550

6651
override fun onCreate(savedInstanceState: Bundle?) {
6752
super.onCreate(savedInstanceState)
6853
setContentView(viewBinding.root)
69-
initStyle()
54+
initStyle(styleId)
55+
viewBinding.buttonChangeStyle.setOnClickListener {
56+
styleId = styles.filter { it != styleId }.random()
57+
initStyle(styleId)
58+
}
59+
viewBinding.buttonChangePrimaryRouteColor.setOnClickListener {
60+
routeColor = colors.filter { it != routeColor }.random()
61+
setRoutes(mapboxMap.getStyle()!!, generateRouteLineOptions(styleId, routeColor))
62+
}
7063
}
7164

7265
@SuppressLint("MissingPermission")
73-
private fun initStyle() {
66+
private fun initStyle(
67+
styleId: String,
68+
) {
7469
mapboxMap.loadStyleUri(
75-
NavigationStyles.NAVIGATION_DAY_STYLE,
70+
styleId,
7671
{ style: Style ->
77-
78-
val route1 = getRoute(R.raw.basic_route1)
79-
val route2 = getRoute(R.raw.basic_route2)
80-
val route3 = getRoute(R.raw.basic_route3)
81-
routeLineApi.setRoutes(
82-
listOf(
83-
RouteLine(route1, null),
84-
RouteLine(route2, "altRoute1"),
85-
RouteLine(route3, "altRoute2")
86-
)
87-
) {
88-
routeLineView.renderRouteDrawData(style, it)
89-
routeLineView.hideTraffic(style)
90-
}
91-
val routeOrigin = Utils.getRouteOriginPoint(route1)
92-
val cameraOptions = CameraOptions.Builder().center(routeOrigin).zoom(14.0).build()
93-
viewBinding.mapView.getMapboxMap().setCamera(cameraOptions)
72+
setRoutes(style, generateRouteLineOptions(styleId, routeColor))
9473
},
9574
object : OnMapLoadErrorListener {
9675
override fun onMapLoadError(eventData: MapLoadingErrorEventData) {
@@ -104,6 +83,53 @@ class CustomAlternativeRouteColoringActivity : AppCompatActivity() {
10483
)
10584
}
10685

86+
private fun setRoutes(style: Style, options: MapboxRouteLineOptions) {
87+
val routeLineApi = MapboxRouteLineApi(options)
88+
val routeLineView = MapboxRouteLineView(options)
89+
val route1 = getRoute(R.raw.basic_route1)
90+
val route2 = getRoute(R.raw.basic_route2)
91+
val route3 = getRoute(R.raw.basic_route3)
92+
routeLineApi.setRoutes(
93+
listOf(
94+
RouteLine(route1, null),
95+
RouteLine(route2, "altRoute1"),
96+
RouteLine(route3, "altRoute2")
97+
)
98+
) {
99+
routeLineView.renderRouteDrawData(style, it)
100+
routeLineView.hideTraffic(style)
101+
}
102+
val routeOrigin = Utils.getRouteOriginPoint(route1)
103+
val cameraOptions = CameraOptions.Builder().center(routeOrigin).zoom(14.0).build()
104+
viewBinding.mapView.getMapboxMap().setCamera(cameraOptions)
105+
}
106+
107+
private fun generateRouteLineOptions(styleId: String, @ColorInt routeColor: Int) =
108+
MapboxRouteLineOptions.Builder(this)
109+
.withRouteLineResources(
110+
RouteLineResources.Builder()
111+
.routeLineColorResources(
112+
RouteLineColorResources.Builder()
113+
.routeDefaultColor(routeColor)
114+
.build()
115+
)
116+
.build()
117+
)
118+
.withRouteLineBelowLayerId(
119+
when (styleId) {
120+
NavigationStyles.NAVIGATION_DAY_STYLE -> "road-label-navigation"
121+
Style.LIGHT -> "road-label"
122+
else -> "road-label-navigation"
123+
}
124+
)
125+
.withRouteStyleDescriptors(
126+
listOf(
127+
RouteStyleDescriptor("altRoute1", Color.YELLOW, Color.GREEN),
128+
RouteStyleDescriptor("altRoute2", Color.CYAN, Color.MAGENTA)
129+
)
130+
)
131+
.build()
132+
107133
private fun getRoute(routeResourceId: Int): DirectionsRoute {
108134
val routeAsString = Utils.readRawFileText(this, routeResourceId)
109135
return DirectionsRoute.fromJson(routeAsString)

‎qa-test-app/src/main/res/layout/custom_alternative_route_coloring_activity_layout.xml‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
3+
xmlns:app="http://schemas.android.com/apk/res-auto"
34
android:layout_width="match_parent"
4-
android:layout_height="match_parent"
5-
xmlns:app="http://schemas.android.com/apk/res-auto">
5+
android:layout_height="match_parent">
66

77
<com.mapbox.maps.MapView
88
android:id="@+id/mapView"
@@ -13,4 +13,25 @@
1313
app:layout_constraintStart_toStartOf="parent"
1414
app:layout_constraintTop_toTopOf="parent" />
1515

16+
<androidx.appcompat.widget.AppCompatImageButton
17+
android:id="@+id/button_change_style"
18+
android:layout_width="48dp"
19+
android:layout_height="48dp"
20+
android:layout_margin="16dp"
21+
android:layout_marginTop="8dp"
22+
android:src="@android:drawable/ic_menu_gallery"
23+
app:layout_constraintBottom_toBottomOf="parent"
24+
app:layout_constraintEnd_toEndOf="parent" />
25+
26+
<androidx.appcompat.widget.AppCompatImageButton
27+
android:id="@+id/button_change_primary_route_color"
28+
android:layout_width="48dp"
29+
android:layout_height="48dp"
30+
android:layout_margin="16dp"
31+
android:layout_marginBottom="8dp"
32+
android:src="@android:drawable/stat_sys_upload_done"
33+
app:layout_constraintBottom_toBottomOf="parent"
34+
app:layout_constraintBottom_toTopOf="@id/button_change_style"
35+
app:layout_constraintEnd_toEndOf="parent" />
36+
1637
</androidx.constraintlayout.widget.ConstraintLayout>

0 commit comments

Comments
 (0)