From 99fab556f4c0d5c0000c7bb45665585f5782be94 Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Tue, 18 Aug 2020 18:19:11 -0400 Subject: [PATCH] For #12287: Address review comments --- .../fenix/tabtray/SyncedTabsController.kt | 30 ++++++++++ .../fenix/tabtray/TabTrayDialogFragment.kt | 1 + .../org/mozilla/fenix/tabtray/TabTrayView.kt | 5 +- app/src/main/res/anim/full_rotation.xml | 3 + .../main/res/layout/sync_tabs_list_item.xml | 4 +- .../res/layout/view_synced_tabs_title.xml | 1 + .../fenix/tabtray/SyncedTabsControllerTest.kt | 57 ++++++++++++++++++- 7 files changed, 96 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt b/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt index 2e47fff15..8fbb87c63 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt @@ -6,13 +6,21 @@ package org.mozilla.fenix.tabtray import android.view.View import androidx.fragment.app.FragmentManager.findFragment +import androidx.lifecycle.LifecycleOwner import androidx.navigation.NavController import androidx.navigation.fragment.findNavController +import androidx.recyclerview.widget.ConcatAdapter import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import mozilla.components.browser.storage.sync.SyncedDeviceTabs import mozilla.components.feature.syncedtabs.view.SyncedTabsView +import mozilla.components.lib.state.ext.flowScoped +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.sync.ListenerDelegate import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.sync.ext.toAdapterList @@ -20,8 +28,12 @@ import org.mozilla.fenix.sync.ext.toAdapterItem import org.mozilla.fenix.sync.ext.toStringRes import kotlin.coroutines.CoroutineContext +@OptIn(ExperimentalCoroutinesApi::class) class SyncedTabsController( + lifecycleOwner: LifecycleOwner, private val view: View, + store: TabTrayDialogFragmentStore, + private val concatAdapter: ConcatAdapter, coroutineContext: CoroutineContext = Dispatchers.Main ) : SyncedTabsView { override var listener: SyncedTabsView.Listener? = null @@ -30,6 +42,24 @@ class SyncedTabsController( private val scope: CoroutineScope = CoroutineScope(coroutineContext) + init { + store.flowScoped(lifecycleOwner) { flow -> + flow.map { it.mode } + .ifChanged() + .drop(1) + .collect { mode -> + when (mode) { + is TabTrayDialogFragmentState.Mode.Normal -> { + concatAdapter.addAdapter(0, adapter) + } + is TabTrayDialogFragmentState.Mode.MultiSelect -> { + concatAdapter.removeAdapter(adapter) + } + } + } + } + } + override fun displaySyncedTabs(syncedTabs: List) { scope.launch { val tabsList = listOf(SyncedTabsAdapter.AdapterItem.Title) + syncedTabs.toAdapterList() diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt index be5ec1818..3b64217f4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt @@ -192,6 +192,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler showAddNewCollectionDialog = ::showAddNewCollectionDialog ) ), + store = tabTrayDialogStore, isPrivate = isPrivate, startingInLandscape = requireContext().resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE, diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index 07426f456..76574c692 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -60,6 +60,7 @@ class TabTrayView( private val container: ViewGroup, private val tabsAdapter: FenixTabsAdapter, private val interactor: TabTrayInteractor, + store: TabTrayDialogFragmentStore, isPrivate: Boolean, startingInLandscape: Boolean, lifecycleOwner: LifecycleOwner, @@ -78,13 +79,14 @@ class TabTrayView( private val behavior = BottomSheetBehavior.from(view.tab_wrapper) + private val concatAdapter = ConcatAdapter(tabsAdapter) private val tabTrayItemMenu: TabTrayItemMenu private var menu: BrowserMenu? = null private var tabsTouchHelper: TabsTouchHelper private val collectionsButtonAdapter = SaveToCollectionsButtonAdapter(interactor, isPrivate) - private val syncedTabsController = SyncedTabsController(view) + private val syncedTabsController = SyncedTabsController(lifecycleOwner, view, store, concatAdapter) private val syncedTabsFeature = ViewBoundFeatureWrapper() private var hasLoaded = false @@ -160,7 +162,6 @@ class TabTrayView( ) } - val concatAdapter = ConcatAdapter(tabsAdapter) view.tabsTray.apply { layoutManager = LinearLayoutManager(container.context).apply { reverseLayout = true diff --git a/app/src/main/res/anim/full_rotation.xml b/app/src/main/res/anim/full_rotation.xml index 357a54041..eefb51740 100644 --- a/app/src/main/res/anim/full_rotation.xml +++ b/app/src/main/res/anim/full_rotation.xml @@ -1,4 +1,7 @@ + diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt index c3d7b33de..6ff967c84 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt @@ -2,31 +2,70 @@ package org.mozilla.fenix.tabtray import android.view.LayoutInflater import android.view.View +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.recyclerview.widget.ConcatAdapter +import io.mockk.Called +import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.runBlockingTest import mozilla.components.browser.storage.sync.SyncedDeviceTabs import mozilla.components.feature.syncedtabs.view.SyncedTabsView.ErrorType +import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertEquals import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.sync.SyncedTabsViewHolder +import org.mozilla.fenix.tabtray.TabTrayDialogFragmentAction.EnterMultiSelectMode +import org.mozilla.fenix.tabtray.TabTrayDialogFragmentAction.ExitMultiSelectMode +import org.mozilla.fenix.tabtray.TabTrayDialogFragmentState.Mode @ExperimentalCoroutinesApi @RunWith(FenixRobolectricTestRunner::class) class SyncedTabsControllerTest { + private val testDispatcher = TestCoroutineDispatcher() + @get:Rule + val coroutinesTestRule = MainCoroutineRule(testDispatcher) + private lateinit var view: View private lateinit var controller: SyncedTabsController + private lateinit var lifecycleOwner: LifecycleOwner + private lateinit var lifecycle: LifecycleRegistry + private lateinit var concatAdapter: ConcatAdapter + private lateinit var store: TabTrayDialogFragmentStore @Before fun setup() = runBlockingTest { + lifecycleOwner = mockk() + lifecycle = LifecycleRegistry(lifecycleOwner) + lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START) + every { lifecycleOwner.lifecycle } returns lifecycle + + concatAdapter = mockk() + every { concatAdapter.addAdapter(any(), any()) } returns true + every { concatAdapter.removeAdapter(any()) } returns true + + store = TabTrayDialogFragmentStore( + initialState = TabTrayDialogFragmentState( + mode = Mode.Normal, + browserState = mockk(relaxed = true) + ) + ) + view = LayoutInflater.from(testContext).inflate(R.layout.about_list_item, null) - controller = SyncedTabsController(view, coroutineContext) + controller = + SyncedTabsController(lifecycleOwner, view, store, concatAdapter, coroutineContext) } @Test @@ -75,4 +114,20 @@ class SyncedTabsControllerTest { controller.adapter.getItemViewType(0) ) } + + @Test + fun `do nothing on init, drop first event`() { + verify { concatAdapter wasNot Called } + } + + @Test + fun `concatAdapter updated on mode changes`() = testDispatcher.runBlockingTest { + store.dispatch(EnterMultiSelectMode).joinBlocking() + + verify { concatAdapter.removeAdapter(any()) } + + store.dispatch(ExitMultiSelectMode).joinBlocking() + + verify { concatAdapter.addAdapter(0, any()) } + } }