From c7c4ad051a4ee62410f6c845f3ec85c1de185e98 Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Thu, 2 Jan 2020 14:31:52 -0800 Subject: [PATCH] For #6413: Adds more snackbar positioning logic (#7444) * For #6413: Adds more snackbar positioning logic * Refactor --- .../fenix/browser/BaseBrowserFragment.kt | 38 +++++----- .../mozilla/fenix/browser/BrowserFragment.kt | 9 ++- .../fenix/browser/FenixSnackbarDelegate.kt | 20 +++--- .../mozilla/fenix/components/FenixSnackbar.kt | 72 +++++++++---------- .../org/mozilla/fenix/home/HomeFragment.kt | 16 ++--- .../library/bookmarks/BookmarkController.kt | 10 +-- .../library/bookmarks/BookmarkFragment.kt | 4 +- .../bookmarks/edit/EditBookmarkFragment.kt | 15 ++-- .../settings/account/TurnOnSyncFragment.kt | 22 ++++-- .../deletebrowsingdata/DeleteAndQuit.kt | 8 ++- .../DeleteBrowsingDataFragment.kt | 2 +- .../mozilla/fenix/share/ShareController.kt | 33 ++++----- .../org/mozilla/fenix/share/ShareFragment.kt | 4 +- app/src/main/res/navigation/nav_graph.xml | 7 +- .../bookmarks/BookmarkControllerTest.kt | 12 ++-- .../deletebrowsingdata/DeleteAndQuitTest.kt | 3 - .../fenix/share/ShareControllerTest.kt | 56 ++++----------- 17 files changed, 158 insertions(+), 173 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 9be1b6a15..483516b96 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -64,7 +64,6 @@ import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R import org.mozilla.fenix.browser.readermode.DefaultReaderModeController import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.BrowserSnackbarPresenter import org.mozilla.fenix.components.FindInPageIntegration import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event @@ -160,7 +159,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session // browsing data on quit" is activated). After the deletion is over, the snackbar // is dismissed. val snackbar: FenixSnackbar? = requireActivity().getRootView()?.let { v -> - FenixSnackbar.make(v, Snackbar.LENGTH_INDEFINITE) + FenixSnackbar.makeWithToolbarPadding(v) .setText(v.context.getString(R.string.deleting_browsing_data_in_progress)) } @@ -283,12 +282,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session download = download, tryAgain = downloadFeature::tryAgain, onCannotOpenFile = { - BrowserSnackbarPresenter(view).present( - text = context.getString(R.string.mozac_feature_downloads_could_not_open_file), - length = Snackbar.LENGTH_SHORT - ) + FenixSnackbar.makeWithToolbarPadding(view, Snackbar.LENGTH_SHORT) + .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file)) + .show() } - ) dialog.show() } @@ -520,9 +517,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session @CallSuper override fun onBackPressed(): Boolean { return findInPageIntegration.onBackPressed() || - fullScreenFeature.onBackPressed() || - sessionFeature.onBackPressed() || - removeSessionIfNeeded() + fullScreenFeature.onBackPressed() || + sessionFeature.onBackPressed() || + removeSessionIfNeeded() } /** @@ -703,17 +700,16 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session requireComponents.analytics.metrics.track(Event.AddBookmark) view?.let { view -> - BrowserSnackbarPresenter(view).present( - text = getString(R.string.bookmark_saved_snackbar), - action = { nav( - R.id.browserFragment, - BrowserFragmentDirections.actionBrowserFragmentToBookmarkEditFragment( - guid - )) - }, - actionName = getString(R.string.edit_bookmark_snackbar_action), - length = Snackbar.LENGTH_LONG - ) + FenixSnackbar.makeWithToolbarPadding(view) + .setText(getString(R.string.bookmark_saved_snackbar)) + .setAction(getString(R.string.edit_bookmark_snackbar_action)) { + nav( + R.id.browserFragment, + BrowserFragmentDirections.actionBrowserFragmentToBookmarkEditFragment( + guid + )) + } + .show() } } } diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 825b786db..f50822896 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -30,7 +30,7 @@ import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components @@ -230,10 +230,9 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { private fun showTabSavedToCollectionSnackbar() { view?.let { view -> - BrowserSnackbarPresenter(view).present( - text = view.context.getString(R.string.create_collection_tab_saved), - length = Snackbar.LENGTH_SHORT - ) + FenixSnackbar.makeWithToolbarPadding(view, Snackbar.LENGTH_SHORT) + .setText(view.context.getString(R.string.create_collection_tab_saved)) + .show() } } } diff --git a/app/src/main/java/org/mozilla/fenix/browser/FenixSnackbarDelegate.kt b/app/src/main/java/org/mozilla/fenix/browser/FenixSnackbarDelegate.kt index 73ffc05af..715f2831e 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/FenixSnackbarDelegate.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/FenixSnackbarDelegate.kt @@ -6,9 +6,8 @@ package org.mozilla.fenix.browser import android.view.View import androidx.annotation.StringRes -import com.google.android.material.snackbar.Snackbar import mozilla.components.feature.contextmenu.ContextMenuCandidate -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar class FenixSnackbarDelegate(val view: View) : ContextMenuCandidate.SnackbarDelegate { @@ -20,17 +19,14 @@ class FenixSnackbarDelegate(val view: View) : listener: ((v: View) -> Unit)? ) { if (listener != null && action != 0) { - BrowserSnackbarPresenter(view).present( - text = view.context.getString(text), - length = Snackbar.LENGTH_LONG, - actionName = view.context.getString(action), - action = { listener.invoke(view) } - ) + FenixSnackbar.makeWithToolbarPadding(view) + .setText(view.context.getString(text)) + .setAction(view.context.getString(action)) { listener.invoke(view) } + .show() } else { - BrowserSnackbarPresenter(view).present( - text = view.context.getString(text), - length = Snackbar.LENGTH_LONG - ) + FenixSnackbar.makeWithToolbarPadding(view) + .setText(view.context.getString(text)) + .show() } } } diff --git a/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt b/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt index 000d5bc32..f044af1ac 100644 --- a/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt +++ b/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt @@ -32,11 +32,7 @@ class FenixSnackbar private constructor( init { view.background = null - view.snackbar_layout.background = if (isError) { - AppCompatResources.getDrawable(context, R.drawable.fenix_snackbar_error_background) - } else { - AppCompatResources.getDrawable(context, R.drawable.fenix_snackbar_background) - } + setAppropriateBackground(isError) content.snackbar_btn.increaseTapArea(actionButtonIncreaseDps) @@ -49,10 +45,22 @@ class FenixSnackbar private constructor( ) } + fun setAppropriateBackground(isError: Boolean) { + view.snackbar_layout.background = if (isError) { + AppCompatResources.getDrawable(context, R.drawable.fenix_snackbar_error_background) + } else { + AppCompatResources.getDrawable(context, R.drawable.fenix_snackbar_background) + } + } + fun setText(text: String) = this.apply { view.snackbar_text.text = text } + fun setLength(duration: Int) = this.apply { + this.duration = duration + } + fun setAction(text: String, action: () -> Unit) = this.apply { view.snackbar_btn.apply { setText(text) @@ -90,6 +98,28 @@ class FenixSnackbar private constructor( } } + /** + * Considers BrowserToolbar for padding when making snackbar + */ + fun makeWithToolbarPadding( + view: View, + duration: Int = LENGTH_LONG, + isError: Boolean = false + ): FenixSnackbar { + val shouldUseBottomToolbar = view.context.settings().shouldUseBottomToolbar + val toolbarHeight = view.context.resources + .getDimensionPixelSize(R.dimen.browser_toolbar_height) + + return make(view, duration, isError).apply { + this.view.setPadding( + 0, + 0, + 0, + if (shouldUseBottomToolbar) toolbarHeight else 0 + ) + } + } + // Use the same implementation of `Snackbar` private fun findSuitableParent(_view: View?): ViewGroup? { var view = _view @@ -147,35 +177,3 @@ private class FenixSnackbarCallback( private const val animateOutDuration = 150L } } - -/** - * This snackbar presenter should be used when displaying a snackbar that will appear in - * the BrowserFragment as it takes into account the position of the BrowserToolbar - */ -class BrowserSnackbarPresenter( - private val view: View -) { - fun present( - text: String, - length: Int = FenixSnackbar.LENGTH_LONG, - action: (() -> Unit)? = null, - actionName: String? = null, - isError: Boolean = false - ) { - val shouldUseBottomToolbar = view.context.settings().shouldUseBottomToolbar - val toolbarHeight = view.context.resources - .getDimensionPixelSize(R.dimen.browser_toolbar_height) - - FenixSnackbar.make(view, length, isError).apply { - if (action != null && actionName != null) setAction(actionName, action) - setText(text) - view.setPadding( - 0, - 0, - 0, - if (shouldUseBottomToolbar) toolbarHeight else 0 - ) - show() - } - } -} 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 0a3fce8d5..698263250 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -20,10 +20,10 @@ import androidx.annotation.StringRes import androidx.appcompat.app.AlertDialog import androidx.constraintlayout.widget.ConstraintSet import androidx.constraintlayout.widget.ConstraintSet.BOTTOM -import androidx.constraintlayout.widget.ConstraintSet.TOP -import androidx.constraintlayout.widget.ConstraintSet.START import androidx.constraintlayout.widget.ConstraintSet.END import androidx.constraintlayout.widget.ConstraintSet.PARENT_ID +import androidx.constraintlayout.widget.ConstraintSet.START +import androidx.constraintlayout.widget.ConstraintSet.TOP import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.lifecycle.Lifecycle @@ -41,12 +41,12 @@ import androidx.transition.TransitionInflater import com.google.android.material.snackbar.Snackbar import kotlinx.android.synthetic.main.fragment_home.* import kotlinx.android.synthetic.main.fragment_home.view.* -import kotlinx.coroutines.delay +import kotlinx.coroutines.Dispatchers.IO +import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.Dispatchers.Main import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.menu.BrowserMenu import mozilla.components.browser.session.Session @@ -546,11 +546,7 @@ class HomeFragment : Fragment() { deleteAndQuit( activity, lifecycleScope, - view?.let { view -> - FenixSnackbar.make(view, Snackbar.LENGTH_INDEFINITE) - .setText(view.context.getString(R.string.deleting_browsing_data_in_progress)) - .setAnchorView(bottom_bar) - } + view?.let { view -> FenixSnackbar.makeWithToolbarPadding(view) } ) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt index 8fc6313da..a8c1caefb 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt @@ -17,7 +17,7 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.Services import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components @@ -46,7 +46,7 @@ interface BookmarkController { class DefaultBookmarkController( private val context: Context, private val navController: NavController, - private val snackbarPresenter: BrowserSnackbarPresenter, + private val snackbar: FenixSnackbar, private val deleteBookmarkNodes: (Set, Event) -> Unit, private val invokePendingDeletion: () -> Unit ) : BookmarkController { @@ -72,13 +72,15 @@ class DefaultBookmarkController( } override fun handleBookmarkSelected(node: BookmarkNode) { - snackbarPresenter.present(resources.getString(R.string.bookmark_cannot_edit_root)) + snackbar.setText(resources.getString(R.string.bookmark_cannot_edit_root)) + snackbar.show() } override fun handleCopyUrl(item: BookmarkNode) { val urlClipData = ClipData.newPlainText(item.url, item.url) context.getSystemService()?.primaryClip = urlClipData - snackbarPresenter.present(resources.getString(R.string.url_copied)) + snackbar.setText(resources.getString(R.string.url_copied)) + snackbar.show() } override fun handleBookmarkSharing(item: BookmarkNode) { 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 ca99abb80..6bf29f4f6 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 @@ -38,7 +38,7 @@ import mozilla.components.concept.sync.OAuthAccount import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.R -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.bookmarkStorage @@ -87,7 +87,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan bookmarksController = DefaultBookmarkController( context = context!!, navController = findNavController(), - snackbarPresenter = BrowserSnackbarPresenter(view), + snackbar = FenixSnackbar.make(view, FenixSnackbar.LENGTH_LONG), deleteBookmarkNodes = ::deleteMulti, invokePendingDeletion = ::invokePendingDeletion ), diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt index 8e6625142..3a2407fd8 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt @@ -37,7 +37,7 @@ import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.R -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView @@ -187,13 +187,14 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) { .popBackStack() bookmarkNode?.let { bookmark -> - BrowserSnackbarPresenter(activity.getRootView()!!).present( - getString( - R.string.bookmark_deletion_snackbar_message, - bookmark.url?.toShortUrl(context.components.publicSuffixList) - ?: bookmark.title + FenixSnackbar.makeWithToolbarPadding(activity.getRootView()!!) + .setText( + getString(R.string.bookmark_deletion_snackbar_message, + bookmark.url?.toShortUrl(context.components.publicSuffixList) + ?: bookmark.title + ) ) - ) + .show() } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt index 5e627ed16..c3453fc02 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt @@ -18,7 +18,6 @@ import mozilla.components.concept.sync.AuthType import mozilla.components.concept.sync.OAuthAccount import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.BrowserSnackbarPresenter import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar @@ -26,6 +25,9 @@ import org.mozilla.fenix.ext.showToolbar @SuppressWarnings("TooManyFunctions") class TurnOnSyncFragment : Fragment(), AccountObserver { + private val safeArguments get() = requireNotNull(arguments) + private val args get() = TurnOnSyncFragmentArgs.fromBundle(safeArguments) + private val signInClickListener = View.OnClickListener { requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext()) // TODO The sign-in web content populates session history, @@ -74,9 +76,19 @@ class TurnOnSyncFragment : Fragment(), AccountObserver { } override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - BrowserSnackbarPresenter(requireView()).present( - text = requireContext().getString(R.string.sync_syncing_in_progress), - length = FenixSnackbar.LENGTH_SHORT - ) + val snackbarText = requireContext().getString(R.string.sync_syncing_in_progress) + val snackbarLength = FenixSnackbar.LENGTH_SHORT + + // Since the snackbar can be presented in BrowserFragment or in SettingsFragment we must + // base our display method on the padSnackbar argument + if (args.padSnackbar) { + FenixSnackbar.makeWithToolbarPadding(requireView(), snackbarLength) + .setText(snackbarText) + .show() + } else { + FenixSnackbar.make(requireView(), snackbarLength) + .setText(snackbarText) + .show() + } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuit.kt b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuit.kt index c140c9c77..0aac06e0a 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuit.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuit.kt @@ -5,11 +5,13 @@ package org.mozilla.fenix.settings.deletebrowsingdata import android.app.Activity +import com.google.android.material.snackbar.Snackbar import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.ext.settings @@ -21,7 +23,11 @@ fun deleteAndQuit(activity: Activity, coroutineScope: CoroutineScope, snackbar: val settings = activity.settings() val controller = DefaultDeleteBrowsingDataController(activity, coroutineContext) - snackbar?.show() + snackbar?.apply { + setText(activity.getString(R.string.deleting_browsing_data_in_progress)) + duration = Snackbar.LENGTH_INDEFINITE + show() + } DeleteBrowsingDataOnQuitType.values().map { type -> launch { diff --git a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt index 59020707e..35ee3c877 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt @@ -146,7 +146,7 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da updateItemCounts() - FenixSnackbar.make(view!!, FenixSnackbar.LENGTH_SHORT) + FenixSnackbar.makeWithToolbarPadding(requireView(), FenixSnackbar.LENGTH_SHORT) .setText(resources.getString(R.string.preferences_delete_browsing_data_snackbar)) .show() 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 9c925dfaf..32b9ea2ba 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -22,7 +22,7 @@ import mozilla.components.concept.sync.Device import mozilla.components.concept.sync.TabData import mozilla.components.feature.accounts.push.SendTabUseCases import org.mozilla.fenix.R -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav @@ -53,7 +53,7 @@ interface ShareController { * @param context [Context] used for various Android interactions. * @param shareData the list of [ShareData]s that can be shared. * @param sendTabUseCases instance of [SendTabUseCases] which allows sending tabs to account devices. - * @param snackbarPresenter - instance of [BrowserSnackbarPresenter] for displaying styled snackbars + * @param snackbar - instance of [FenixSnackbar] for displaying styled snackbars * @param navController - [NavController] used for navigation. * @param dismiss - callback signalling sharing can be closed. */ @@ -62,7 +62,7 @@ class DefaultShareController( private val context: Context, private val shareData: List, private val sendTabUseCases: SendTabUseCases, - private val snackbarPresenter: BrowserSnackbarPresenter, + private val snackbar: FenixSnackbar, private val navController: NavController, private val dismiss: (ShareController.Result) -> Unit ) : ShareController { @@ -89,7 +89,8 @@ class DefaultShareController( context.startActivity(intent) ShareController.Result.SUCCESS } catch (e: SecurityException) { - snackbarPresenter.present(context.getString(R.string.share_error_snackbar)) + snackbar.setText(context.getString(R.string.share_error_snackbar)) + snackbar.show() ShareController.Result.SHARE_ERROR } dismiss(result) @@ -111,7 +112,8 @@ class DefaultShareController( override fun handleSignIn() { context.metrics.track(Event.SignInToSendTab) - val directions = ShareFragmentDirections.actionShareFragmentToTurnOnSyncFragment() + val directions = + ShareFragmentDirections.actionShareFragmentToTurnOnSyncFragment(padSnackbar = true) navController.nav(R.id.shareFragment, directions) dismiss(ShareController.Result.DISMISSED) } @@ -132,21 +134,20 @@ class DefaultShareController( @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) fun showSuccess() { - snackbarPresenter.present( - getSuccessMessage(), - Snackbar.LENGTH_SHORT - ) + snackbar.apply { + setText(getSuccessMessage()) + setLength(Snackbar.LENGTH_SHORT) + show() + } } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) fun showFailureWithRetryOption(operation: () -> Unit) { - snackbarPresenter.present( - text = context.getString(R.string.sync_sent_tab_error_snackbar), - length = Snackbar.LENGTH_LONG, - action = operation, - actionName = context.getString(R.string.sync_sent_tab_error_snackbar_action), - isError = true - ) + snackbar.setText(context.getString(R.string.sync_sent_tab_error_snackbar)) + snackbar.setLength(Snackbar.LENGTH_LONG) + snackbar.setAction(context.getString(R.string.sync_sent_tab_error_snackbar_action), operation) + snackbar.setAppropriateBackground(true) + snackbar.show() } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) 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 d7df65418..7ffebbda9 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt @@ -21,7 +21,7 @@ import mozilla.components.browser.state.selector.findTabOrCustomTab import mozilla.components.concept.engine.prompt.PromptRequest import mozilla.components.feature.accounts.push.SendTabUseCases import org.mozilla.fenix.R -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.requireComponents @@ -65,7 +65,7 @@ class ShareFragment : AppCompatDialogFragment() { DefaultShareController( context = requireContext(), shareData = shareData, - snackbarPresenter = BrowserSnackbarPresenter(requireActivity().getRootView()!!), + snackbar = FenixSnackbar.makeWithToolbarPadding(requireActivity().getRootView()!!), navController = findNavController(), sendTabUseCases = SendTabUseCases(accountManager) ) { result -> diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index a31013539..054e772f8 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -444,6 +444,10 @@ android:id="@+id/turnOnSyncFragment" android:name="org.mozilla.fenix.settings.account.TurnOnSyncFragment" android:label="@string/preferences_sync"> + @@ -555,7 +559,8 @@ android:id="@+id/action_shareFragment_to_turnOnSyncFragment" app:destination="@+id/turnOnSyncFragment" app:popUpTo="@id/shareFragment" - app:popUpToInclusive="true" /> + app:popUpToInclusive="true" > + diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt index d8ce0f03b..1fcbe92fa 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -29,7 +29,7 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.Services import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components @@ -41,7 +41,7 @@ class BookmarkControllerTest { private val context: Context = mockk(relaxed = true) private val navController: NavController = mockk(relaxed = true) - private val snackbarPresenter: BrowserSnackbarPresenter = mockk(relaxed = true) + private val snackbar: FenixSnackbar = mockk(relaxed = true) private val deleteBookmarkNodes: (Set, Event) -> Unit = mockk(relaxed = true) private val invokePendingDeletion: () -> Unit = mockk(relaxed = true) @@ -90,7 +90,7 @@ class BookmarkControllerTest { controller = DefaultBookmarkController( context = homeActivity, navController = navController, - snackbarPresenter = snackbarPresenter, + snackbar = snackbar, deleteBookmarkNodes = deleteBookmarkNodes, invokePendingDeletion = invokePendingDeletion ) @@ -165,7 +165,8 @@ class BookmarkControllerTest { controller.handleBookmarkSelected(root) verify { - snackbarPresenter.present(errorMessage, any(), any(), any()) + snackbar.setText(errorMessage) + snackbar.show() } } @@ -180,7 +181,8 @@ class BookmarkControllerTest { verifyOrder { ClipData.newPlainText(item.url, item.url) - snackbarPresenter.present(urlCopiedMessage, any(), any(), any()) + snackbar.setText(urlCopiedMessage) + snackbar.show() } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuitTest.kt b/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuitTest.kt index 7de4766b5..8fc0f842c 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuitTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteAndQuitTest.kt @@ -88,7 +88,6 @@ class DeleteAndQuitTest { verifyOrder { snackbar.show() removeAllTabsUseCases.invoke() - snackbar.dismiss() activity.finish() } @@ -141,8 +140,6 @@ class DeleteAndQuitTest { historyStorage - snackbar.dismiss() - activity.finish() } } diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt index bac142a5d..6d4cb9d7b 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt @@ -12,8 +12,6 @@ import assertk.assertAll import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isNotEqualTo -import assertk.assertions.isSameAs -import assertk.assertions.isSuccess import assertk.assertions.isTrue import com.google.android.material.snackbar.Snackbar import io.mockk.Runs @@ -35,7 +33,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R import org.mozilla.fenix.TestApplication -import org.mozilla.fenix.components.BrowserSnackbarPresenter +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.metrics @@ -61,11 +59,11 @@ class ShareControllerTest { ) private val textToShare = "${shareData[0].url}\n${shareData[1].url}" private val sendTabUseCases = mockk(relaxed = true) - private val snackbarPresenter = mockk(relaxed = true) + private val snackbar = mockk(relaxed = true) private val navController = mockk(relaxed = true) private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true) private val controller = DefaultShareController( - context, shareData, sendTabUseCases, snackbarPresenter, navController, dismiss + context, shareData, sendTabUseCases, snackbar, navController, dismiss ) @Before @@ -121,7 +119,7 @@ class ShareControllerTest { // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // need to use an Activity Context. val activityContext: Context = mockk() - val testController = DefaultShareController(activityContext, shareData, mockk(), snackbarPresenter, mockk(), dismiss) + val testController = DefaultShareController(activityContext, shareData, mockk(), snackbar, mockk(), dismiss) every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException() every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app" @@ -129,7 +127,8 @@ class ShareControllerTest { verifyOrder { activityContext.startActivity(shareIntent.captured) - snackbarPresenter.present("Cannot share to this app") + snackbar.setText("Cannot share to this app") + snackbar.show() dismiss(ShareController.Result.SHARE_ERROR) } } @@ -211,18 +210,12 @@ class ShareControllerTest { fun `showSuccess should show a snackbar with a success message`() { val expectedMessage = controller.getSuccessMessage() val expectedTimeout = Snackbar.LENGTH_SHORT - val messageSlot = slot() - val timeoutSlot = slot() controller.showSuccess() - verify { snackbarPresenter.present(capture(messageSlot), capture(timeoutSlot)) } - assertAll { - assertThat(messageSlot.isCaptured).isTrue() - assertThat(timeoutSlot.isCaptured).isTrue() - - assertThat(messageSlot.captured).isEqualTo(expectedMessage) - assertThat(timeoutSlot.captured).isEqualTo(expectedTimeout) + verify { + snackbar.setText(expectedMessage) + snackbar.setLength(expectedTimeout) } } @@ -233,35 +226,16 @@ class ShareControllerTest { val operation: () -> Unit = { println("Hello World") } val expectedRetryMessage = context.getString(R.string.sync_sent_tab_error_snackbar_action) - val messageSlot = slot() - val timeoutSlot = slot() - val operationSlot = slot<() -> Unit>() - val retryMesageSlot = slot() - val isFailureSlot = slot() controller.showFailureWithRetryOption(operation) verify { - snackbarPresenter.present( - capture(messageSlot), - capture(timeoutSlot), - capture(operationSlot), - capture(retryMesageSlot), - capture(isFailureSlot) - ) - } - assertAll { - assertThat(messageSlot.isCaptured).isTrue() - assertThat(timeoutSlot.isCaptured).isTrue() - assertThat(operationSlot.isCaptured).isTrue() - assertThat(retryMesageSlot.isCaptured).isTrue() - assertThat(isFailureSlot.isCaptured).isTrue() - - assertThat(messageSlot.captured).isEqualTo(expectedMessage) - assertThat(timeoutSlot.captured).isEqualTo(expectedTimeout) - assertThat { operationSlot.captured }.isSuccess().isSameAs(operation) - assertThat(retryMesageSlot.captured).isEqualTo(expectedRetryMessage) - assertThat(isFailureSlot.captured).isEqualTo(true) + snackbar.apply { + setText(expectedMessage) + setLength(expectedTimeout) + setAction(expectedRetryMessage, operation) + setAppropriateBackground(true) + } } }