1
0
Fork 0

For #12287: Address review comments

master
Jonathan Almeida 2020-08-18 18:19:11 -04:00 committed by Jonathan Almeida
parent f92485d1e8
commit 99fab556f4
7 changed files with 96 additions and 5 deletions

View File

@ -6,13 +6,21 @@ package org.mozilla.fenix.tabtray
import android.view.View import android.view.View
import androidx.fragment.app.FragmentManager.findFragment import androidx.fragment.app.FragmentManager.findFragment
import androidx.lifecycle.LifecycleOwner
import androidx.navigation.NavController import androidx.navigation.NavController
import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.findNavController
import androidx.recyclerview.widget.ConcatAdapter
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers 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 kotlinx.coroutines.launch
import mozilla.components.browser.storage.sync.SyncedDeviceTabs import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.feature.syncedtabs.view.SyncedTabsView 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.ListenerDelegate
import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.sync.SyncedTabsAdapter
import org.mozilla.fenix.sync.ext.toAdapterList 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 org.mozilla.fenix.sync.ext.toStringRes
import kotlin.coroutines.CoroutineContext import kotlin.coroutines.CoroutineContext
@OptIn(ExperimentalCoroutinesApi::class)
class SyncedTabsController( class SyncedTabsController(
lifecycleOwner: LifecycleOwner,
private val view: View, private val view: View,
store: TabTrayDialogFragmentStore,
private val concatAdapter: ConcatAdapter,
coroutineContext: CoroutineContext = Dispatchers.Main coroutineContext: CoroutineContext = Dispatchers.Main
) : SyncedTabsView { ) : SyncedTabsView {
override var listener: SyncedTabsView.Listener? = null override var listener: SyncedTabsView.Listener? = null
@ -30,6 +42,24 @@ class SyncedTabsController(
private val scope: CoroutineScope = CoroutineScope(coroutineContext) 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<SyncedDeviceTabs>) { override fun displaySyncedTabs(syncedTabs: List<SyncedDeviceTabs>) {
scope.launch { scope.launch {
val tabsList = listOf(SyncedTabsAdapter.AdapterItem.Title) + syncedTabs.toAdapterList() val tabsList = listOf(SyncedTabsAdapter.AdapterItem.Title) + syncedTabs.toAdapterList()

View File

@ -192,6 +192,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
showAddNewCollectionDialog = ::showAddNewCollectionDialog showAddNewCollectionDialog = ::showAddNewCollectionDialog
) )
), ),
store = tabTrayDialogStore,
isPrivate = isPrivate, isPrivate = isPrivate,
startingInLandscape = requireContext().resources.configuration.orientation == startingInLandscape = requireContext().resources.configuration.orientation ==
Configuration.ORIENTATION_LANDSCAPE, Configuration.ORIENTATION_LANDSCAPE,

View File

@ -60,6 +60,7 @@ class TabTrayView(
private val container: ViewGroup, private val container: ViewGroup,
private val tabsAdapter: FenixTabsAdapter, private val tabsAdapter: FenixTabsAdapter,
private val interactor: TabTrayInteractor, private val interactor: TabTrayInteractor,
store: TabTrayDialogFragmentStore,
isPrivate: Boolean, isPrivate: Boolean,
startingInLandscape: Boolean, startingInLandscape: Boolean,
lifecycleOwner: LifecycleOwner, lifecycleOwner: LifecycleOwner,
@ -78,13 +79,14 @@ class TabTrayView(
private val behavior = BottomSheetBehavior.from(view.tab_wrapper) private val behavior = BottomSheetBehavior.from(view.tab_wrapper)
private val concatAdapter = ConcatAdapter(tabsAdapter)
private val tabTrayItemMenu: TabTrayItemMenu private val tabTrayItemMenu: TabTrayItemMenu
private var menu: BrowserMenu? = null private var menu: BrowserMenu? = null
private var tabsTouchHelper: TabsTouchHelper private var tabsTouchHelper: TabsTouchHelper
private val collectionsButtonAdapter = SaveToCollectionsButtonAdapter(interactor, isPrivate) private val collectionsButtonAdapter = SaveToCollectionsButtonAdapter(interactor, isPrivate)
private val syncedTabsController = SyncedTabsController(view) private val syncedTabsController = SyncedTabsController(lifecycleOwner, view, store, concatAdapter)
private val syncedTabsFeature = ViewBoundFeatureWrapper<SyncedTabsFeature>() private val syncedTabsFeature = ViewBoundFeatureWrapper<SyncedTabsFeature>()
private var hasLoaded = false private var hasLoaded = false
@ -160,7 +162,6 @@ class TabTrayView(
) )
} }
val concatAdapter = ConcatAdapter(tabsAdapter)
view.tabsTray.apply { view.tabsTray.apply {
layoutManager = LinearLayoutManager(container.context).apply { layoutManager = LinearLayoutManager(container.context).apply {
reverseLayout = true reverseLayout = true

View File

@ -1,4 +1,7 @@
<?xml version="1.0" encoding="utf-8"?> <?xml version="1.0" encoding="utf-8"?>
<!-- This Source Code Form is subject to the terms of the Mozilla Public
License, v. 2.0. If a copy of the MPL was not distributed with this
file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<rotate xmlns:android="http://schemas.android.com/apk/res/android" <rotate xmlns:android="http://schemas.android.com/apk/res/android"
android:duration="275" android:duration="275"

View File

@ -6,9 +6,10 @@
xmlns:android="http://schemas.android.com/apk/res/android" xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools" xmlns:tools="http://schemas.android.com/tools"
android:paddingTop="6dp"
android:paddingBottom="8dp"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginBottom="8dp"
android:background="?android:attr/selectableItemBackground"> android:background="?android:attr/selectableItemBackground">
<TextView <TextView
@ -17,7 +18,6 @@
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="72dp" android:layout_marginStart="72dp"
android:layout_marginEnd="48dp" android:layout_marginEnd="48dp"
android:layout_marginTop="6dp"
android:singleLine="true" android:singleLine="true"
android:textAlignment="viewStart" android:textAlignment="viewStart"
android:textColor="?primaryText" android:textColor="?primaryText"

View File

@ -23,6 +23,7 @@
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginTop="60dp" android:layout_marginTop="60dp"
android:layout_marginEnd="16dp" android:layout_marginEnd="16dp"
android:background="?android:attr/selectableItemBackgroundBorderless"
app:srcCompat="@drawable/mozac_ic_refresh" app:srcCompat="@drawable/mozac_ic_refresh"
app:tint="?primaryText" /> app:tint="?primaryText" />

View File

@ -2,31 +2,70 @@ package org.mozilla.fenix.tabtray
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View 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.mockk
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineDispatcher
import kotlinx.coroutines.test.runBlockingTest import kotlinx.coroutines.test.runBlockingTest
import mozilla.components.browser.storage.sync.SyncedDeviceTabs import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.feature.syncedtabs.view.SyncedTabsView.ErrorType 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.robolectric.testContext
import mozilla.components.support.test.rule.MainCoroutineRule
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Before import org.junit.Before
import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.sync.SyncedTabsViewHolder 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 @ExperimentalCoroutinesApi
@RunWith(FenixRobolectricTestRunner::class) @RunWith(FenixRobolectricTestRunner::class)
class SyncedTabsControllerTest { class SyncedTabsControllerTest {
private val testDispatcher = TestCoroutineDispatcher()
@get:Rule
val coroutinesTestRule = MainCoroutineRule(testDispatcher)
private lateinit var view: View private lateinit var view: View
private lateinit var controller: SyncedTabsController 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 @Before
fun setup() = runBlockingTest { 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) view = LayoutInflater.from(testContext).inflate(R.layout.about_list_item, null)
controller = SyncedTabsController(view, coroutineContext) controller =
SyncedTabsController(lifecycleOwner, view, store, concatAdapter, coroutineContext)
} }
@Test @Test
@ -75,4 +114,20 @@ class SyncedTabsControllerTest {
controller.adapter.getItemViewType(0) 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()) }
}
} }