From 3da6cfd98a724e4907c6df166011166a89c903dc Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Tue, 28 May 2019 18:05:16 -0400 Subject: [PATCH] For #2886: Fix sending multiple tabs to another device (#2923) * For #2886: Fix sending multiple tabs to another device * Update share methods to use new API --- .../org/mozilla/fenix/home/HomeFragment.kt | 25 ++++++----- .../library/bookmarks/BookmarkFragment.kt | 8 ++-- .../fenix/library/history/HistoryFragment.kt | 12 +++--- .../org/mozilla/fenix/share/ShareFragment.kt | 43 +++++++++++++------ app/src/main/res/navigation/nav_graph.xml | 9 +++- 5 files changed, 61 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 4df762b07..e68b456c5 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -70,6 +70,7 @@ import org.mozilla.fenix.mvi.getAutoDisposeObservable import org.mozilla.fenix.mvi.getManagedEmitter import org.mozilla.fenix.onboarding.FenixOnboarding import org.mozilla.fenix.settings.SupportUtils +import org.mozilla.fenix.share.ShareTab import org.mozilla.fenix.utils.ItsNotBrokenSnack import kotlin.coroutines.CoroutineContext import kotlin.math.roundToInt @@ -323,10 +324,9 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver { } is TabAction.Share -> { invokePendingDeleteJobs() - requireComponents.core.sessionManager.findSessionById(action.sessionId) - ?.let { session -> - share(session.url) - } + requireComponents.core.sessionManager.findSessionById(action.sessionId)?.let { session -> + share(session.url) + } } is TabAction.CloseAll -> { removeAllTabsWithUndo(action.private) @@ -346,10 +346,10 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver { } is TabAction.ShareTabs -> { invokePendingDeleteJobs() - val shareText = requireComponents.core.sessionManager.sessions.joinToString("\n") { - it.url + val shareTabs = requireComponents.core.sessionManager.sessions.map { + ShareTab(it.url, it.title, it.id) } - share(shareText) + share(tabs = shareTabs) } } } @@ -433,10 +433,8 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver { } } is CollectionAction.ShareTabs -> { - val shareText = action.collection.tabs.joinToString("\n") { - it.url - } - share(shareText) + val shareTabs = action.collection.tabs.map { ShareTab(it.url, it.title) } + share(tabs = shareTabs) } is CollectionAction.RemoveTab -> { launch(Dispatchers.IO) { @@ -664,8 +662,9 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver { } } - private fun share(text: String) { - val directions = HomeFragmentDirections.actionHomeFragmentToShareFragment(text) + private fun share(url: String? = null, tabs: List? = null) { + val directions = + HomeFragmentDirections.actionHomeFragmentToShareFragment(url = url, tabs = tabs?.toTypedArray()) Navigation.findNavController(view!!).navigate(directions) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index c64f76ac9..d9fa0df5a 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -223,10 +223,12 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve } is BookmarkAction.Share -> { it.item.url?.apply { - navigation - .navigate( - BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(this) + navigation.navigate( + BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment( + this, + it.item.title ) + ) requireComponents.analytics.metrics.track(Event.ShareBookmark) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index 49eb0ad97..75623daa8 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -41,6 +41,7 @@ import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.getAutoDisposeObservable import org.mozilla.fenix.mvi.getManagedEmitter +import org.mozilla.fenix.share.ShareTab import java.net.MalformedURLException import java.net.URL import kotlin.coroutines.CoroutineContext @@ -183,10 +184,8 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler { when { selectedHistory.size == 1 -> share(selectedHistory.first().url) selectedHistory.size > 1 -> { - val shareText = selectedHistory.joinToString("\n") { - it.url - } - share(shareText) + val shareTabs = selectedHistory.map { ShareTab(it.url, it.title) } + share(tabs = shareTabs) } } true @@ -283,8 +282,9 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler { } } - private fun share(text: String) { - val directions = HistoryFragmentDirections.actionHistoryFragmentToShareFragment(text) + private fun share(url: String? = null, tabs: List? = null) { + val directions = + HistoryFragmentDirections.actionHistoryFragmentToShareFragment(url = url, tabs = tabs?.toTypedArray()) Navigation.findNavController(view!!).navigate(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt index f3f929d57..4edeec7d1 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt @@ -9,6 +9,7 @@ import android.content.Intent.ACTION_SEND import android.content.Intent.EXTRA_TEXT import android.content.Intent.FLAG_ACTIVITY_NEW_TASK import android.os.Bundle +import android.os.Parcelable import android.view.ContextThemeWrapper import android.view.LayoutInflater import android.view.View @@ -16,11 +17,13 @@ import android.view.ViewGroup import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatDialogFragment import androidx.navigation.fragment.NavHostFragment.findNavController +import kotlinx.android.parcel.Parcelize import kotlinx.android.synthetic.main.fragment_share.view.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import mozilla.components.concept.sync.DeviceEventOutgoing +import mozilla.components.concept.sync.OAuthAccount import org.mozilla.fenix.FenixViewModelProvider import org.mozilla.fenix.R import org.mozilla.fenix.ext.requireComponents @@ -33,8 +36,7 @@ class ShareFragment : AppCompatDialogFragment(), CoroutineScope { get() = Dispatchers.Main + job private lateinit var job: Job private lateinit var component: ShareComponent - private lateinit var url: String - private lateinit var title: String + private var tabs: Array = emptyArray() override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -45,9 +47,13 @@ class ShareFragment : AppCompatDialogFragment(), CoroutineScope { val view = inflater.inflate(R.layout.fragment_share, container, false) val args = ShareFragmentArgs.fromBundle(arguments!!) + if (args.url == null && args.tabs.isNullOrEmpty()) { + throw IllegalStateException("URL and tabs cannot both be null.") + } + job = Job() - url = args.url - title = args.title ?: "" + tabs = args.tabs ?: arrayOf(ShareTab(args.url!!, args.title ?: "")) + component = ShareComponent( view.share_wrapper, ActionBusFactory.get(this), @@ -99,10 +105,7 @@ class ShareFragment : AppCompatDialogFragment(), CoroutineScope { is ShareAction.ShareDeviceClicked -> { val authAccount = requireComponents.backgroundServices.accountManager.authenticatedAccount() authAccount?.run { - deviceConstellation().sendEventToDeviceAsync( - it.device.id, - DeviceEventOutgoing.SendTab(title, url) - ) + sendSendTab(this, it.device.id, tabs) } dismiss() } @@ -110,17 +113,17 @@ class ShareFragment : AppCompatDialogFragment(), CoroutineScope { val authAccount = requireComponents.backgroundServices.accountManager.authenticatedAccount() authAccount?.run { it.devices.forEach { device -> - deviceConstellation().sendEventToDeviceAsync( - device.id, - DeviceEventOutgoing.SendTab(title, url) - ) + sendSendTab(this, device.id, tabs) } } dismiss() } is ShareAction.ShareAppClicked -> { + + val shareText = tabs.joinToString("\n") { tab -> tab.url } + val intent = Intent(ACTION_SEND).apply { - putExtra(EXTRA_TEXT, url) + putExtra(EXTRA_TEXT, shareText) type = "text/plain" flags = FLAG_ACTIVITY_NEW_TASK `package` = it.packageName @@ -131,4 +134,18 @@ class ShareFragment : AppCompatDialogFragment(), CoroutineScope { } } } + + private fun sendSendTab(account: OAuthAccount, deviceId: String, tabs: Array) { + account.run { + tabs.forEach { tab -> + deviceConstellation().sendEventToDeviceAsync( + deviceId, + DeviceEventOutgoing.SendTab(tab.title, tab.url) + ) + } + } + } } + +@Parcelize +data class ShareTab(val url: String, val title: String, val sessionId: String? = null) : Parcelable diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 2bc5334e1..90b5304c4 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -402,12 +402,19 @@ tools:layout="@layout/fragment_share" > + android:defaultValue="@null" + app:argType="string" + app:nullable="true" /> +