Merge #7102

7102: Make sure the bottom toolbar is never stuck invisible r=Mugurell a=miDeb Closes #7101. This is my attempt to solve this issue. Essentially, if the user is scrolling upwards, but the toolbar is not being scrolled and not already showing or animating up, just forceExpand it. Co-authored-by: Michael <[email protected]>
mozilla-mobile · May 29, 2020 · cb312ff · cb312ff
2 parents 2a18bde + 3307103
commit cb312ff
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 0 deletions.
diff --git a/...src/main/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBottomBehavior.kt b/...src/main/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBottomBehavior.kt
@@ -44,6 +44,8 @@ class BrowserToolbarBottomBehavior(
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal var shouldSnapAfterScroll: Boolean = false
 
+    private var lastSnapStartedWasUp = false
+
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal var snapAnimator: ValueAnimator = ValueAnimator().apply {
         interpolator = DecelerateInterpolator()
@@ -147,6 +149,7 @@ class BrowserToolbarBottomBehavior(
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun animateSnap(child: View, direction: SnapDirection) = with(snapAnimator) {
+        lastSnapStartedWasUp = direction == SnapDirection.UP
         addUpdateListener { child.translationY = it.animatedValue as Float }
         setFloatValues(child.translationY, if (direction == SnapDirection.UP) 0f else child.height.toFloat())
         start()
@@ -188,6 +191,15 @@ class BrowserToolbarBottomBehavior(
         if (shouldScroll) {
             browserToolbar.translationY =
                 max(0f, min(browserToolbar.height.toFloat(), browserToolbar.translationY + distance))
+        } else {
+            // Force expand the toolbar if the user scrolled up, it is not already expanded and
+            // an animation to expand it is not already in progress,
+            // otherwise the user could get stuck in a state where they cannot show the toolbar
+            val isAnimatingUp = snapAnimator.isStarted && lastSnapStartedWasUp
+            if (distance < 0 && browserToolbar.translationY != 0f && !isAnimatingUp) {
+                snapAnimator.cancel()
+                forceExpand(browserToolbar)
+            }
         }
     }
 

diff --git a/...test/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBottomBehaviorTest.kt b/...test/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBottomBehaviorTest.kt
@@ -153,6 +153,8 @@ class BrowserToolbarBottomBehaviorTest {
     fun `Behavior will apply translation to toolbar only for vertical scrolls`() {
         val behavior = spy(BrowserToolbarBottomBehavior(testContext, attrs = null))
         behavior.initGesturesDetector(behavior.createGestureDetector())
+        val child = spy(BrowserToolbar(testContext, null, 0))
+        behavior.browserToolbar = child
         val downEvent = TestUtils.getMotionEvent(ACTION_DOWN, 0f, 0f)
         val moveEvent = TestUtils.getMotionEvent(ACTION_MOVE, 0f, 100f, downEvent)
 
@@ -317,4 +319,28 @@ class BrowserToolbarBottomBehaviorTest {
 
         verify(behavior).animateSnap(toolbar, SnapDirection.UP)
     }
+
+    @Test
+    fun `Behavior will forceExpand when scrolling up and !shouldScroll`() {
+        val behavior = spy(BrowserToolbarBottomBehavior(testContext, attrs = null))
+        behavior.initGesturesDetector(behavior.createGestureDetector())
+        doReturn(false).`when`(behavior).shouldScroll
+        val toolbar: BrowserToolbar = spy(BrowserToolbar(testContext, null, 0))
+        behavior.browserToolbar = toolbar
+        val animator: ValueAnimator = mock()
+        behavior.snapAnimator = animator
+        doReturn(false).`when`(animator).isStarted
+
+        doReturn(100).`when`(toolbar).height
+        doReturn(100f).`when`(toolbar).translationY
+
+        val downEvent = TestUtils.getMotionEvent(ACTION_DOWN, 0f, 0f)
+        val moveEvent = TestUtils.getMotionEvent(ACTION_MOVE, 0f, 30f, downEvent)
+
+        behavior.onInterceptTouchEvent(mock(), mock(), downEvent)
+        behavior.onInterceptTouchEvent(mock(), mock(), moveEvent)
+
+        verify(behavior).tryToScrollVertically(-30f)
+        verify(behavior).forceExpand(toolbar)
+    }
 }