From 333ff8c9418727223fe7c2d96ea22775ec55888b Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Mon, 4 Nov 2019 18:33:02 -0800 Subject: [PATCH] Fixes #4528 - Prevent share menu from jumping Plus a bunch of docs and refactoring --- .../fenix/share/AddNewDeviceFragment.kt | 3 + .../org/mozilla/fenix/share/ShareCloseView.kt | 3 +- .../mozilla/fenix/share/ShareController.kt | 13 +- .../org/mozilla/fenix/share/ShareFragment.kt | 171 +++++++++--------- .../mozilla/fenix/share/ShareInteractor.kt | 4 +- .../fenix/share/ShareToAccountDevicesView.kt | 13 +- .../mozilla/fenix/share/ShareToAppsView.kt | 6 +- .../listadapters/AccountDevicesAdapter.kt | 38 ++-- .../share/listadapters/AppShareAdapter.kt | 51 ++++-- .../share/listadapters/ShareTabsAdapter.kt | 31 +--- .../viewholders/AccountDeviceViewHolder.kt | 58 +++--- .../fenix/share/viewholders/AppViewHolder.kt | 29 ++- .../res/layout/fragment_add_new_device.xml | 1 + .../fenix/share/ShareControllerTest.kt | 4 +- .../fenix/share/ShareInteractorTest.kt | 4 +- .../AccountDevicesShareAdapterTest.kt | 30 +-- .../share/listadapters/AppShareAdapterTest.kt | 9 +- 17 files changed, 250 insertions(+), 218 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/share/AddNewDeviceFragment.kt b/app/src/main/java/org/mozilla/fenix/share/AddNewDeviceFragment.kt index bb1e47c90..b3bd795af 100644 --- a/app/src/main/java/org/mozilla/fenix/share/AddNewDeviceFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/AddNewDeviceFragment.kt @@ -13,6 +13,9 @@ import kotlinx.android.synthetic.main.fragment_add_new_device.* import org.mozilla.fenix.R import org.mozilla.fenix.settings.SupportUtils +/** + * Fragment to add a new device. Tabs can be shared to devices after they are added. + */ class AddNewDeviceFragment : Fragment(R.layout.fragment_add_new_device) { override fun onResume() { diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareCloseView.kt b/app/src/main/java/org/mozilla/fenix/share/ShareCloseView.kt index b12ff4fbb..88d1ab1b2 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareCloseView.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareCloseView.kt @@ -23,6 +23,7 @@ class ShareCloseView( override val containerView: ViewGroup, private val interactor: ShareCloseInteractor ) : LayoutContainer { + val adapter = ShareTabsAdapter() init { @@ -36,6 +37,6 @@ class ShareCloseView( } fun setTabs(tabs: List) { - adapter.setTabs(tabs) + adapter.submitList(tabs) } } diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt index 5aec7401c..fa367707e 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -26,7 +26,7 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav -import org.mozilla.fenix.share.listadapters.AppShareOption +import org.mozilla.fenix.share.listadapters.AndroidShareOption /** * [ShareFragment] controller. @@ -36,7 +36,7 @@ import org.mozilla.fenix.share.listadapters.AppShareOption interface ShareController { fun handleReauth() fun handleShareClosed() - fun handleShareToApp(app: AppShareOption) + fun handleShareToApp(app: AndroidShareOption.App) fun handleAddNewDevice() fun handleShareToDevice(device: Device) fun handleShareToAllDevices(devices: List) @@ -72,7 +72,7 @@ class DefaultShareController( dismiss() } - override fun handleShareToApp(app: AppShareOption) { + override fun handleShareToApp(app: AndroidShareOption.App) { val intent = Intent(ACTION_SEND).apply { putExtra(EXTRA_TEXT, getShareText()) type = "text/plain" @@ -116,9 +116,10 @@ class DefaultShareController( private fun shareToDevicesWithRetry(shareOperation: () -> Deferred) { // Use GlobalScope to allow the continuation of this method even if the share fragment is closed. GlobalScope.launch(Dispatchers.Main) { - when (shareOperation.invoke().await()) { - true -> showSuccess() - false -> showFailureWithRetryOption { shareToDevicesWithRetry(shareOperation) } + if (shareOperation.invoke().await()) { + showSuccess() + } else { + showFailureWithRetryOption { shareToDevicesWithRetry(shareOperation) } } dismiss() } 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 752e8d086..90c645a8e 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt @@ -17,17 +17,19 @@ import android.os.Parcelable import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.annotation.WorkerThread import androidx.appcompat.app.AppCompatDialogFragment +import androidx.core.content.getSystemService import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController +import androidx.navigation.fragment.navArgs import kotlinx.android.parcel.Parcelize import kotlinx.android.synthetic.main.fragment_share.view.* import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.async import kotlinx.coroutines.launch import mozilla.components.concept.sync.DeviceCapability -import mozilla.components.concept.sync.DeviceType import mozilla.components.feature.sendtab.SendTabUseCases import mozilla.components.service.fxa.manager.FxaAccountManager import org.mozilla.fenix.R @@ -35,7 +37,7 @@ import org.mozilla.fenix.components.FenixSnackbarPresenter import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.requireComponents -import org.mozilla.fenix.share.listadapters.AppShareOption +import org.mozilla.fenix.share.listadapters.AndroidShareOption import org.mozilla.fenix.share.listadapters.SyncShareOption @Suppress("TooManyFunctions") @@ -44,20 +46,39 @@ class ShareFragment : AppCompatDialogFragment() { private lateinit var shareCloseView: ShareCloseView private lateinit var shareToAccountDevicesView: ShareToAccountDevicesView private lateinit var shareToAppsView: ShareToAppsView - private lateinit var appsListDeferred: Deferred> + private lateinit var appsListDeferred: Deferred> private lateinit var devicesListDeferred: Deferred> private var connectivityManager: ConnectivityManager? = null + private val networkCallback = object : ConnectivityManager.NetworkCallback() { + override fun onLost(network: Network?) = reloadDevices() + override fun onAvailable(network: Network?) = reloadDevices() + + private fun reloadDevices() { + context?.let { context -> + val fxaAccountManager = context.components.backgroundServices.accountManager + lifecycleScope.launch { + fxaAccountManager.authenticatedAccount() + ?.deviceConstellation() + ?.refreshDevicesAsync() + ?.await() + + val devicesShareOptions = buildDeviceList(fxaAccountManager) + shareToAccountDevicesView.setShareTargets(devicesShareOptions) + } + } + } + } + override fun onAttach(context: Context) { super.onAttach(context) - connectivityManager = - context.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager + connectivityManager = context.getSystemService() val networkRequest = NetworkRequest.Builder().build() connectivityManager?.registerNetworkCallback(networkRequest, networkCallback) // Start preparing the data as soon as we have a valid Context - appsListDeferred = lifecycleScope.async(Dispatchers.IO) { + appsListDeferred = lifecycleScope.async(IO) { val shareIntent = Intent(ACTION_SEND).apply { type = "text/plain" flags = FLAG_ACTIVITY_NEW_TASK @@ -66,53 +87,29 @@ class ShareFragment : AppCompatDialogFragment() { buildAppsList(shareAppsActivities, context) } - devicesListDeferred = lifecycleScope.async(Dispatchers.IO) { + devicesListDeferred = lifecycleScope.async(IO) { val fxaAccountManager = context.components.backgroundServices.accountManager buildDeviceList(fxaAccountManager) } } - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - setStyle(STYLE_NO_TITLE, R.style.ShareDialogStyle) - } - - private val networkCallback = object : ConnectivityManager.NetworkCallback() { - override fun onLost(network: Network?) { - reloadDevices() - } - - override fun onAvailable(network: Network?) { - reloadDevices() - } - } - - private fun reloadDevices() { - context?.let { - val fxaAccountManager = it.components.backgroundServices.accountManager - lifecycleScope.launch { - val refreshDevicesAsync = - fxaAccountManager.authenticatedAccount()?.deviceConstellation() - ?.refreshDevicesAsync() - refreshDevicesAsync?.await() - val devicesShareOptions = buildDeviceList(fxaAccountManager) - shareToAccountDevicesView.setSharetargets(devicesShareOptions) - } - } - } - override fun onDetach() { connectivityManager?.unregisterNetworkCallback(networkCallback) super.onDetach() } + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setStyle(STYLE_NO_TITLE, R.style.ShareDialogStyle) + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View? { val view = inflater.inflate(R.layout.fragment_share, container, false) - val args = ShareFragmentArgs.fromBundle(arguments!!) + val args by navArgs() check(!(args.url == null && args.tabs.isNullOrEmpty())) { "URL and tabs cannot both be null." } val tabs = args.tabs?.toList() ?: listOf(ShareTab(args.url!!, args.title.orEmpty())) @@ -153,74 +150,84 @@ class ShareFragment : AppCompatDialogFragment() { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + // Start with some invisible views so the share menu height doesn't jump later + shareToAppsView.setShareTargets( + listOf(AndroidShareOption.Invisible, AndroidShareOption.Invisible) + ) + lifecycleScope.launch { val devicesShareOptions = devicesListDeferred.await() - shareToAccountDevicesView.setSharetargets(devicesShareOptions) + shareToAccountDevicesView.setShareTargets(devicesShareOptions) val appsToShareTo = appsListDeferred.await() shareToAppsView.setShareTargets(appsToShareTo) } } + @WorkerThread private fun getIntentActivities(shareIntent: Intent, context: Context): List? { return context.packageManager.queryIntentActivities(shareIntent, 0) } + /** + * Returns a list of apps that can be shared to. + * @param intentActivities List of activities from [getIntentActivities]. + */ + @WorkerThread private fun buildAppsList( intentActivities: List?, context: Context - ): List { - return intentActivities?.map { resolveInfo -> - AppShareOption( - resolveInfo.loadLabel(context.packageManager).toString(), - resolveInfo.loadIcon(context.packageManager), - resolveInfo.activityInfo.packageName, - resolveInfo.activityInfo.name - ) - }?.filter { it.packageName != context.packageName }.orEmpty() + ): List { + return intentActivities + .orEmpty() + .filter { it.activityInfo.packageName != context.packageName } + .map { resolveInfo -> + AndroidShareOption.App( + resolveInfo.loadLabel(context.packageManager).toString(), + resolveInfo.loadIcon(context.packageManager), + resolveInfo.activityInfo.packageName, + resolveInfo.activityInfo.name + ) + } } - @Suppress("ReturnCount") + /** + * Builds list of options to display in the top row of the share sheet. + * This will primarily include devices that tabs can be sent to, but also options + * for reconnecting the account or sending to all devices. + */ private fun buildDeviceList(accountManager: FxaAccountManager): List { - val list = mutableListOf() - val activeNetwork = connectivityManager?.activeNetworkInfo - if (activeNetwork?.isConnected != true) { - list.add(SyncShareOption.Offline) - return list - } + val account = accountManager.authenticatedAccount() - if (accountManager.authenticatedAccount() == null) { - list.add(SyncShareOption.SignIn) - return list - } + return when { + // No network + activeNetwork?.isConnected != true -> listOf(SyncShareOption.Offline) + // No account signed in + account == null -> listOf(SyncShareOption.SignIn) + // Account needs to be re-authenticated + accountManager.accountNeedsReauth() -> listOf(SyncShareOption.Reconnect) + // Signed in + else -> { + val shareableDevices = account.deviceConstellation().state() + ?.otherDevices + .orEmpty() + .filter { it.capabilities.contains(DeviceCapability.SEND_TAB) } - if (accountManager.accountNeedsReauth()) { - list.add(SyncShareOption.Reconnect) - return list - } - - accountManager.authenticatedAccount()?.deviceConstellation()?.state() - ?.otherDevices?.let { devices -> - val shareableDevices = - devices.filter { it.capabilities.contains(DeviceCapability.SEND_TAB) } - - if (shareableDevices.isEmpty()) { - list.add(SyncShareOption.AddNewDevice) - } - - val shareOptions = shareableDevices.map { - when (it.deviceType) { - DeviceType.MOBILE -> SyncShareOption.Mobile(it.displayName, it) - else -> SyncShareOption.Desktop(it.displayName, it) + val list = mutableListOf() + if (shareableDevices.isEmpty()) { + // Show add device button if there are no devices + list.add(SyncShareOption.AddNewDevice) } - } - list.addAll(shareOptions) - if (shareableDevices.size > 1) { - list.add(SyncShareOption.SendAll(shareableDevices)) + shareableDevices.mapTo(list) { SyncShareOption.SingleDevice(it) } + + if (shareableDevices.size > 1) { + // Show send all button if there are multiple devices + list.add(SyncShareOption.SendAll(shareableDevices)) + } + list } } - return list } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareInteractor.kt b/app/src/main/java/org/mozilla/fenix/share/ShareInteractor.kt index 64a45831c..b2b8c01da 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareInteractor.kt @@ -5,7 +5,7 @@ package org.mozilla.fenix.share import mozilla.components.concept.sync.Device -import org.mozilla.fenix.share.listadapters.AppShareOption +import org.mozilla.fenix.share.listadapters.AndroidShareOption /** * Interactor for the share screen. @@ -37,7 +37,7 @@ class ShareInteractor( controller.handleShareToAllDevices(devices) } - override fun onShareToApp(appToShareTo: AppShareOption) { + override fun onShareToApp(appToShareTo: AndroidShareOption.App) { controller.handleShareToApp(appToShareTo) } } diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareToAccountDevicesView.kt b/app/src/main/java/org/mozilla/fenix/share/ShareToAccountDevicesView.kt index 151e095e4..acf2120ee 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareToAccountDevicesView.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareToAccountDevicesView.kt @@ -26,18 +26,19 @@ interface ShareToAccountDevicesInteractor { class ShareToAccountDevicesView( override val containerView: ViewGroup, - private val interactor: ShareToAccountDevicesInteractor + interactor: ShareToAccountDevicesInteractor ) : LayoutContainer { + + private val adapter = AccountDevicesShareAdapter(interactor) + init { LayoutInflater.from(containerView.context) .inflate(R.layout.share_to_account_devices, containerView, true) - devicesList.adapter = AccountDevicesShareAdapter(interactor) + devicesList.adapter = adapter } - fun setSharetargets(targets: List) { - with(devicesList.adapter as AccountDevicesShareAdapter) { - updateData(targets) - } + fun setShareTargets(targets: List) { + adapter.submitList(targets) } } diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareToAppsView.kt b/app/src/main/java/org/mozilla/fenix/share/ShareToAppsView.kt index 7a15297fd..b0f8b976f 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareToAppsView.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareToAppsView.kt @@ -10,14 +10,14 @@ import android.view.ViewGroup import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.share_to_apps.* import org.mozilla.fenix.R +import org.mozilla.fenix.share.listadapters.AndroidShareOption import org.mozilla.fenix.share.listadapters.AppShareAdapter -import org.mozilla.fenix.share.listadapters.AppShareOption /** * Callbacks for possible user interactions on the [ShareCloseView] */ interface ShareToAppsInteractor { - fun onShareToApp(appToShareTo: AppShareOption) + fun onShareToApp(appToShareTo: AndroidShareOption.App) } class ShareToAppsView( @@ -34,7 +34,7 @@ class ShareToAppsView( appsList.adapter = adapter } - fun setShareTargets(targets: List) { + fun setShareTargets(targets: List) { progressBar.visibility = View.GONE appsList.visibility = View.VISIBLE diff --git a/app/src/main/java/org/mozilla/fenix/share/listadapters/AccountDevicesAdapter.kt b/app/src/main/java/org/mozilla/fenix/share/listadapters/AccountDevicesAdapter.kt index d779369a4..c0233afc4 100644 --- a/app/src/main/java/org/mozilla/fenix/share/listadapters/AccountDevicesAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/share/listadapters/AccountDevicesAdapter.kt @@ -6,15 +6,19 @@ package org.mozilla.fenix.share.listadapters import android.view.LayoutInflater import android.view.ViewGroup -import androidx.recyclerview.widget.RecyclerView +import androidx.recyclerview.widget.DiffUtil +import androidx.recyclerview.widget.ListAdapter import mozilla.components.concept.sync.Device import org.mozilla.fenix.share.ShareToAccountDevicesInteractor import org.mozilla.fenix.share.viewholders.AccountDeviceViewHolder +/** + * Adapter for a list of devices that can be shared to. + * May also display buttons to reconnect, add a device, or send to all devices. + */ class AccountDevicesShareAdapter( - private val interactor: ShareToAccountDevicesInteractor, - private val devices: MutableList = mutableListOf() -) : RecyclerView.Adapter() { + private val interactor: ShareToAccountDevicesInteractor +) : ListAdapter(DiffCallback) { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AccountDeviceViewHolder { val view = LayoutInflater.from(parent.context) @@ -23,25 +27,33 @@ class AccountDevicesShareAdapter( return AccountDeviceViewHolder(view, interactor) } - override fun getItemCount(): Int = devices.size - override fun onBindViewHolder(holder: AccountDeviceViewHolder, position: Int) { - holder.bind(devices[position]) + holder.bind(getItem(position)) } - fun updateData(deviceOptions: List) { - this.devices.clear() - this.devices.addAll(deviceOptions) - notifyDataSetChanged() + private object DiffCallback : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: SyncShareOption, newItem: SyncShareOption) = + when (oldItem) { + is SyncShareOption.SendAll -> newItem is SyncShareOption.SendAll + is SyncShareOption.SingleDevice -> + newItem is SyncShareOption.SingleDevice && oldItem.device.id == newItem.device.id + else -> oldItem === newItem + } + + @Suppress("DiffUtilEquals") + override fun areContentsTheSame(oldItem: SyncShareOption, newItem: SyncShareOption) = + oldItem == newItem } } +/** + * Different options to be displayed by [AccountDevicesShareAdapter]. + */ sealed class SyncShareOption { object Reconnect : SyncShareOption() object Offline : SyncShareOption() object SignIn : SyncShareOption() object AddNewDevice : SyncShareOption() data class SendAll(val devices: List) : SyncShareOption() - data class Mobile(val name: String, val device: Device) : SyncShareOption() - data class Desktop(val name: String, val device: Device) : SyncShareOption() + data class SingleDevice(val device: Device) : SyncShareOption() } diff --git a/app/src/main/java/org/mozilla/fenix/share/listadapters/AppShareAdapter.kt b/app/src/main/java/org/mozilla/fenix/share/listadapters/AppShareAdapter.kt index 2914fc1b5..1a585688b 100644 --- a/app/src/main/java/org/mozilla/fenix/share/listadapters/AppShareAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/share/listadapters/AppShareAdapter.kt @@ -12,9 +12,12 @@ import androidx.recyclerview.widget.ListAdapter import org.mozilla.fenix.share.ShareToAppsInteractor import org.mozilla.fenix.share.viewholders.AppViewHolder +/** + * Adapter for a list of apps that can be shared to. + */ class AppShareAdapter( private val interactor: ShareToAppsInteractor -) : ListAdapter(DiffCallback) { +) : ListAdapter(DiffCallback) { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AppViewHolder { val view = LayoutInflater.from(parent.context) @@ -26,20 +29,38 @@ class AppShareAdapter( override fun onBindViewHolder(holder: AppViewHolder, position: Int) { holder.bind(getItem(position)) } + + private object DiffCallback : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: AndroidShareOption, newItem: AndroidShareOption) = + when (oldItem) { + AndroidShareOption.Invisible -> oldItem === newItem + is AndroidShareOption.App -> + newItem is AndroidShareOption.App && oldItem.packageName == newItem.packageName + } + + @Suppress("DiffUtilEquals") + override fun areContentsTheSame(oldItem: AndroidShareOption, newItem: AndroidShareOption) = + oldItem == newItem + } } -private object DiffCallback : DiffUtil.ItemCallback() { - - override fun areItemsTheSame(oldItem: AppShareOption, newItem: AppShareOption) = - oldItem.packageName == newItem.packageName - - override fun areContentsTheSame(oldItem: AppShareOption, newItem: AppShareOption) = - oldItem == newItem +/** + * Represents an app that can be shared to. + */ +sealed class AndroidShareOption { + object Invisible : AndroidShareOption() + /** + * Represents an app that can be shared to. + * + * @property name Name of the app. + * @property icon Icon representing the share target. + * @property packageName Package of the app. + * @property activityName Activity that will be shared to. + */ + data class App( + val name: String, + val icon: Drawable, + val packageName: String, + val activityName: String + ) : AndroidShareOption() } - -data class AppShareOption( - val name: String, - val icon: Drawable, - val packageName: String, - val activityName: String -) diff --git a/app/src/main/java/org/mozilla/fenix/share/listadapters/ShareTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/share/listadapters/ShareTabsAdapter.kt index 655c97b90..f87689845 100644 --- a/app/src/main/java/org/mozilla/fenix/share/listadapters/ShareTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/share/listadapters/ShareTabsAdapter.kt @@ -16,8 +16,11 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.loadIntoView import org.mozilla.fenix.share.ShareTab +/** + * Adapter for a list of tabs to be shared. + */ class ShareTabsAdapter : - ListAdapter(ShareTabDiffCallback()) { + ListAdapter(ShareTabDiffCallback) { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int) = ShareTabViewHolder( LayoutInflater.from(parent.context) @@ -27,13 +30,7 @@ class ShareTabsAdapter : override fun onBindViewHolder(holder: ShareTabViewHolder, position: Int) = holder.bind(getItem(position)) - fun setTabs(tabs: List) { - submitList(tabs.toMutableList()) - } - - inner class ShareTabViewHolder( - itemView: View - ) : RecyclerView.ViewHolder(itemView) { + class ShareTabViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) { fun bind(item: ShareTab) = with(itemView) { context.components.core.icons.loadIntoView(itemView.share_tab_favicon, item.url) @@ -42,19 +39,11 @@ class ShareTabsAdapter : } } - private class ShareTabDiffCallback : DiffUtil.ItemCallback() { - override fun areItemsTheSame( - oldItem: ShareTab, - newItem: ShareTab - ): Boolean { - return oldItem.url == newItem.url - } + private object ShareTabDiffCallback : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: ShareTab, newItem: ShareTab) = + oldItem.url == newItem.url - override fun areContentsTheSame( - oldItem: ShareTab, - newItem: ShareTab - ): Boolean { - return oldItem.url == newItem.url && oldItem.title == newItem.title - } + override fun areContentsTheSame(oldItem: ShareTab, newItem: ShareTab) = + oldItem == newItem } } diff --git a/app/src/main/java/org/mozilla/fenix/share/viewholders/AccountDeviceViewHolder.kt b/app/src/main/java/org/mozilla/fenix/share/viewholders/AccountDeviceViewHolder.kt index 2427d8337..9d02dbd23 100644 --- a/app/src/main/java/org/mozilla/fenix/share/viewholders/AccountDeviceViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/share/viewholders/AccountDeviceViewHolder.kt @@ -11,6 +11,7 @@ import androidx.annotation.VisibleForTesting import androidx.core.content.ContextCompat import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.account_share_list_item.view.* +import mozilla.components.concept.sync.DeviceType import org.mozilla.fenix.R import org.mozilla.fenix.lib.Do import org.mozilla.fenix.share.ShareToAccountDevicesInteractor @@ -35,8 +36,7 @@ class AccountDeviceViewHolder( SyncShareOption.SignIn -> interactor.onSignIn() SyncShareOption.AddNewDevice -> interactor.onAddNewDevice() is SyncShareOption.SendAll -> interactor.onShareToAllDevices(option.devices) - is SyncShareOption.Mobile -> interactor.onShareToDevice(option.device) - is SyncShareOption.Desktop -> interactor.onShareToDevice(option.device) + is SyncShareOption.SingleDevice -> interactor.onShareToDevice(option.device) SyncShareOption.Reconnect -> interactor.onReauth() SyncShareOption.Offline -> { // nothing we are offline @@ -46,7 +46,25 @@ class AccountDeviceViewHolder( } private fun bindView(option: SyncShareOption) { - val (name, drawableRes, colorRes) = when (option) { + val (name, drawableRes, colorRes) = getNameIconBackground(context, option) + + itemView.deviceIcon.apply { + setImageResource(drawableRes) + background.setColorFilter(ContextCompat.getColor(context, colorRes), PorterDuff.Mode.SRC_IN) + drawable.setTint(ContextCompat.getColor(context, R.color.device_foreground)) + } + itemView.isClickable = option != SyncShareOption.Offline + itemView.deviceName.text = name + } + + companion object { + const val LAYOUT_ID = R.layout.account_share_list_item + + /** + * Returns a triple with the name, icon drawable resource, and background color drawable resource + * corresponding to the given [SyncShareOption]. + */ + private fun getNameIconBackground(context: Context, option: SyncShareOption) = when (option) { SyncShareOption.SignIn -> Triple( context.getText(R.string.sync_sign_in), R.drawable.mozac_ic_sync, @@ -72,28 +90,18 @@ class AccountDeviceViewHolder( R.drawable.mozac_ic_select_all, R.color.default_share_background ) - is SyncShareOption.Mobile -> Triple( - option.name, - R.drawable.mozac_ic_device_mobile, - R.color.device_type_mobile_background - ) - is SyncShareOption.Desktop -> Triple( - option.name, - R.drawable.mozac_ic_device_desktop, - R.color.device_type_desktop_background - ) + is SyncShareOption.SingleDevice -> when (option.device.deviceType) { + DeviceType.MOBILE -> Triple( + option.device.displayName, + R.drawable.mozac_ic_device_mobile, + R.color.device_type_mobile_background + ) + else -> Triple( + option.device.displayName, + R.drawable.mozac_ic_device_desktop, + R.color.device_type_desktop_background + ) + } } - - itemView.deviceIcon.apply { - setImageResource(drawableRes) - background.setColorFilter(ContextCompat.getColor(context, colorRes), PorterDuff.Mode.SRC_IN) - drawable.setTint(ContextCompat.getColor(context, R.color.device_foreground)) - } - itemView.isClickable = option != SyncShareOption.Offline - itemView.deviceName.text = name - } - - companion object { - const val LAYOUT_ID = R.layout.account_share_list_item } } diff --git a/app/src/main/java/org/mozilla/fenix/share/viewholders/AppViewHolder.kt b/app/src/main/java/org/mozilla/fenix/share/viewholders/AppViewHolder.kt index 02787247b..bdf098cd3 100644 --- a/app/src/main/java/org/mozilla/fenix/share/viewholders/AppViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/share/viewholders/AppViewHolder.kt @@ -6,32 +6,43 @@ package org.mozilla.fenix.share.viewholders import android.view.View import androidx.annotation.VisibleForTesting +import androidx.core.view.isInvisible import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.app_share_list_item.view.* import org.mozilla.fenix.R +import org.mozilla.fenix.lib.Do import org.mozilla.fenix.share.ShareToAppsInteractor -import org.mozilla.fenix.share.listadapters.AppShareOption +import org.mozilla.fenix.share.listadapters.AndroidShareOption class AppViewHolder( itemView: View, - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - val interactor: ShareToAppsInteractor + @VisibleForTesting val interactor: ShareToAppsInteractor ) : RecyclerView.ViewHolder(itemView) { - private var application: AppShareOption? = null + private var application: AndroidShareOption? = null init { itemView.setOnClickListener { - application?.let { application -> - interactor.onShareToApp(application) + Do exhaustive when (val app = application) { + AndroidShareOption.Invisible, null -> { /* no-op */ } + is AndroidShareOption.App -> interactor.onShareToApp(app) } } } - fun bind(item: AppShareOption) { + fun bind(item: AndroidShareOption) { application = item - itemView.appName.text = item.name - itemView.appIcon.setImageDrawable(item.icon) + + when (item) { + AndroidShareOption.Invisible -> { + itemView.isInvisible = true + } + is AndroidShareOption.App -> { + itemView.isInvisible = false + itemView.appName.text = item.name + itemView.appIcon.setImageDrawable(item.icon) + } + } } companion object { diff --git a/app/src/main/res/layout/fragment_add_new_device.xml b/app/src/main/res/layout/fragment_add_new_device.xml index 59897d3cb..026d12313 100644 --- a/app/src/main/res/layout/fragment_add_new_device.xml +++ b/app/src/main/res/layout/fragment_add_new_device.xml @@ -65,6 +65,7 @@ android:orientation="vertical"/> + () // Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareInteractorTest.kt index 3afac05d6..f59e488da 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareInteractorTest.kt @@ -8,7 +8,7 @@ import io.mockk.mockk import io.mockk.verify import mozilla.components.concept.sync.Device import org.junit.Test -import org.mozilla.fenix.share.listadapters.AppShareOption +import org.mozilla.fenix.share.listadapters.AndroidShareOption class ShareInteractorTest { private val controller = mockk(relaxed = true) @@ -62,7 +62,7 @@ class ShareInteractorTest { @Test fun onShareToApp() { - val app = mockk() + val app = mockk() interactor.onShareToApp(app) diff --git a/app/src/test/java/org/mozilla/fenix/share/listadapters/AccountDevicesShareAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/share/listadapters/AccountDevicesShareAdapterTest.kt index b1ad2a542..6fc132ecd 100644 --- a/app/src/test/java/org/mozilla/fenix/share/listadapters/AccountDevicesShareAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/listadapters/AccountDevicesShareAdapterTest.kt @@ -13,7 +13,6 @@ import io.mockk.just import io.mockk.mockk import io.mockk.spyk import io.mockk.verify -import io.mockk.verifyOrder import mozilla.components.support.test.robolectric.testContext import org.junit.Test import org.junit.runner.RunWith @@ -26,26 +25,8 @@ import org.robolectric.annotation.Config @RunWith(RobolectricTestRunner::class) @Config(application = TestApplication::class) class AccountDevicesShareAdapterTest { - private val syncOptions = mutableListOf(SyncShareOption.AddNewDevice, SyncShareOption.SignIn) - private val syncOptionsEmpty = mutableListOf() private val interactor: ShareInteractor = mockk(relaxed = true) - @Test - fun `updateData should replace all previous data with argument and call notifyDataSetChanged()`() { - // Used AccountDevicesShareAdapter as a spy to ease testing of notifyDataSetChanged() - // and syncOptionsEmpty to be able to record them being called - val adapter = spyk(AccountDevicesShareAdapter(mockk(), syncOptionsEmpty)) - every { adapter.notifyDataSetChanged() } just Runs - - adapter.updateData(syncOptions) - - verifyOrder { - syncOptionsEmpty.clear() - syncOptionsEmpty.addAll(syncOptions) - adapter.notifyDataSetChanged() - } - } - @Test fun `getItemCount on a default instantiated Adapter should return 0`() { val adapter = AccountDevicesShareAdapter(mockk()) @@ -53,13 +34,6 @@ class AccountDevicesShareAdapterTest { assertThat(adapter.itemCount).isEqualTo(0) } - @Test - fun `getItemCount after updateData() call should return the the passed in list's size`() { - val adapter = AccountDevicesShareAdapter(mockk(), syncOptions) - - assertThat(adapter.itemCount).isEqualTo(2) - } - @Test fun `the adapter uses the right ViewHolder`() { val adapter = AccountDevicesShareAdapter(interactor) @@ -84,7 +58,9 @@ class AccountDevicesShareAdapterTest { @Test fun `the adapter binds the right item to a ViewHolder`() { - val adapter = AccountDevicesShareAdapter(interactor, syncOptions) + val syncOptions = listOf(SyncShareOption.AddNewDevice, SyncShareOption.SignIn) + val adapter = AccountDevicesShareAdapter(interactor) + adapter.submitList(syncOptions) val parentView: ViewGroup = mockk(relaxed = true) val itemView: ViewGroup = mockk(relaxed = true) every { parentView.context } returns testContext diff --git a/app/src/test/java/org/mozilla/fenix/share/listadapters/AppShareAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/share/listadapters/AppShareAdapterTest.kt index a2816dd31..e2cc09cf9 100644 --- a/app/src/test/java/org/mozilla/fenix/share/listadapters/AppShareAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/listadapters/AppShareAdapterTest.kt @@ -14,6 +14,7 @@ import io.mockk.mockk import io.mockk.spyk import io.mockk.verify import io.mockk.verifyOrder +import kotlinx.android.synthetic.main.app_share_list_item.view.* import mozilla.components.support.test.robolectric.testContext import org.junit.Test import org.junit.runner.RunWith @@ -27,11 +28,11 @@ import org.robolectric.annotation.Config @Config(application = TestApplication::class) class AppShareAdapterTest { - private val appOptions = mutableListOf( - AppShareOption("App 0", mockk(), "package 0", "activity 0"), - AppShareOption("App 1", mockk(), "package 1", "activity 1") + private val appOptions = mutableListOf( + AndroidShareOption.App("App 0", mockk(), "package 0", "activity 0"), + AndroidShareOption.App("App 1", mockk(), "package 1", "activity 1") ) - private val appOptionsEmpty = emptyList() + private val appOptionsEmpty = emptyList() private val interactor: ShareInteractor = mockk(relaxed = true) @Test