improve camera layout observer

Zayankovsky · Zayankovsky · commit 9274508ab6bf · 2023-01-20T12:32:23.000+03:00
diff --git a/libnavui-dropin/src/main/java/com/mapbox/navigation/dropin/camera/CameraLayoutObserver.kt b/libnavui-dropin/src/main/java/com/mapbox/navigation/dropin/camera/CameraLayoutObserver.kt
@@ -2,17 +2,17 @@ package com.mapbox.navigation.dropin.camera
 
 import android.content.res.Configuration
 import android.view.View
+import com.google.android.material.bottomsheet.BottomSheetBehavior
 import com.mapbox.maps.EdgeInsets
 import com.mapbox.navigation.core.MapboxNavigation
 import com.mapbox.navigation.dropin.R
 import com.mapbox.navigation.dropin.databinding.MapboxNavigationViewLayoutBinding
-import com.mapbox.navigation.ui.app.internal.Store
+import com.mapbox.navigation.dropin.navigationview.NavigationViewContext
 import com.mapbox.navigation.ui.app.internal.camera.CameraAction
-import com.mapbox.navigation.ui.app.internal.navigation.NavigationState
 import com.mapbox.navigation.ui.base.lifecycle.UIComponent
 
 internal class CameraLayoutObserver(
-    private val store: Store,
+    private val context: NavigationViewContext,
     private val mapView: View,
     private val binding: MapboxNavigationViewLayoutBinding,
 ) : UIComponent() {
@@ -21,63 +21,43 @@ internal class CameraLayoutObserver(
         .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_v).toDouble()
     private val hPadding = binding.root.resources
         .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_h).toDouble()
-    private val vPaddingLandscape = binding.root.resources
-        .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_landscape_v).toDouble()
-    private val hPaddingLandscape = binding.root.resources
-        .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_landscape_h).toDouble()
 
     private val layoutListener = View.OnLayoutChangeListener { _, _, _, _, _, _, _, _, _ ->
-        val edgeInsets = when (deviceOrientation()) {
-            Configuration.ORIENTATION_LANDSCAPE -> getLandscapePadding()
-            else -> getPortraitPadding()
-        }
-        store.dispatch(CameraAction.UpdatePadding(edgeInsets))
+        updateCameraPadding()
     }
 
     override fun onAttached(mapboxNavigation: MapboxNavigation) {
         super.onAttached(mapboxNavigation)
         binding.coordinatorLayout.addOnLayoutChangeListener(layoutListener)
+        context.behavior.infoPanelBehavior.bottomSheetState.observe { updateCameraPadding() }
     }
 
     override fun onDetached(mapboxNavigation: MapboxNavigation) {
         super.onDetached(mapboxNavigation)
         binding.coordinatorLayout.removeOnLayoutChangeListener(layoutListener)
     }
 
-    private fun getPortraitPadding(): EdgeInsets {
-        val top = binding.guidanceLayout.height.toDouble()
-        val bottom = mapView.height.toDouble() - binding.roadNameLayout.top.toDouble()
-        return when (store.state.value.navigation) {
-            is NavigationState.DestinationPreview,
-            is NavigationState.FreeDrive,
-            is NavigationState.RoutePreview -> {
-                EdgeInsets(vPadding, hPadding, bottom, hPadding)
-            }
-            is NavigationState.ActiveNavigation,
-            is NavigationState.Arrival -> {
-                EdgeInsets(vPadding + top, hPadding, bottom, hPadding)
-            }
+    private fun updateCameraPadding() {
+        val edgeInsets = when (deviceOrientation()) {
+            Configuration.ORIENTATION_LANDSCAPE -> getLandscapePadding()
+            else -> getPortraitPadding()
         }
+        context.store.dispatch(CameraAction.UpdatePadding(edgeInsets))
+    }
+
+    private fun getPortraitPadding(): EdgeInsets {
+        val top = binding.guidanceLayout.bottom
+        val bottom = mapView.height - binding.roadNameLayout.top
+        return EdgeInsets(vPadding + top, hPadding, vPadding + bottom, hPadding)
     }
 
     private fun getLandscapePadding(): EdgeInsets {
-        val bottom = mapView.height.toDouble() - binding.roadNameLayout.top.toDouble()
-        return when (store.state.value.navigation) {
-            is NavigationState.FreeDrive -> {
-                EdgeInsets(vPaddingLandscape, hPaddingLandscape, bottom, hPaddingLandscape)
-            }
-            is NavigationState.DestinationPreview,
-            is NavigationState.RoutePreview,
-            is NavigationState.ActiveNavigation,
-            is NavigationState.Arrival -> {
-                EdgeInsets(
-                    vPaddingLandscape,
-                    hPaddingLandscape + binding.infoPanelLayout.right.toDouble(),
-                    bottom,
-                    hPaddingLandscape
-                )
-            }
-        }
+        val bottomSheetState = context.behavior.infoPanelBehavior.bottomSheetState.value
+        val isBottomSheetVisible =
+            bottomSheetState != null && bottomSheetState != BottomSheetBehavior.STATE_HIDDEN
+        val bottomSheetWidth = if (isBottomSheetVisible) binding.infoPanelLayout.right else 0
+        val bottom = mapView.height - binding.roadNameLayout.top
+        return EdgeInsets(vPadding, hPadding + bottomSheetWidth, vPadding + bottom, hPadding)
     }
 
     private fun deviceOrientation() = binding.root.resources.configuration.orientation
diff --git a/libnavui-dropin/src/main/java/com/mapbox/navigation/dropin/map/MapViewBinder.kt b/libnavui-dropin/src/main/java/com/mapbox/navigation/dropin/map/MapViewBinder.kt
@@ -67,7 +67,7 @@ abstract class MapViewBinder : UIBinder {
         val store = context.store
         val navigationState = store.select { it.navigation }
         return navigationListOf(
-            CameraLayoutObserver(store, mapView, navigationViewBinding),
+            CameraLayoutObserver(context, mapView, navigationViewBinding),
             LocationComponent(context.locationProvider),
             context.locationPuckComponent(mapView),
             LogoAttributionComponent(mapView, context.systemBarsInsets),
diff --git a/libnavui-dropin/src/main/res/values-land/dimens.xml b/libnavui-dropin/src/main/res/values-land/dimens.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+    <!-- Overview camera padding to ensure location puck and target pin are not clipped -->
+    <dimen name="mapbox_camera_overview_padding_v">30dp</dimen>
+    <dimen name="mapbox_camera_overview_padding_h">35dp</dimen>
+</resources>
diff --git a/libnavui-dropin/src/main/res/values/dimens.xml b/libnavui-dropin/src/main/res/values/dimens.xml
@@ -25,10 +25,8 @@
     <dimen name="mapbox_maneuver_view_padding">4dp</dimen>
 
     <!-- Overview camera padding to ensure location puck and target pin are not clipped -->
-    <dimen name="mapbox_camera_overview_padding_v">50dp</dimen>
-    <dimen name="mapbox_camera_overview_padding_landscape_v">35dp</dimen>
+    <dimen name="mapbox_camera_overview_padding_v">30dp</dimen>
     <dimen name="mapbox_camera_overview_padding_h">50dp</dimen>
-    <dimen name="mapbox_camera_overview_padding_landscape_h">35dp</dimen>
 
     <dimen name="mapbox_extendable_button_width">72dp</dimen>
     <dimen name="mapbox_extendable_button_height">52dp</dimen>
diff --git a/libnavui-dropin/src/test/java/com/mapbox/navigation/dropin/camera/CameraLayoutObserverTest.kt b/libnavui-dropin/src/test/java/com/mapbox/navigation/dropin/camera/CameraLayoutObserverTest.kt
@@ -1,27 +1,29 @@
-@file:Suppress("PrivatePropertyName", "MaxLineLength")
-
 package com.mapbox.navigation.dropin.camera
 
 import android.app.Service
 import android.content.Context
 import android.view.LayoutInflater
 import android.view.View
-import android.view.View.OnLayoutChangeListener
 import android.widget.FrameLayout
+import androidx.core.view.doOnNextLayout
 import androidx.test.core.app.ApplicationProvider
+import com.google.android.material.bottomsheet.BottomSheetBehavior
 import com.mapbox.navigation.dropin.R
 import com.mapbox.navigation.dropin.databinding.MapboxNavigationViewLayoutBinding
+import com.mapbox.navigation.dropin.infopanel.InfoPanelBehavior
+import com.mapbox.navigation.dropin.navigationview.NavigationViewContext
+import com.mapbox.navigation.dropin.navigationview.NavigationViewModel
+import com.mapbox.navigation.dropin.testutil.TestLifecycleOwner
 import com.mapbox.navigation.dropin.util.TestStore
+import com.mapbox.navigation.testing.LoggingFrontendTestRule
 import com.mapbox.navigation.testing.MainCoroutineRule
 import com.mapbox.navigation.ui.app.internal.camera.CameraAction
-import com.mapbox.navigation.ui.app.internal.navigation.NavigationState
 import io.mockk.mockk
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNotEquals
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -37,9 +39,13 @@ class CameraLayoutObserverTest {
     @get:Rule
     val coroutineRule = MainCoroutineRule()
 
+    @get:Rule
+    val loggerRule = LoggingFrontendTestRule()
+
     private lateinit var store: TestStore
     private lateinit var binding: MapboxNavigationViewLayoutBinding
     private lateinit var mapView: View
+    private lateinit var infoPanelBehavior: InfoPanelBehavior
     private lateinit var sut: CameraLayoutObserver
 
     @Before
@@ -51,103 +57,66 @@ class CameraLayoutObserverTest {
         ).apply {
             // slightly modifying default layout to simulate layout changes
             guidanceLayout.bottom = 100
-            roadNameLayout.top = 100
-            actionListLayout.left = 100
-            infoPanelLayout.right = 100
+            roadNameLayout.top = 900
+            infoPanelLayout.right = 500
         }
-        mapView = View(context)
+        mapView = View(context).apply { bottom = 1000 }
         store = TestStore()
-        sut = CameraLayoutObserver(store, mapView, binding)
+        val navContext = NavigationViewContext(
+            context,
+            TestLifecycleOwner(),
+            NavigationViewModel(),
+        ) { store }
+        infoPanelBehavior = navContext.behavior.infoPanelBehavior
+        sut = CameraLayoutObserver(navContext, mapView, binding)
     }
 
     @Test
     @Config(qualifiers = "port")
-    fun `portrait - should update bottom padding for DestinationPreview, FreeDrive and RoutePreview state`() =
+    fun `portrait - should update camera paddings`() =
         coroutineRule.runBlockingTest {
             sut.onAttached(mockk())
 
-            triggerLayoutChangesAndVerifyDispatchedActions(
-                givenNavigationStates = listOf(
-                    NavigationState.DestinationPreview,
-                    NavigationState.FreeDrive,
-                    NavigationState.RoutePreview,
-                )
-            ) { state, action ->
-                assertEquals("$state|top", action.padding.top, PADDING_V_PORT, 0.001)
-                assertNotEquals("$state|bottom", action.padding.bottom, PADDING_V_PORT, 0.001)
-                assertEquals("$state|left", action.padding.left, PADDING_H_PORT, 0.001)
-                assertEquals("$state|right", action.padding.right, PADDING_H_PORT, 0.001)
-            }
-        }
-
-    @Test
-    @Config(qualifiers = "port")
-    fun `portrait - should update top and bottom padding for ActiveNavigation and Arrival state`() =
-        coroutineRule.runBlockingTest {
-            sut.onAttached(mockk())
+            binding.coordinatorLayout.triggerLayoutChange()
 
-            triggerLayoutChangesAndVerifyDispatchedActions(
-                givenNavigationStates = listOf(
-                    NavigationState.ActiveNavigation,
-                    NavigationState.Arrival,
-                )
-            ) { s, action ->
-                assertNotEquals("$s|top", action.padding.top, PADDING_V_PORT, 0.001)
-                assertNotEquals("$s|bottom", action.padding.bottom, PADDING_V_PORT, 0.001)
-                assertEquals("$s|left", action.padding.left, PADDING_H_PORT, 0.001)
-                assertEquals("$s|right", action.padding.right, PADDING_H_PORT, 0.001)
-            }
+            val action = store.actions.last() as CameraAction.UpdatePadding
+            assertEquals("left", paddingH, action.padding.left, 0.001)
+            assertEquals("top", 100 + paddingV, action.padding.top, 0.001)
+            assertEquals("right", paddingH, action.padding.right, 0.001)
+            assertEquals("bottom", 100 + paddingV, action.padding.bottom, 0.001)
         }
 
     @Test
     @Config(qualifiers = "land")
-    fun `landscape - should update bottom padding for FreeDrive state`() =
+    fun `landscape - should update camera paddings when bottom sheet is collapsed`() =
         coroutineRule.runBlockingTest {
             sut.onAttached(mockk())
 
-            triggerLayoutChangesAndVerifyDispatchedActions(
-                givenNavigationStates = listOf(
-                    NavigationState.FreeDrive
-                )
-            ) { s, action ->
-                assertEquals("$s|top", action.padding.top, PADDING_V_LAND, 0.001)
-                assertNotEquals("$s|bottom", action.padding.bottom, PADDING_V_LAND, 0.001)
-                assertEquals("$s|left", action.padding.left, PADDING_H_LAND, 0.001)
-                assertEquals("$s|right", action.padding.right, PADDING_H_LAND, 0.001)
-            }
+            infoPanelBehavior.updateBottomSheetState(BottomSheetBehavior.STATE_COLLAPSED)
+            binding.coordinatorLayout.triggerLayoutChange()
+
+            val action = store.actions.last() as CameraAction.UpdatePadding
+            assertEquals("left", 500 + paddingH, action.padding.left, 0.001)
+            assertEquals("top", paddingV, action.padding.top, 0.001)
+            assertEquals("right", paddingH, action.padding.right, 0.001)
+            assertEquals("bottom", 100 + paddingV, action.padding.bottom, 0.001)
         }
 
     @Test
     @Config(qualifiers = "land")
-    fun `landscape - should update left and bottom padding for all states except FreeDrive`() =
+    fun `landscape - should update camera paddings when bottom sheet is hidden`() =
         coroutineRule.runBlockingTest {
             sut.onAttached(mockk())
 
-            triggerLayoutChangesAndVerifyDispatchedActions(
-                givenNavigationStates = listOf(
-                    NavigationState.DestinationPreview,
-                    NavigationState.RoutePreview,
-                    NavigationState.ActiveNavigation,
-                    NavigationState.Arrival,
-                )
-            ) { s, action ->
-                assertEquals("$s|top", action.padding.top, PADDING_V_LAND, 0.001)
-                assertNotEquals("$s|bottom", action.padding.bottom, PADDING_V_LAND, 0.001)
-                assertNotEquals("$s|left", action.padding.left, PADDING_H_LAND, 0.001)
-                assertEquals("$s|right", action.padding.right, PADDING_H_LAND, 0.001)
-            }
-        }
-
-    private suspend fun triggerLayoutChangesAndVerifyDispatchedActions(
-        givenNavigationStates: List<NavigationState>,
-        assertAction: (state: NavigationState, action: CameraAction.UpdatePadding) -> Unit
-    ) {
-        givenNavigationStates.forEachIndexed { index, navState ->
-            store.updateState { it.copy(navigation = navState) }
+            infoPanelBehavior.updateBottomSheetState(BottomSheetBehavior.STATE_HIDDEN)
             binding.coordinatorLayout.triggerLayoutChange()
-            assertAction(navState, store.actions[index] as CameraAction.UpdatePadding)
+
+            val action = store.actions.last() as CameraAction.UpdatePadding
+            assertEquals("left", paddingH, action.padding.left, 0.001)
+            assertEquals("top", paddingV, action.padding.top, 0.001)
+            assertEquals("right", paddingH, action.padding.right, 0.001)
+            assertEquals("bottom", 100 + paddingV, action.padding.bottom, 0.001)
         }
-    }
 
     private suspend fun View.triggerLayoutChange() = coroutineScope {
         launch {
@@ -158,34 +127,13 @@ class CameraLayoutObserverTest {
     }
 
     private suspend fun View.waitForLayoutChange() = suspendCancellableCoroutine<Unit> { cont ->
-        addOnLayoutChangeListener(object : OnLayoutChangeListener {
-            override fun onLayoutChange(
-                v: View?,
-                left: Int,
-                top: Int,
-                right: Int,
-                bottom: Int,
-                oldLeft: Int,
-                oldTop: Int,
-                oldRight: Int,
-                oldBottom: Int
-            ) {
-                cont.resume(Unit)
-                removeOnLayoutChangeListener(this)
-            }
-        })
+        doOnNextLayout { cont.resume(Unit) }
     }
 
-    private val PADDING_V_PORT: Double
+    private val paddingV: Double
         get() = binding.root.resources
             .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_v).toDouble()
-    private val PADDING_H_PORT: Double
+    private val paddingH: Double
         get() = binding.root.resources
             .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_h).toDouble()
-    private val PADDING_V_LAND: Double
-        get() = binding.root.resources
-            .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_landscape_v).toDouble()
-    private val PADDING_H_LAND: Double
-        get() = binding.root.resources
-            .getDimensionPixelSize(R.dimen.mapbox_camera_overview_padding_landscape_h).toDouble()
 }
