Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #12287: Add Synced Tabs to Tabs Tray #13893

Merged
merged 4 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/src/main/java/org/mozilla/fenix/BrowserDirection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ enum class BrowserDirection(@IdRes val fragmentId: Int) {
FromEditCustomSearchEngineFragment(R.id.editCustomSearchEngineFragment),
FromAddonDetailsFragment(R.id.addonDetailsFragment),
FromAddonPermissionsDetailsFragment(R.id.addonPermissionsDetailFragment),
FromLoginDetailFragment(R.id.loginDetailFragment)
FromLoginDetailFragment(R.id.loginDetailFragment),
FromTabTray(R.id.tabTrayDialogFragment)
}
11 changes: 4 additions & 7 deletions app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@ object FeatureFlags {
const val loginsEdit = true

/**
* Enable tab sync feature
* Shows Synced Tabs in the tabs tray.
*
* Tracking issue: https://github.com/mozilla-mobile/fenix/issues/13892
*/
const val syncedTabs = true

/**
* Enables new tab tray pref
*/
val tabTray = Config.channel.isNightlyOrDebug
val syncedTabsInTabsTray = Config.channel.isNightlyOrDebug

/**
* Enables gestures on the browser chrome that depend on a [SwipeGestureLayout]
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/org/mozilla/fenix/HomeActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import org.mozilla.fenix.settings.search.EditCustomSearchEngineFragmentDirection
import org.mozilla.fenix.share.AddNewDeviceFragmentDirections
import org.mozilla.fenix.sync.SyncedTabsFragmentDirections
import org.mozilla.fenix.tabtray.TabTrayDialogFragment
import org.mozilla.fenix.tabtray.TabTrayDialogFragmentDirections
import org.mozilla.fenix.theme.DefaultThemeManager
import org.mozilla.fenix.theme.ThemeManager
import org.mozilla.fenix.utils.BrowsersCache
Expand Down Expand Up @@ -597,6 +598,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
AddonPermissionsDetailsFragmentDirections.actionGlobalBrowser(customTabSessionId)
BrowserDirection.FromLoginDetailFragment ->
LoginDetailFragmentDirections.actionGlobalBrowser(customTabSessionId)
BrowserDirection.FromTabTray ->
TabTrayDialogFragmentDirections.actionGlobalBrowser(customTabSessionId)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider
import mozilla.components.service.sync.logins.SyncableLoginsStorage
import mozilla.components.support.utils.RunWhenReadyQueue
import org.mozilla.fenix.Config
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
Expand Down Expand Up @@ -85,11 +84,8 @@ class BackgroundServices(
)

@VisibleForTesting
val supportedEngines = if (FeatureFlags.syncedTabs) {
val supportedEngines =
setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords, SyncEngine.Tabs)
} else {
setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords)
}
private val syncConfig = SyncConfig(supportedEngines, syncPeriodInMinutes = 240L) // four hours

init {
Expand All @@ -98,10 +94,7 @@ class BackgroundServices(
GlobalSyncableStoreProvider.configureStore(SyncEngine.History to historyStorage)
GlobalSyncableStoreProvider.configureStore(SyncEngine.Bookmarks to bookmarkStorage)
GlobalSyncableStoreProvider.configureStore(SyncEngine.Passwords to passwordsStorage)

if (FeatureFlags.syncedTabs) {
GlobalSyncableStoreProvider.configureStore(SyncEngine.Tabs to remoteTabsStorage)
}
GlobalSyncableStoreProvider.configureStore(SyncEngine.Tabs to remoteTabsStorage)
}

private val telemetryAccountObserver = TelemetryAccountObserver(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.support.ktx.android.content.getColorFromAttr
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
Expand Down Expand Up @@ -177,11 +176,13 @@ class DefaultToolbarMenu(
?.browsingModeManager?.mode == BrowsingMode.Normal
val shouldDeleteDataOnQuit = context.components.settings
.shouldDeleteBrowsingDataOnQuit
val syncedTabsInTabsTray = context.components.settings
.syncedTabsInTabsTray

val menuItems = listOfNotNull(
historyItem,
bookmarksItem,
if (FeatureFlags.syncedTabs) syncedTabs else null,
if (syncedTabsInTabsTray) null else syncedTabs,
settings,
if (shouldDeleteDataOnQuit) deleteDataOnQuit else null,
BrowserMenuDivider(),
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.AuthType
import mozilla.components.concept.sync.OAuthAccount
import mozilla.components.support.ktx.android.content.getColorFromAttr
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings
Expand Down Expand Up @@ -158,7 +157,7 @@ class HomeMenu(
if (settings.shouldDeleteBrowsingDataOnQuit) quitItem else null,
settingsItem,
BrowserMenuDivider(),
if (FeatureFlags.syncedTabs) syncedTabsItem else null,
if (settings.syncedTabsInTabsTray) null else syncedTabsItem,
bookmarksItem,
historyItem,
BrowserMenuDivider(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,11 @@ class SecretSettingsFragment : PreferenceFragmentCompat() {
isChecked = context.settings().waitToShowPageUntilFirstPaint
onPreferenceChangeListener = SharedPreferenceUpdater()
}

requirePreference<SwitchPreference>(R.string.pref_key_synced_tabs_tabs_tray).apply {
isVisible = FeatureFlags.syncedTabsInTabsTray
isChecked = context.settings().syncedTabsInTabsTray
onPreferenceChangeListener = SharedPreferenceUpdater()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import mozilla.components.service.fxa.sync.SyncStatusObserver
import mozilla.components.service.fxa.sync.getLastSynced
import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.ktx.android.util.dpToPx
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.StoreProvider
Expand Down Expand Up @@ -271,9 +270,8 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
isChecked = syncEnginesStatus.getOrElse(SyncEngine.Passwords) { true }
}
requirePreference<CheckBoxPreference>(R.string.pref_key_sync_tabs).apply {
isVisible = FeatureFlags.syncedTabs
isEnabled = syncEnginesStatus.containsKey(SyncEngine.Tabs)
isChecked = syncEnginesStatus.getOrElse(SyncEngine.Tabs) { FeatureFlags.syncedTabs }
isChecked = syncEnginesStatus.getOrElse(SyncEngine.Tabs) { true }
}
}

Expand Down
54 changes: 42 additions & 12 deletions app/src/main/java/org/mozilla/fenix/sync/SyncedTabsAdapter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@ import androidx.navigation.NavController
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter
import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.feature.syncedtabs.view.SyncedTabsView
import org.mozilla.fenix.sync.SyncedTabsViewHolder.DeviceViewHolder
import org.mozilla.fenix.sync.SyncedTabsViewHolder.ErrorViewHolder
import org.mozilla.fenix.sync.SyncedTabsViewHolder.NoTabsViewHolder
import org.mozilla.fenix.sync.SyncedTabsViewHolder.TabViewHolder
import org.mozilla.fenix.sync.SyncedTabsViewHolder.TitleViewHolder
import org.mozilla.fenix.sync.ext.toAdapterList
import mozilla.components.browser.storage.sync.Tab as SyncTab
import mozilla.components.concept.sync.Device as SyncDevice

class SyncedTabsAdapter(
private val listener: (SyncTab) -> Unit
private val newListener: SyncedTabsView.Listener
) : ListAdapter<SyncedTabsAdapter.AdapterItem, SyncedTabsViewHolder>(DiffCallback) {

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): SyncedTabsViewHolder {
Expand All @@ -27,30 +31,26 @@ class SyncedTabsAdapter(
DeviceViewHolder.LAYOUT_ID -> DeviceViewHolder(itemView)
TabViewHolder.LAYOUT_ID -> TabViewHolder(itemView)
ErrorViewHolder.LAYOUT_ID -> ErrorViewHolder(itemView)
TitleViewHolder.LAYOUT_ID -> TitleViewHolder(itemView)
NoTabsViewHolder.LAYOUT_ID -> NoTabsViewHolder(itemView)
else -> throw IllegalStateException()
}
}

override fun onBindViewHolder(holder: SyncedTabsViewHolder, position: Int) {
holder.bind(getItem(position), listener)
holder.bind(getItem(position), newListener)
}

override fun getItemViewType(position: Int) = when (getItem(position)) {
is AdapterItem.Device -> DeviceViewHolder.LAYOUT_ID
is AdapterItem.Tab -> TabViewHolder.LAYOUT_ID
is AdapterItem.Error -> ErrorViewHolder.LAYOUT_ID
is AdapterItem.Title -> TitleViewHolder.LAYOUT_ID
is AdapterItem.NoTabs -> NoTabsViewHolder.LAYOUT_ID
}

fun updateData(syncedTabs: List<SyncedDeviceTabs>) {
val allDeviceTabs = mutableListOf<AdapterItem>()

syncedTabs.forEach { (device, tabs) ->
if (tabs.isNotEmpty()) {
allDeviceTabs.add(AdapterItem.Device(device))
tabs.mapTo(allDeviceTabs) { AdapterItem.Tab(it) }
}
}

val allDeviceTabs = syncedTabs.toAdapterList()
submitList(allDeviceTabs)
}

Expand All @@ -59,7 +59,11 @@ class SyncedTabsAdapter(
when (oldItem) {
is AdapterItem.Device ->
newItem is AdapterItem.Device && oldItem.device.id == newItem.device.id
is AdapterItem.Tab, is AdapterItem.Error ->
is AdapterItem.NoTabs ->
newItem is AdapterItem.NoTabs && oldItem.device.id == newItem.device.id
is AdapterItem.Tab,
is AdapterItem.Error,
is AdapterItem.Title ->
oldItem == newItem
}

Expand All @@ -68,9 +72,35 @@ class SyncedTabsAdapter(
oldItem == newItem
}

/**
* The various types of adapter items that can be found in a [SyncedTabsAdapter].
*/
sealed class AdapterItem {

/**
* A title header of the Synced Tabs UI that has a refresh button in it. This may be seen
* only in some views depending on where the Synced Tabs UI is displayed.
*/
object Title : AdapterItem()

/**
* A device header for displaying a synced device.
*/
data class Device(val device: SyncDevice) : AdapterItem()

/**
* A tab that was synced.
*/
data class Tab(val tab: SyncTab) : AdapterItem()

/**
* A placeholder for a device that has no tabs synced.
*/
data class NoTabs(val device: SyncDevice) : AdapterItem()

/**
* A message displayed if an error was encountered.
*/
data class Error(
val descriptionResId: Int,
val navController: NavController? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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
Expand All @@ -22,6 +23,11 @@ import org.mozilla.fenix.library.LibraryPageFragment
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?,
Expand Down
48 changes: 24 additions & 24 deletions app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package org.mozilla.fenix.sync
import android.content.Context
import android.util.AttributeSet
import android.widget.FrameLayout
import androidx.annotation.StringRes
import androidx.fragment.app.findFragment
import androidx.navigation.NavController
import androidx.navigation.fragment.findNavController
Expand All @@ -18,8 +17,12 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.cancel
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.sync.ext.toAdapterItem
import org.mozilla.fenix.sync.ext.toStringRes
import java.lang.IllegalStateException

class SyncedTabsLayout @JvmOverloads constructor(
Expand All @@ -30,7 +33,7 @@ class SyncedTabsLayout @JvmOverloads constructor(

override var listener: SyncedTabsView.Listener? = null

private val adapter = SyncedTabsAdapter { listener?.onTabClicked(it) }
private val adapter = SyncedTabsAdapter(ListenerDelegate { listener })
private val coroutineScope = CoroutineScope(Dispatchers.Main)

init {
Expand All @@ -40,6 +43,9 @@ 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) {
Expand All @@ -53,8 +59,8 @@ class SyncedTabsLayout @JvmOverloads constructor(
null
}

val descriptionResId = stringResourceForError(error)
val errorItem = getErrorItem(navController, error, descriptionResId)
val descriptionResId = error.toStringRes()
val errorItem = error.toAdapterItem(descriptionResId, navController)

val errorList: List<SyncedTabsAdapter.AdapterItem> = listOf(errorItem)
adapter.submitList(errorList)
Expand Down Expand Up @@ -96,27 +102,21 @@ class SyncedTabsLayout @JvmOverloads constructor(
SyncedTabsView.ErrorType.MULTIPLE_DEVICES_UNAVAILABLE,
SyncedTabsView.ErrorType.NO_TABS_AVAILABLE -> true
}
}
}

internal fun stringResourceForError(error: SyncedTabsView.ErrorType) = when (error) {
SyncedTabsView.ErrorType.MULTIPLE_DEVICES_UNAVAILABLE -> R.string.synced_tabs_connect_another_device
SyncedTabsView.ErrorType.SYNC_ENGINE_UNAVAILABLE -> R.string.synced_tabs_enable_tab_syncing
SyncedTabsView.ErrorType.SYNC_UNAVAILABLE -> R.string.synced_tabs_sign_in_message
SyncedTabsView.ErrorType.SYNC_NEEDS_REAUTHENTICATION -> R.string.synced_tabs_reauth
SyncedTabsView.ErrorType.NO_TABS_AVAILABLE -> R.string.synced_tabs_no_tabs
}
/**
* We have to do this weird daisy-chaining of callbacks because the listener is nullable and
* when we get a null reference, we never get a new binding to the non-null listener.
*/
class ListenerDelegate(
private val listener: (() -> SyncedTabsView.Listener?)
) : SyncedTabsView.Listener {
override fun onRefresh() {
listener.invoke()?.onRefresh()
}

internal fun getErrorItem(
navController: NavController?,
error: SyncedTabsView.ErrorType,
@StringRes stringResId: Int
): SyncedTabsAdapter.AdapterItem = when (error) {
SyncedTabsView.ErrorType.MULTIPLE_DEVICES_UNAVAILABLE,
SyncedTabsView.ErrorType.SYNC_ENGINE_UNAVAILABLE,
SyncedTabsView.ErrorType.SYNC_NEEDS_REAUTHENTICATION,
SyncedTabsView.ErrorType.NO_TABS_AVAILABLE -> SyncedTabsAdapter.AdapterItem
.Error(descriptionResId = stringResId)
SyncedTabsView.ErrorType.SYNC_UNAVAILABLE -> SyncedTabsAdapter.AdapterItem
.Error(descriptionResId = stringResId, navController = navController)
}
override fun onTabClicked(tab: Tab) {
listener.invoke()?.onTabClicked(tab)
}
}
Loading