From 2c6395cafef5a585bef25b7bbf485af99703cbf5 Mon Sep 17 00:00:00 2001
From: Jonathan Almeida <jalmeida@mozilla.com>
Date: Tue, 30 Mar 2021 14:51:43 -0400
Subject: [PATCH] Issue #18535: Do not animate first scroll to position

This looks less that ideal with a grid layout that swings by from the
normal tabs to private tabs.
---
 .../mozilla/fenix/tabstray/TabLayoutMediator.kt | 13 ++++++++++++-
 .../mozilla/fenix/tabstray/TabsTrayFragment.kt  |  4 ++--
 .../fenix/tabstray/TabsTrayInteractor.kt        |  5 ++++-
 .../fenix/tabstray/TabLayoutObserverTest.kt     | 17 ++++++++++++++++-
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
index 25f1d6b62..fb41c0235 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
@@ -57,8 +57,19 @@ class TabLayoutMediator(
 internal class TabLayoutObserver(
     private val interactor: TabsTrayInteractor
 ) : TabLayout.OnTabSelectedListener {
+
+    private var initialScroll = true
+
     override fun onTabSelected(tab: TabLayout.Tab) {
-        interactor.setCurrentTrayPosition(tab.position)
+        // Do not animate the initial scroll when opening the tabs tray.
+        val animate = if (initialScroll) {
+            initialScroll = false
+            false
+        } else {
+            true
+        }
+
+        interactor.setCurrentTrayPosition(tab.position, animate)
     }
 
     override fun onTabUnselected(tab: TabLayout.Tab) = Unit
diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
index a2dea00ae..5ceead5e4 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
@@ -104,8 +104,8 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor {
         }
     }
 
-    override fun setCurrentTrayPosition(position: Int) {
-        tabsTray.currentItem = position
+    override fun setCurrentTrayPosition(position: Int, smoothScroll: Boolean) {
+        tabsTray.setCurrentItem(position, smoothScroll)
     }
 
     override fun navigateToBrowser() {
diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt
index 2544d66a3..9fd950a6b 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt
@@ -7,8 +7,11 @@ package org.mozilla.fenix.tabstray
 interface TabsTrayInteractor {
     /**
      * Set the current tray item to the clamped [position].
+     *
+     * @param position The position on the tray to focus.
+     * @param smoothScroll If true, animate the scrolling from the current tab to [position].
      */
-    fun setCurrentTrayPosition(position: Int)
+    fun setCurrentTrayPosition(position: Int, smoothScroll: Boolean)
 
     /**
      * Dismisses the tabs tray and navigates to the browser.
diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt
index 4ee3645ae..ba14eea72 100644
--- a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt
@@ -21,6 +21,21 @@ class TabLayoutObserverTest {
 
         observer.onTabSelected(tab)
 
-        verify { interactor.setCurrentTrayPosition(1) }
+        verify { interactor.setCurrentTrayPosition(1, false) }
+    }
+
+    @Test
+    fun `WHEN observer is first started THEN do not smooth scroll`() {
+        val observer = TabLayoutObserver(interactor)
+        val tab = mockk<TabLayout.Tab>()
+        every { tab.position } returns 1
+
+        observer.onTabSelected(tab)
+
+        verify { interactor.setCurrentTrayPosition(1, false) }
+
+        observer.onTabSelected(tab)
+
+        verify { interactor.setCurrentTrayPosition(1, true) }
     }
 }