From ef5846dc3ed87bae3ec930cc535f3d1448e2891e Mon Sep 17 00:00:00 2001 From: Mugurell Date: Tue, 17 Nov 2020 20:33:41 +0200 Subject: [PATCH] For #16614 - Ensure a stable order for the items in tabs tray. (#16618) Items should follow the following ordering: - current session open tabs - collections options - currently the "Select tabs" button - synced tabs items This order should also be kept after returning from Multiselect mode. --- .../mozilla/fenix/tabtray/SyncedTabsController.kt | 7 +------ .../java/org/mozilla/fenix/tabtray/TabTrayView.kt | 9 ++------- .../fenix/tabtray/SyncedTabsControllerTest.kt | 15 +-------------- 3 files changed, 4 insertions(+), 27 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 d878a35170..c415790950 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt @@ -21,7 +21,6 @@ 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.ext.settings import org.mozilla.fenix.sync.ListenerDelegate import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.sync.ext.toAdapterList @@ -51,11 +50,7 @@ class SyncedTabsController( .collect { mode -> when (mode) { is TabTrayDialogFragmentState.Mode.Normal -> { - if (view.context.settings().gridTabView) { - concatAdapter.addAdapter(adapter) - } else { - concatAdapter.addAdapter(0, adapter) - } + concatAdapter.addAdapter(adapter) } is TabTrayDialogFragmentState.Mode.MultiSelect -> { concatAdapter.removeAdapter(adapter) 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 422fefb89f..5512320b0c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -192,13 +192,8 @@ class TabTrayView( tabsAdapter.tabTrayInteractor = interactor tabsAdapter.onTabsUpdated = { - if (view.context.settings().gridTabView) { - concatAdapter.addAdapter(syncedTabsController.adapter) - concatAdapter.addAdapter(collectionsButtonAdapter) - } else { - concatAdapter.addAdapter(syncedTabsController.adapter) - concatAdapter.addAdapter(collectionsButtonAdapter) - } + concatAdapter.addAdapter(collectionsButtonAdapter) + concatAdapter.addAdapter(syncedTabsController.adapter) if (hasAccessibilityEnabled) { tabsAdapter.notifyItemRangeChanged(0, tabs.size) 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 26e3ecb352..962611317a 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt @@ -25,7 +25,6 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.sync.SyncedTabsViewHolder import org.mozilla.fenix.tabtray.TabTrayDialogFragmentAction.EnterMultiSelectMode @@ -56,7 +55,6 @@ class SyncedTabsControllerTest { concatAdapter = mockk() every { concatAdapter.addAdapter(any()) } returns true - every { concatAdapter.addAdapter(any(), any()) } returns true every { concatAdapter.removeAdapter(any()) } returns true store = TabTrayDialogFragmentStore( @@ -130,22 +128,11 @@ class SyncedTabsControllerTest { @Test fun `concatAdapter updated on mode changes`() = testDispatcher.runBlockingTest { - // When returning from Multiselect while in grid view the adapter should be added at the end - every { view.context.settings().gridTabView } returns true - store.dispatch(EnterMultiSelectMode).joinBlocking() verify { concatAdapter.removeAdapter(any()) } store.dispatch(ExitMultiSelectMode).joinBlocking() + // When returning from Multiselect the adapter should be added at the end verify { concatAdapter.addAdapter(any()) } - - // When returning from Multiselect while in list view the adapter should be added at the front - every { view.context.settings().gridTabView } returns false - - store.dispatch(EnterMultiSelectMode).joinBlocking() - verify { concatAdapter.removeAdapter(any()) } - - store.dispatch(ExitMultiSelectMode).joinBlocking() - verify { concatAdapter.addAdapter(0, any()) } } }