From 1288a5aae28f3c89d5762de2ab1b9c1ae88eef8b Mon Sep 17 00:00:00 2001 From: Vitaly Zayankovsky Date: Fri, 13 Jan 2023 19:55:55 +0300 Subject: [PATCH 1/3] restore overview camera when routes change during route preview state --- changelog/unreleased/bugfixes/changes.md | 1 + .../AudioGuidanceStateController.kt | 2 - .../controller/CameraStateController.kt | 16 +- .../controller/DestinationStateController.kt | 2 - .../controller/LocationStateController.kt | 2 - .../controller/NavigationStateController.kt | 2 - .../controller/RouteStateController.kt | 2 - .../internal/controller/StateController.kt | 2 - .../controller/CameraStateControllerTest.kt | 199 ++++++++++++------ 9 files changed, 145 insertions(+), 83 deletions(-) create mode 100644 changelog/unreleased/bugfixes/changes.md diff --git a/changelog/unreleased/bugfixes/changes.md b/changelog/unreleased/bugfixes/changes.md new file mode 100644 index 00000000000..b0b38d47a7e --- /dev/null +++ b/changelog/unreleased/bugfixes/changes.md @@ -0,0 +1 @@ +- Improved `NavigationView` camera behavior to go back into overview state if routes change during route preview state. diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/AudioGuidanceStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/AudioGuidanceStateController.kt index c5e88b3c1b9..603f07b3faa 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/AudioGuidanceStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/AudioGuidanceStateController.kt @@ -1,6 +1,5 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.ui.app.internal.Action import com.mapbox.navigation.ui.app.internal.State @@ -13,7 +12,6 @@ import com.mapbox.navigation.ui.voice.api.MapboxAudioGuidance * This class is responsible for playing voice instructions. Use the [AudioAction] to turning the * audio on or off. */ -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class AudioGuidanceStateController( private val store: Store ) : StateController() { diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateController.kt index 7434b5f179f..7d73d0bc517 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateController.kt @@ -1,6 +1,5 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.ui.app.internal.Action import com.mapbox.navigation.ui.app.internal.State @@ -9,8 +8,9 @@ import com.mapbox.navigation.ui.app.internal.camera.CameraAction import com.mapbox.navigation.ui.app.internal.camera.CameraState import com.mapbox.navigation.ui.app.internal.camera.TargetCameraMode import com.mapbox.navigation.ui.app.internal.navigation.NavigationState +import com.mapbox.navigation.ui.app.internal.routefetch.RoutePreviewState +import kotlinx.coroutines.flow.distinctUntilChanged -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class CameraStateController( private val store: Store, ) : StateController() { @@ -21,8 +21,16 @@ class CameraStateController( override fun onAttached(mapboxNavigation: MapboxNavigation) { super.onAttached(mapboxNavigation) - store.select { it.navigation }.observe { navigationState -> - when (navigationState) { + store.state.distinctUntilChanged { old, new -> + if (new.navigation is NavigationState.RoutePreview) { + new.previewRoutes !is RoutePreviewState.Ready || + old.navigation is NavigationState.RoutePreview && + new.previewRoutes == old.previewRoutes + } else { + new.navigation == old.navigation + } + }.observe { state -> + when (state.navigation) { NavigationState.FreeDrive, NavigationState.RoutePreview -> { store.dispatch(CameraAction.SetCameraMode(TargetCameraMode.Overview)) } diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/DestinationStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/DestinationStateController.kt index 3e8bd5584cb..9ef8cb22320 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/DestinationStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/DestinationStateController.kt @@ -1,12 +1,10 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.ui.app.internal.Action import com.mapbox.navigation.ui.app.internal.State import com.mapbox.navigation.ui.app.internal.Store import com.mapbox.navigation.ui.app.internal.destination.DestinationAction -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class DestinationStateController( store: Store ) : StateController() { diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/LocationStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/LocationStateController.kt index 1aff46b147e..5f81c54b69c 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/LocationStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/LocationStateController.kt @@ -1,6 +1,5 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.internal.extensions.flowLocationMatcherResult import com.mapbox.navigation.core.trip.session.LocationMatcherResult @@ -9,7 +8,6 @@ import com.mapbox.navigation.ui.app.internal.State import com.mapbox.navigation.ui.app.internal.Store import com.mapbox.navigation.ui.app.internal.location.LocationAction -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class LocationStateController( private val store: Store ) : StateController() { diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/NavigationStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/NavigationStateController.kt index 86367436e43..351f75c72b1 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/NavigationStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/NavigationStateController.kt @@ -1,6 +1,5 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.internal.extensions.flowOnFinalDestinationArrival import com.mapbox.navigation.ui.app.internal.Action @@ -16,7 +15,6 @@ import kotlinx.coroutines.launch * [NavigationStateAction] received. * @param store the default [NavigationState] */ -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class NavigationStateController( private val store: Store ) : StateController() { diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/RouteStateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/RouteStateController.kt index 737df5c06b0..cc23b00010e 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/RouteStateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/RouteStateController.kt @@ -1,6 +1,5 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.route.NavigationRoute import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.internal.extensions.flowRoutesUpdated @@ -9,7 +8,6 @@ import com.mapbox.navigation.ui.app.internal.State import com.mapbox.navigation.ui.app.internal.Store import com.mapbox.navigation.ui.app.internal.routefetch.RoutesAction -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class RouteStateController(private val store: Store) : StateController() { init { store.register(this) diff --git a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/StateController.kt b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/StateController.kt index 626c0e09b1a..8e6942402e4 100644 --- a/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/StateController.kt +++ b/libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/StateController.kt @@ -1,8 +1,6 @@ package com.mapbox.navigation.ui.app.internal.controller -import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.ui.app.internal.Reducer import com.mapbox.navigation.ui.base.lifecycle.UIComponent -@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) abstract class StateController : UIComponent(), Reducer diff --git a/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt b/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt index 39921833d53..78745858dc5 100644 --- a/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt +++ b/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt @@ -2,6 +2,7 @@ package com.mapbox.navigation.ui.app.internal.controller import com.mapbox.geojson.Point import com.mapbox.maps.EdgeInsets +import com.mapbox.navigation.base.route.NavigationRoute import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp import com.mapbox.navigation.testing.MainCoroutineRule @@ -9,6 +10,7 @@ import com.mapbox.navigation.ui.app.internal.camera.CameraAction import com.mapbox.navigation.ui.app.internal.camera.CameraAction.SetCameraMode import com.mapbox.navigation.ui.app.internal.camera.TargetCameraMode import com.mapbox.navigation.ui.app.internal.navigation.NavigationState +import com.mapbox.navigation.ui.app.internal.routefetch.RoutePreviewState import com.mapbox.navigation.ui.app.testing.TestStore import io.mockk.every import io.mockk.mockk @@ -40,7 +42,7 @@ class CameraStateControllerTest { } @Test - fun `when action toIdle updates camera mode`() = coroutineRule.runBlockingTest { + fun `when action toIdle updates camera mode`() { val sut = CameraStateController(testStore) sut.onAttached(mockMapboxNavigation()) @@ -51,21 +53,20 @@ class CameraStateControllerTest { } @Test - fun `when action toIdle should copy currentCamera mode value to savedCameraMode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `when action toIdle should copy currentCamera mode value to savedCameraMode`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - val initialCameraMode = TargetCameraMode.Following - testStore.dispatch(SetCameraMode(initialCameraMode)) - testStore.dispatch(SetCameraMode(TargetCameraMode.Idle)) + val initialCameraMode = TargetCameraMode.Following + testStore.dispatch(SetCameraMode(initialCameraMode)) + testStore.dispatch(SetCameraMode(TargetCameraMode.Idle)) - val cameraState = testStore.state.value.camera - assertEquals(initialCameraMode, cameraState.savedCameraMode) - } + val cameraState = testStore.state.value.camera + assertEquals(initialCameraMode, cameraState.savedCameraMode) + } @Test - fun `when action toOverview updates camera mode`() = coroutineRule.runBlockingTest { + fun `when action toOverview updates camera mode`() { val sut = CameraStateController(testStore) sut.onAttached(mockMapboxNavigation()) @@ -76,29 +77,27 @@ class CameraStateControllerTest { } @Test - fun `when action toFollowing updates camera mode and zoomUpdatesAllowed`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `when action toFollowing updates camera mode and zoomUpdatesAllowed`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) - val cameraState = testStore.state.value.camera - assertEquals(TargetCameraMode.Following, cameraState.cameraMode) - } + val cameraState = testStore.state.value.camera + assertEquals(TargetCameraMode.Following, cameraState.cameraMode) + } @Test - fun `when action UpdatePadding updates cameraPadding`() = - coroutineRule.runBlockingTest { - val padding = EdgeInsets(1.0, 2.0, 3.0, 4.0) - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `when action UpdatePadding updates cameraPadding`() { + val padding = EdgeInsets(1.0, 2.0, 3.0, 4.0) + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - testStore.dispatch(CameraAction.UpdatePadding(padding)) + testStore.dispatch(CameraAction.UpdatePadding(padding)) - val cameraState = testStore.state.value.camera - assertEquals(padding, cameraState.cameraPadding) - } + val cameraState = testStore.state.value.camera + assertEquals(padding, cameraState.cameraPadding) + } @Test fun `on SaveMapState action should save map camera state in the store`() { @@ -119,64 +118,130 @@ class CameraStateControllerTest { } @Test - fun `camera is set to overview in route preview mode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `camera is unchanged in route preview mode without preview routes`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) + + testStore.updateState { it.copy(navigation = NavigationState.DestinationPreview) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, previewRoutes = RoutePreviewState.Empty, + ) + } - val state = testStore.state.value.copy(navigation = NavigationState.RoutePreview) - testStore.setState(state) + assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) + } - assertEquals(TargetCameraMode.Overview, testStore.state.value.camera.cameraMode) + @Test + fun `camera is set to overview in route preview mode with preview routes`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) + + testStore.updateState { it.copy(navigation = NavigationState.DestinationPreview) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Ready(listOf(mockk())), + ) } + assertEquals(TargetCameraMode.Overview, testStore.state.value.camera.cameraMode) + } + @Test - fun `camera is set to overview in free drive mode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `camera is restored to overview in route preview mode with new preview routes`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) + + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Ready(listOf(mockk())), + ) + } + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Ready(listOf(mockk())), + ) + } + + assertEquals(TargetCameraMode.Overview, testStore.state.value.camera.cameraMode) + } - val state = testStore.state.value.copy(navigation = NavigationState.FreeDrive) - testStore.setState(state) + @Test + fun `camera is unchanged in route preview mode with the same preview routes`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) + val route = mockk() - assertEquals(TargetCameraMode.Overview, testStore.state.value.camera.cameraMode) + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Ready(listOf(route)), + ) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.updateState { state -> + state.copy( + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Ready(listOf(route)), + ) + } + + assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) + } @Test - fun `camera is set to following in active navigation mode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `camera is set to overview in free drive mode`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - val state = testStore.state.value.copy(navigation = NavigationState.ActiveNavigation) - testStore.setState(state) + testStore.updateState { it.copy(navigation = NavigationState.DestinationPreview) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) + testStore.updateState { it.copy(navigation = NavigationState.FreeDrive) } - assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) - } + assertEquals(TargetCameraMode.Overview, testStore.state.value.camera.cameraMode) + } @Test - fun `camera is set to following in arrival mode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `camera is set to following in active navigation mode`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - val state = testStore.state.value.copy(navigation = NavigationState.Arrival) - testStore.setState(state) + testStore.updateState { it.copy(navigation = NavigationState.RoutePreview) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Overview)) + testStore.updateState { it.copy(navigation = NavigationState.ActiveNavigation) } - assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) - } + assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) + } @Test - fun `camera is set to idle in destination preview mode`() = - coroutineRule.runBlockingTest { - val sut = CameraStateController(testStore) - sut.onAttached(mockMapboxNavigation()) + fun `camera is set to following in arrival mode`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) - val state = testStore.state.value.copy(navigation = NavigationState.DestinationPreview) - testStore.setState(state) + testStore.updateState { it.copy(navigation = NavigationState.ActiveNavigation) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Overview)) + testStore.updateState { it.copy(navigation = NavigationState.Arrival) } - assertEquals(TargetCameraMode.Idle, testStore.state.value.camera.cameraMode) - } + assertEquals(TargetCameraMode.Following, testStore.state.value.camera.cameraMode) + } + + @Test + fun `camera is set to idle in destination preview mode`() { + val sut = CameraStateController(testStore) + sut.onAttached(mockMapboxNavigation()) + + testStore.updateState { it.copy(navigation = NavigationState.FreeDrive) } + testStore.dispatch(SetCameraMode(TargetCameraMode.Overview)) + testStore.updateState { it.copy(navigation = NavigationState.DestinationPreview) } + + assertEquals(TargetCameraMode.Idle, testStore.state.value.camera.cameraMode) + } private fun mockMapboxNavigation(): MapboxNavigation { val mapboxNavigation = mockk(relaxed = true) From d0646af5a4c1fcf142249f5c2097ee8319bea75c Mon Sep 17 00:00:00 2001 From: runner Date: Fri, 13 Jan 2023 18:39:16 +0000 Subject: [PATCH 2/3] Rename changelog files --- changelog/unreleased/bugfixes/{changes.md => 6840.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/unreleased/bugfixes/{changes.md => 6840.md} (100%) diff --git a/changelog/unreleased/bugfixes/changes.md b/changelog/unreleased/bugfixes/6840.md similarity index 100% rename from changelog/unreleased/bugfixes/changes.md rename to changelog/unreleased/bugfixes/6840.md From 4c24bfebade50e5bf0b825e01a47014f2280208c Mon Sep 17 00:00:00 2001 From: Vitaly Zayankovsky Date: Mon, 16 Jan 2023 11:31:30 +0300 Subject: [PATCH 3/3] ktlint --- .../ui/app/internal/controller/CameraStateControllerTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt b/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt index 78745858dc5..9a6856b849b 100644 --- a/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt +++ b/libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/CameraStateControllerTest.kt @@ -126,7 +126,8 @@ class CameraStateControllerTest { testStore.dispatch(SetCameraMode(TargetCameraMode.Following)) testStore.updateState { state -> state.copy( - navigation = NavigationState.RoutePreview, previewRoutes = RoutePreviewState.Empty, + navigation = NavigationState.RoutePreview, + previewRoutes = RoutePreviewState.Empty, ) }