1
0
Fork 0

For #12857 - Use Collection title when sharing tabs collection

Avoided passing the subject for sharing a collection of tabs in the ShareData
object since ShareData is part of a web standard.
master
Mugurell 2020-08-11 18:20:35 +03:00
parent c9e26a3f88
commit b993b94be1
6 changed files with 43 additions and 9 deletions

View File

@ -232,7 +232,10 @@ class DefaultSessionControlController(
} }
override fun handleCollectionShareTabsClicked(collection: TabCollection) { override fun handleCollectionShareTabsClicked(collection: TabCollection) {
showShareFragment(collection.tabs.map { ShareData(url = it.url, title = it.title) }) showShareFragment(
collection.title,
collection.tabs.map { ShareData(url = it.url, title = it.title) }
)
metrics.track(Event.CollectionShared) metrics.track(Event.CollectionShared)
} }
@ -366,8 +369,9 @@ class DefaultSessionControlController(
showTabTrayCollectionCreation() showTabTrayCollectionCreation()
} }
private fun showShareFragment(data: List<ShareData>) { private fun showShareFragment(shareSubject: String, data: List<ShareData>) {
val directions = HomeFragmentDirections.actionGlobalShareFragment( val directions = HomeFragmentDirections.actionGlobalShareFragment(
shareSubject = shareSubject,
data = data.toTypedArray() data = data.toTypedArray()
) )
navController.nav(R.id.homeFragment, directions) navController.nav(R.id.homeFragment, directions)

View File

@ -55,6 +55,7 @@ interface ShareController {
* Default behavior of [ShareController]. Other implementations are possible. * Default behavior of [ShareController]. Other implementations are possible.
* *
* @param context [Context] used for various Android interactions. * @param context [Context] used for various Android interactions.
* @param shareSubject desired message subject used when sharing through 3rd party apps, like email clients.
* @param shareData the list of [ShareData]s that can be shared. * @param shareData the list of [ShareData]s that can be shared.
* @param sendTabUseCases instance of [SendTabUseCases] which allows sending tabs to account devices. * @param sendTabUseCases instance of [SendTabUseCases] which allows sending tabs to account devices.
* @param snackbar - instance of [FenixSnackbar] for displaying styled snackbars * @param snackbar - instance of [FenixSnackbar] for displaying styled snackbars
@ -64,6 +65,7 @@ interface ShareController {
@Suppress("TooManyFunctions") @Suppress("TooManyFunctions")
class DefaultShareController( class DefaultShareController(
private val context: Context, private val context: Context,
private val shareSubject: String?,
private val shareData: List<ShareData>, private val shareData: List<ShareData>,
private val sendTabUseCases: SendTabUseCases, private val sendTabUseCases: SendTabUseCases,
private val snackbar: FenixSnackbar, private val snackbar: FenixSnackbar,
@ -90,7 +92,7 @@ class DefaultShareController(
val intent = Intent(ACTION_SEND).apply { val intent = Intent(ACTION_SEND).apply {
putExtra(EXTRA_TEXT, getShareText()) putExtra(EXTRA_TEXT, getShareText())
putExtra(EXTRA_SUBJECT, shareData.map { it.title }.joinToString(", ")) putExtra(EXTRA_SUBJECT, getShareSubject())
type = "text/plain" type = "text/plain"
flags = FLAG_ACTIVITY_NEW_TASK flags = FLAG_ACTIVITY_NEW_TASK
setClassName(app.packageName, app.activityName) setClassName(app.packageName, app.activityName)
@ -189,6 +191,9 @@ class DefaultShareController(
} }
} }
@VisibleForTesting
internal fun getShareSubject() = shareSubject ?: shareData.map { it.title }.joinToString(", ")
// Navigation between app fragments uses ShareTab as arguments. SendTabUseCases uses TabData. // Navigation between app fragments uses ShareTab as arguments. SendTabUseCases uses TabData.
@VisibleForTesting @VisibleForTesting
internal fun List<ShareData>.toTabData() = map { data -> internal fun List<ShareData>.toTabData() = map { data ->

View File

@ -67,6 +67,7 @@ class ShareFragment : AppCompatDialogFragment() {
shareInteractor = ShareInteractor( shareInteractor = ShareInteractor(
DefaultShareController( DefaultShareController(
context = requireContext(), context = requireContext(),
shareSubject = args.shareSubject,
shareData = shareData, shareData = shareData,
snackbar = FenixSnackbar.make( snackbar = FenixSnackbar.make(
view = requireActivity().getRootView()!!, view = requireActivity().getRootView()!!,

View File

@ -677,6 +677,11 @@
android:defaultValue="null" android:defaultValue="null"
app:argType="string" app:argType="string"
app:nullable="true" /> app:nullable="true" />
<argument
android:name="shareSubject"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
</dialog> </dialog>
<dialog <dialog
android:id="@+id/quickSettingsSheetDialogFragment" android:id="@+id/quickSettingsSheetDialogFragment"

View File

@ -212,6 +212,7 @@ class DefaultSessionControlControllerTest {
fun handleCollectionShareTabsClicked() { fun handleCollectionShareTabsClicked() {
val collection = mockk<TabCollection> { val collection = mockk<TabCollection> {
every { tabs } returns emptyList() every { tabs } returns emptyList()
every { title } returns ""
} }
controller.handleCollectionShareTabsClicked(collection) controller.handleCollectionShareTabsClicked(collection)

View File

@ -48,6 +48,7 @@ class ShareControllerTest {
// Need a valid context to retrieve Strings for example, but we also need it to return our "metrics" // Need a valid context to retrieve Strings for example, but we also need it to return our "metrics"
private val context: Context = spyk(testContext) private val context: Context = spyk(testContext)
private val metrics: MetricController = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true)
private val shareSubject = "shareSubject"
private val shareData = listOf( private val shareData = listOf(
ShareData(url = "url0", title = "title0"), ShareData(url = "url0", title = "title0"),
ShareData(url = "url1", title = "title1") ShareData(url = "url1", title = "title1")
@ -65,7 +66,7 @@ class ShareControllerTest {
private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true) private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true)
private val recentAppStorage = mockk<RecentAppsStorage>(relaxed = true) private val recentAppStorage = mockk<RecentAppsStorage>(relaxed = true)
private val controller = DefaultShareController( private val controller = DefaultShareController(
context, shareData, sendTabUseCases, snackbar, navController, context, shareSubject, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss recentAppStorage, testCoroutineScope, dismiss
) )
@ -91,8 +92,8 @@ class ShareControllerTest {
// needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we
// need to use an Activity Context. // need to use an Activity Context.
val activityContext: Context = mockk<Activity>() val activityContext: Context = mockk<Activity>()
val testController = DefaultShareController(activityContext, shareData, mockk(), mockk(), mockk(), val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(),
recentAppStorage, testCoroutineScope, dismiss) mockk(), mockk(), recentAppStorage, testCoroutineScope, dismiss)
every { activityContext.startActivity(capture(shareIntent)) } just Runs every { activityContext.startActivity(capture(shareIntent)) } just Runs
every { recentAppStorage.updateRecentApp(appShareOption.activityName) } just Runs every { recentAppStorage.updateRecentApp(appShareOption.activityName) } just Runs
@ -101,6 +102,7 @@ class ShareControllerTest {
// Check that the Intent used for querying apps has the expected structure // Check that the Intent used for querying apps has the expected structure
assertTrue(shareIntent.isCaptured) assertTrue(shareIntent.isCaptured)
assertEquals(Intent.ACTION_SEND, shareIntent.captured.action) assertEquals(Intent.ACTION_SEND, shareIntent.captured.action)
assertEquals(shareSubject, shareIntent.captured.extras!![Intent.EXTRA_SUBJECT])
assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT]) assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT])
assertEquals("text/plain", shareIntent.captured.type) assertEquals("text/plain", shareIntent.captured.type)
assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags) assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags)
@ -124,8 +126,8 @@ class ShareControllerTest {
// needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we
// need to use an Activity Context. // need to use an Activity Context.
val activityContext: Context = mockk<Activity>() val activityContext: Context = mockk<Activity>()
val testController = DefaultShareController(activityContext, shareData, mockk(), snackbar, val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(),
mockk(), mockk(), testCoroutineScope, dismiss) snackbar, mockk(), mockk(), testCoroutineScope, dismiss)
every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException() every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException()
every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app" every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app"
@ -247,6 +249,7 @@ class ShareControllerTest {
fun `getSuccessMessage should return different strings depending on the number of shared tabs`() { fun `getSuccessMessage should return different strings depending on the number of shared tabs`() {
val controllerWithOneSharedTab = DefaultShareController( val controllerWithOneSharedTab = DefaultShareController(
context, context,
shareSubject,
listOf(ShareData(url = "url0", title = "title0")), listOf(ShareData(url = "url0", title = "title0")),
mockk(), mockk(),
mockk(), mockk(),
@ -280,7 +283,7 @@ class ShareControllerTest {
ShareData(url = "url1") ShareData(url = "url1")
) )
val controller = DefaultShareController( val controller = DefaultShareController(
context, shareData, sendTabUseCases, snackbar, navController, context, shareSubject, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss recentAppStorage, testCoroutineScope, dismiss
) )
@ -288,6 +291,21 @@ class ShareControllerTest {
assertEquals(expectedShareText, controller.getShareText()) assertEquals(expectedShareText, controller.getShareText())
} }
@Test
fun `getShareSubject will return "shareSubject" if that is non null`() {
assertEquals(shareSubject, controller.getShareSubject())
}
@Test
fun `getShareSubject will return a concatenation of tab titles if "shareSubject" is null`() {
val controller = DefaultShareController(
context, null, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss
)
assertEquals("title0, title1", controller.getShareSubject())
}
@Test @Test
fun `ShareTab#toTabData maps a list of ShareTab to a TabData list`() { fun `ShareTab#toTabData maps a list of ShareTab to a TabData list`() {
var tabData: List<TabData> var tabData: List<TabData>