Close #13892: Remove Synced Tabs appended to tabs tray

upstream-sync
Jonathan Almeida 3 years ago committed by Jonathan Almeida
parent 516a6a343f
commit 2f6fcbf196

@ -14,13 +14,6 @@ object FeatureFlags {
*/
val pullToRefreshEnabled = Config.channel.isNightlyOrDebug
/**
* Shows Synced Tabs in the tabs tray.
*
* Tracking issue: https://github.com/mozilla-mobile/fenix/issues/13892
*/
val syncedTabsInTabsTray = Config.channel.isNightlyOrDebug
/**
* Enables the Nimbus experiments library.
*/

@ -332,14 +332,12 @@ class DefaultToolbarMenu(
?.browsingModeManager?.mode == BrowsingMode.Normal
val shouldDeleteDataOnQuit = context.components.settings
.shouldDeleteBrowsingDataOnQuit
val syncedTabsInTabsTray = context.components.settings
.syncedTabsInTabsTray
val menuItems = listOfNotNull(
downloadsItem,
historyItem,
bookmarksItem,
if (syncedTabsInTabsTray) null else syncedTabs,
syncedTabs,
settings,
if (shouldDeleteDataOnQuit) deleteDataOnQuit else null,
BrowserMenuDivider(),
@ -471,9 +469,6 @@ class DefaultToolbarMenu(
onItemTapped.invoke(ToolbarMenu.Item.Settings)
}
val syncedTabsInTabsTray = context.components.settings
.syncedTabsInTabsTray
val menuItems = listOfNotNull(
newTabItem,
BrowserMenuDivider(),
@ -481,7 +476,7 @@ class DefaultToolbarMenu(
historyItem,
downloadsItem,
extensionsItem,
if (syncedTabsInTabsTray) null else syncedTabsItem,
syncedTabsItem,
BrowserMenuDivider(),
findInPageItem,
desktopSiteItem,

@ -190,7 +190,7 @@ class HomeMenu(
if (settings.shouldDeleteBrowsingDataOnQuit) quitItem else null,
settingsItem,
BrowserMenuDivider(),
if (settings.syncedTabsInTabsTray) null else syncedTabsItem,
syncedTabsItem,
bookmarksItem,
historyItem,
downloadsItem,

@ -6,10 +6,7 @@ package org.mozilla.fenix.settings
import android.os.Bundle
import androidx.preference.PreferenceFragmentCompat
import androidx.preference.SwitchPreference
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.ext.showToolbar
class SecretSettingsFragment : PreferenceFragmentCompat() {
@ -21,11 +18,5 @@ class SecretSettingsFragment : PreferenceFragmentCompat() {
override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
setPreferencesFromResource(R.xml.secret_settings_preferences, rootKey)
requirePreference<SwitchPreference>(R.string.pref_key_synced_tabs_tabs_tray).apply {
isVisible = FeatureFlags.syncedTabsInTabsTray
isChecked = context.settings().syncedTabsInTabsTray
onPreferenceChangeListener = SharedPreferenceUpdater()
}
}
}

@ -17,7 +17,6 @@ import mozilla.components.browser.storage.sync.Tab
import mozilla.components.feature.syncedtabs.SyncedTabsFeature
import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
@ -28,11 +27,6 @@ import org.mozilla.fenix.theme.ThemeManager
class SyncedTabsFragment : LibraryPageFragment<Tab>() {
private val syncedTabsFeature = ViewBoundFeatureWrapper<SyncedTabsFeature>()
init {
// Sanity-check: Remove this class when the feature flag is always enabled.
FeatureFlags.syncedTabsInTabsTray
}
override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,

@ -19,7 +19,6 @@ import kotlinx.coroutines.launch
import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.browser.storage.sync.Tab
import mozilla.components.feature.syncedtabs.view.SyncedTabsView
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
@ -47,9 +46,6 @@ class SyncedTabsLayout @JvmOverloads constructor(
synced_tabs_list.adapter = adapter
synced_tabs_pull_to_refresh.setOnRefreshListener { listener?.onRefresh() }
// Sanity-check: Remove this class when the feature flag is always enabled.
FeatureFlags.syncedTabsInTabsTray
}
override fun onError(error: SyncedTabsView.ErrorType) {

@ -1,87 +0,0 @@
/* 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/. */
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.components.metrics.MetricController
import org.mozilla.fenix.sync.ListenerDelegate
import org.mozilla.fenix.sync.SyncedTabsAdapter
import org.mozilla.fenix.sync.ext.toAdapterList
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,
metrics: MetricController
) : SyncedTabsView {
override var listener: SyncedTabsView.Listener? = null
val adapter = SyncedTabsAdapter(ListenerDelegate(metrics) { listener })
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(adapter)
}
is TabTrayDialogFragmentState.Mode.MultiSelect -> {
concatAdapter.removeAdapter(adapter)
}
}
}
}
}
override fun displaySyncedTabs(syncedTabs: List<SyncedDeviceTabs>) {
scope.launch {
val tabsList = listOf(SyncedTabsAdapter.AdapterItem.Title) + syncedTabs.toAdapterList()
// Reverse layout for TabTrayView which does things backwards.
adapter.submitList(tabsList.reversed())
}
}
override fun onError(error: SyncedTabsView.ErrorType) {
scope.launch {
val navController: NavController? = try {
findFragment<TabTrayDialogFragment>(view).findNavController()
} catch (exception: IllegalStateException) {
null
}
val descriptionResId = error.toStringRes()
val errorItem = error.toAdapterItem(descriptionResId, navController)
adapter.submitList(listOf(errorItem))
}
}
}

@ -20,7 +20,6 @@ import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.concept.tabstray.Tab
import mozilla.components.feature.tabs.TabsUseCases
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
@ -28,7 +27,6 @@ import org.mozilla.fenix.components.TabCollectionStorage
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.home.HomeFragment
import mozilla.components.browser.storage.sync.Tab as SyncTab
/**
* [TabTrayDialogFragment] controller.
@ -42,7 +40,6 @@ interface TabTrayController {
fun handleTabSettingsClicked()
fun handleShareTabsOfTypeClicked(private: Boolean)
fun handleShareSelectedTabsClicked(selectedTabs: Set<Tab>)
fun handleSyncedTabClicked(syncTab: SyncTab)
fun handleSaveToCollectionClicked(selectedTabs: Set<Tab>)
fun handleBookmarkSelectedTabs(selectedTabs: Set<Tab>)
fun handleDeleteSelectedTabs(selectedTabs: Set<Tab>)
@ -197,14 +194,6 @@ class DefaultTabTrayController(
}
}
override fun handleSyncedTabClicked(syncTab: SyncTab) {
activity.openToBrowserAndLoad(
searchTermOrURL = syncTab.active().url,
newTab = true,
from = BrowserDirection.FromTabTray
)
}
@OptIn(ExperimentalCoroutinesApi::class)
override fun handleCloseAllTabsClicked(private: Boolean) {
val sessionsToClose = if (private) {

@ -215,7 +215,6 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
showBookmarksSnackbar = ::showBookmarksSnackbar
)
),
store = tabTrayDialogStore,
isPrivate = isPrivate,
isInLandscape = ::isInLandscape,
lifecycleOwner = viewLifecycleOwner

@ -5,7 +5,6 @@
package org.mozilla.fenix.tabtray
import mozilla.components.concept.tabstray.Tab
import mozilla.components.browser.storage.sync.Tab as SyncTab
@Suppress("TooManyFunctions")
interface TabTrayInteractor {
@ -54,11 +53,6 @@ interface TabTrayInteractor {
*/
fun onCloseAllTabsClicked(private: Boolean)
/**
* Called when the user clicks on a synced tab entry.
*/
fun onSyncedTabClicked(syncTab: SyncTab)
/**
* Called when the physical back button is clicked.
*/
@ -146,10 +140,6 @@ class TabTrayFragmentInteractor(private val controller: TabTrayController) : Tab
controller.handleCloseAllTabsClicked(private)
}
override fun onSyncedTabClicked(syncTab: SyncTab) {
controller.handleSyncedTabClicked(syncTab)
}
override fun onBackPressed(): Boolean {
return controller.handleBackPressed()
}

@ -41,8 +41,6 @@ import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.tabstray.TabViewHolder
import mozilla.components.feature.syncedtabs.SyncedTabsFeature
import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
import mozilla.components.support.ktx.android.util.dpToPx
import mozilla.components.ui.tabcounter.TabCounter.Companion.INFINITE_CHAR_PADDING_BOTTOM
import org.mozilla.fenix.R
@ -59,7 +57,6 @@ import org.mozilla.fenix.utils.Settings
import java.text.NumberFormat
import kotlin.math.max
import kotlin.math.roundToInt
import mozilla.components.browser.storage.sync.Tab as SyncTab
/**
* View that contains and configures the BrowserAwesomeBar
@ -69,7 +66,6 @@ class TabTrayView(
private val container: ViewGroup,
private val tabsAdapter: FenixTabsAdapter,
private val interactor: TabTrayInteractor,
store: TabTrayDialogFragmentStore,
isPrivate: Boolean,
private val isInLandscape: () -> Boolean,
lifecycleOwner: LifecycleOwner,
@ -97,11 +93,6 @@ class TabTrayView(
private var tabsTouchHelper: TabsTouchHelper
private val collectionsButtonAdapter = SaveToCollectionsButtonAdapter(interactor, isPrivate)
private val metrics = container.context.components.analytics.metrics
private val syncedTabsController =
SyncedTabsController(lifecycleOwner, view, store, concatAdapter, metrics = metrics)
private val syncedTabsFeature = ViewBoundFeatureWrapper<SyncedTabsFeature>()
private var hasLoaded = false
@ -167,21 +158,6 @@ class TabTrayView(
setTopOffset(isInLandscape())
if (view.context.settings().syncedTabsInTabsTray) {
syncedTabsFeature.set(
feature = SyncedTabsFeature(
context = container.context,
storage = components.backgroundServices.syncedTabsStorage,
accountManager = components.backgroundServices.accountManager,
view = syncedTabsController,
lifecycleOwner = lifecycleOwner,
onTabClicked = ::handleTabClicked
),
owner = lifecycleOwner,
view = view
)
}
updateTabsTrayLayout()
view.tabsTray.apply {
@ -197,7 +173,6 @@ class TabTrayView(
tabsAdapter.tabTrayInteractor = interactor
tabsAdapter.onTabsUpdated = {
concatAdapter.addAdapter(collectionsButtonAdapter)
concatAdapter.addAdapter(syncedTabsController.adapter)
if (hasAccessibilityEnabled) {
tabsAdapter.notifyItemRangeChanged(0, tabs.size)
@ -344,10 +319,6 @@ class TabTrayView(
}
}
private fun handleTabClicked(tab: SyncTab) {
interactor.onSyncedTabClicked(tab)
}
private fun adjustNewTabButtonsForNormalMode() {
view.tab_tray_new_tab.apply {
isVisible = hasAccessibilityEnabled

@ -26,12 +26,10 @@ import mozilla.components.support.ktx.android.content.longPreference
import mozilla.components.support.ktx.android.content.stringPreference
import org.mozilla.fenix.BuildConfig
import org.mozilla.fenix.Config
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.metrics.MozillaProductDetector
import org.mozilla.fenix.components.settings.counterPreference
import org.mozilla.fenix.components.settings.featureFlagPreference
import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getPreferenceKey
@ -123,12 +121,6 @@ class Settings(private val appContext: Context) : PreferencesHolder {
val canShowCfr: Boolean
get() = (System.currentTimeMillis() - lastCfrShownTimeInMillis) > THREE_DAYS_MS
var syncedTabsInTabsTray by featureFlagPreference(
appContext.getPreferenceKey(R.string.pref_key_synced_tabs_tabs_tray),
default = false,
featureFlag = FeatureFlags.syncedTabsInTabsTray
)
var forceEnableZoom by booleanPreference(
appContext.getPreferenceKey(R.string.pref_key_accessibility_force_enable_zoom),
default = false

@ -213,8 +213,6 @@
<string name="pref_key_migrating_from_fenix_tip" translatable="false">pref_key_migrating_from_fenix_tip</string>
<string name="pref_key_master_password_tip" translatable="false">pref_key_master_password_tip</string>
<string name="pref_key_synced_tabs_tabs_tray" translatable="false">pref_key_synced_tabs_tabs_tray</string>
<string name="pref_key_debug_settings" translatable="false">pref_key_debug_settings</string>
<string name="pref_key_open_tabs_count" translatable="false">pref_key_open_tabs_count</string>

@ -4,9 +4,4 @@
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">
<SwitchPreference
android:defaultValue="false"
android:key="@string/pref_key_synced_tabs_tabs_tray"
android:title="@string/preferences_debug_synced_tabs_tabs_tray"
app:iconSpaceReserved="false" />
</PreferenceScreen>

@ -32,7 +32,6 @@ import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
@ -179,15 +178,6 @@ class DefaultTabTrayControllerTest {
}
}
@Test
fun onSyncedTabClicked() {
controller.handleSyncedTabClicked(mockk(relaxed = true))
verify {
activity.openToBrowserAndLoad(any(), true, BrowserDirection.FromTabTray)
}
}
@Test
fun handleBackPressed() {
every { tabTrayFragmentStore.state.mode } returns TabTrayDialogFragmentState.Mode.MultiSelect(

@ -1,140 +0,0 @@
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.After
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.components.metrics.MetricController
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()) } 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)
val metrics: MetricController = mockk()
controller =
SyncedTabsController(lifecycleOwner, view, store, concatAdapter, coroutineContext, metrics)
}
@After
fun cleanUp() {
testDispatcher.cleanupTestCoroutines()
}
@Test
fun `display synced tabs in reverse`() {
val tabs = listOf(
SyncedDeviceTabs(
device = mockk(relaxed = true),
tabs = listOf(
mockk(relaxed = true),
mockk(relaxed = true)
)
)
)
controller.displaySyncedTabs(tabs)
val itemCount = controller.adapter.itemCount
// title + device name + 2 tabs
assertEquals(4, itemCount)
assertEquals(
SyncedTabsViewHolder.TitleViewHolder.LAYOUT_ID,
controller.adapter.getItemViewType(itemCount - 1)
)
assertEquals(
SyncedTabsViewHolder.DeviceViewHolder.LAYOUT_ID,
controller.adapter.getItemViewType(itemCount - 2)
)
assertEquals(
SyncedTabsViewHolder.TabViewHolder.LAYOUT_ID,
controller.adapter.getItemViewType(itemCount - 3)
)
assertEquals(
SyncedTabsViewHolder.TabViewHolder.LAYOUT_ID,
controller.adapter.getItemViewType(itemCount - 4)
)
}
@Test
fun `show error when we go kaput`() {
controller.onError(ErrorType.SYNC_NEEDS_REAUTHENTICATION)
assertEquals(1, controller.adapter.itemCount)
assertEquals(
SyncedTabsViewHolder.ErrorViewHolder.LAYOUT_ID,
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()
// When returning from Multiselect the adapter should be added at the end
verify { concatAdapter.addAdapter(any()) }
}
}

@ -89,12 +89,6 @@ class TabTrayFragmentInteractorTest {
verify { controller.handleCloseAllTabsClicked(true) }
}
@Test
fun onSyncedTabClicked() {
interactor.onSyncedTabClicked(mockk(relaxed = true))
verify { controller.handleSyncedTabClicked(any()) }
}
@Test
fun onBackPressed() {
interactor.onBackPressed()

Loading…
Cancel
Save