Issue #9128 & #9222 & #9499: Refactors snackbar creation and fixes placement (#9628)

app/src/main/java/org/mozilla/fenix/addons/Extensions.kt

@@ -26,7 +26,11 @@ internal fun getFormattedAmount(amount: Int): String {
  * @param text The text to display in the [FenixSnackbar].
  */
 internal fun showSnackBar(view: View, text: String) {
-    FenixSnackbar.make(view, FenixSnackbar.LENGTH_SHORT)
+    FenixSnackbar.make(
+        view = view,
+        duration = FenixSnackbar.LENGTH_SHORT,
+        isDisplayedOnBrowserFragment = true
+    )
         .setText(text)
         .show()
 }

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -305,7 +305,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                         download = download,
                         tryAgain = downloadFeature::tryAgain,
                         onCannotOpenFile = {
-                            FenixSnackbar.makeWithToolbarPadding(view, Snackbar.LENGTH_SHORT)
+                            FenixSnackbar.make(
+                                view = view,
+                                duration = Snackbar.LENGTH_SHORT,
+                                isDisplayedOnBrowserFragment = true
+                            )
                                 .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file))
                                 .show()
                         }
@@ -422,7 +426,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                     customTabSessionId
                 ) { inFullScreen ->
                     if (inFullScreen) {
-                        FenixSnackbar.make(view.rootView, Snackbar.LENGTH_SHORT)
+                        FenixSnackbar.make(
+                            view = view,
+                            duration = Snackbar.LENGTH_SHORT,
+                            isDisplayedOnBrowserFragment = true
+                        )
                             .setText(getString(R.string.full_screen_notification))
                             .show()
                         activity?.enterToImmersiveMode()
@@ -785,7 +793,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                 requireComponents.analytics.metrics.track(Event.AddBookmark)
 
                 view?.let { view ->
-                    FenixSnackbar.makeWithToolbarPadding(view, FenixSnackbar.LENGTH_LONG)
+                    FenixSnackbar.make(
+                        view = view,
+                        duration = FenixSnackbar.LENGTH_LONG,
+                        isDisplayedOnBrowserFragment = true
+                    )
                         .setText(getString(R.string.bookmark_saved_snackbar))
                         .setAction(getString(R.string.edit_bookmark_snackbar_action)) {
                             nav(

app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt

@@ -210,7 +210,11 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
 
         private fun showTabSavedToCollectionSnackbar() {
             view?.let { view ->
-                FenixSnackbar.makeWithToolbarPadding(view, Snackbar.LENGTH_SHORT)
+                FenixSnackbar.make(
+                    view = view,
+                    duration = Snackbar.LENGTH_SHORT,
+                    isDisplayedOnBrowserFragment = true
+                )
                     .setText(view.context.getString(R.string.create_collection_tab_saved))
                     .show()
             }

app/src/main/java/org/mozilla/fenix/browser/FenixSnackbarDelegate.kt

@@ -19,12 +19,19 @@ class FenixSnackbarDelegate(val view: View) :
         listener: ((v: View) -> Unit)?
     ) {
         if (listener != null && action != 0) {
-            FenixSnackbar.makeWithToolbarPadding(view, duration = FenixSnackbar.LENGTH_SHORT)
+            FenixSnackbar.make(
+                view = view,
+                duration = FenixSnackbar.LENGTH_SHORT,
+                isDisplayedOnBrowserFragment = true
+            )
                 .setText(view.context.getString(text))
                 .setAction(view.context.getString(action)) { listener.invoke(view) }
                 .show()
         } else {
-            FenixSnackbar.makeWithToolbarPadding(view, duration = FenixSnackbar.LENGTH_SHORT)
+            FenixSnackbar.make(view,
+                duration = FenixSnackbar.LENGTH_SHORT,
+                isDisplayedOnBrowserFragment = true
+            )
                 .setText(view.context.getString(text))
                 .show()
         }

app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt

@@ -10,12 +10,14 @@ import android.view.View
 import android.view.ViewGroup
 import android.widget.FrameLayout
 import androidx.appcompat.content.res.AppCompatResources
+import androidx.appcompat.widget.ContentFrameLayout
 import androidx.coordinatorlayout.widget.CoordinatorLayout
 import androidx.core.widget.TextViewCompat
 import com.google.android.material.snackbar.BaseTransientBottomBar
 import com.google.android.material.snackbar.ContentViewCallback
 import com.google.android.material.snackbar.Snackbar
 import kotlinx.android.synthetic.main.fenix_snackbar.view.*
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.increaseTapArea
 import org.mozilla.fenix.ext.settings
@@ -86,8 +88,20 @@ class FenixSnackbar private constructor(
         /**
          * Display a snackbar in the given view with duration and proper normal/error styling.
          * Note: Duration is overriden for users with accessibility settings enabled
+         * displayedOnFragmentWithToolbar should be true for all snackbars that will end up
+         * being displayed on a BrowserFragment and must be true in cases where the fragment is
+         * going to pop TO BrowserFragment (e.g. EditBookmarkFragment, ShareFragment)
+         *
+         * Suppressing ComplexCondition. Yes it's unfortunately complex but that's the nature
+         * of the snackbar handling by Android. It will be simpler once dynamic toolbar is always on.
          */
-        fun make(view: View, duration: Int, isError: Boolean = false): FenixSnackbar {
+        @Suppress("ComplexCondition")
+        fun make(
+            view: View,
+            duration: Int = LENGTH_LONG,
+            isError: Boolean = false,
+            isDisplayedOnBrowserFragment: Boolean
+        ): FenixSnackbar {
             val parent = findSuitableParent(view) ?: run {
                 throw IllegalArgumentException(
                     "No suitable parent found from the given view. Please provide a valid view."
@@ -104,30 +118,31 @@ class FenixSnackbar private constructor(
             }
 
             val callback = FenixSnackbarCallback(content)
+            val shouldUseBottomToolbar = view.context.settings().shouldUseBottomToolbar
+            val toolbarHeight = view.context.resources
+                .getDimensionPixelSize(R.dimen.browser_toolbar_height)
+
             return FenixSnackbar(parent, content, callback, isError).also {
                 it.duration = durationOrAccessibleDuration
-            }
-        }
 
-        /**
-         * Considers BrowserToolbar for padding when making snackbar. The vast majority of the time
-         * you will want to pass in `fragment.view`.
-         */
-        fun makeWithToolbarPadding(
-            fragmentView: View,
-            duration: Int = LENGTH_LONG,
-            isError: Boolean = false
-        ): FenixSnackbar {
-            val shouldUseBottomToolbar = fragmentView.context.settings().shouldUseBottomToolbar
-            val toolbarHeight = fragmentView.context.resources
-                .getDimensionPixelSize(R.dimen.browser_toolbar_height)
-
-            return make(fragmentView, duration, isError).apply {
-                this.view.setPadding(
+                it.view.setPadding(
                     0,
                     0,
                     0,
-                    if (shouldUseBottomToolbar) toolbarHeight else 0
+                    if (
+                        isDisplayedOnBrowserFragment &&
+                        shouldUseBottomToolbar &&
+                        // If the view passed in is a ContentFrameLayout, it does not matter
+                        // if the user has a dynamicBottomToolbar or not, as the Android system
+                        // can't intelligently position the snackbar on the upper most view.
+                        // Ideally we should not pass ContentFrameLayout in, but it's the only
+                        // way to display snackbars through a fragment transition.
+                        (!FeatureFlags.dynamicBottomToolbar || view is ContentFrameLayout)
+                    ) {
+                        toolbarHeight
+                    } else {
+                        0
+                    }
                 )
             }
         }

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt

@@ -166,7 +166,11 @@ class DefaultBrowserToolbarController(
                         topSiteStorage.addTopSite(it.title, it.url)
                     }
                     MainScope().launch {
-                        FenixSnackbar.makeWithToolbarPadding(swipeRefresh, Snackbar.LENGTH_SHORT)
+                        FenixSnackbar.make(
+                            view = swipeRefresh,
+                            duration = Snackbar.LENGTH_SHORT,
+                            isDisplayedOnBrowserFragment = true
+                        )
                             .setText(
                                 swipeRefresh.context.getString(R.string.snackbar_added_to_top_sites)
                             )
@@ -258,7 +262,11 @@ class DefaultBrowserToolbarController(
                 // browsing data on quit" is activated). After the deletion is over, the snackbar
                 // is dismissed.
                 val snackbar: FenixSnackbar? = activity.getRootView()?.let { v ->
-                    FenixSnackbar.makeWithToolbarPadding(v)
+                    FenixSnackbar.make(
+                        view = v,
+                        duration = Snackbar.LENGTH_LONG,
+                        isDisplayedOnBrowserFragment = true
+                    )
                         .setText(v.context.getString(R.string.deleting_browsing_data_in_progress))
                 }
 

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt

@@ -108,9 +108,10 @@ class BrowserToolbarView(
                     clipboard.text = selectedSession?.url
                 }
 
-                FenixSnackbar.makeWithToolbarPadding(
-                    view.rootView.findViewById(android.R.id.content),
-                    Snackbar.LENGTH_SHORT
+                FenixSnackbar.make(
+                    view = view,
+                    duration = Snackbar.LENGTH_SHORT,
+                    isDisplayedOnBrowserFragment = true
                 )
                     .setText(view.context.getString(R.string.browser_toolbar_url_copied_to_clipboard_snackbar))
                     .show()

app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt

@@ -85,7 +85,11 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
                         customTabSessionId
                     ) { exception ->
                         components.analytics.crashReporter.submitCaughtException(exception)
-                        FenixSnackbar.make(view.swipeRefresh, FenixSnackbar.LENGTH_LONG).apply {
+                        FenixSnackbar.make(
+                            view = view.swipeRefresh,
+                            duration = FenixSnackbar.LENGTH_LONG,
+                            isDisplayedOnBrowserFragment = true
+                        ).apply {
                             setText(resources.getString(R.string.unknown_scheme_error_message))
                             setAppropriateBackground(true)
                         }.show()

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -384,7 +384,10 @@ class HomeFragment : Fragment() {
                 override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
                     if (authType != AuthType.Existing) {
                         view?.let {
-                            FenixSnackbar.make(it, Snackbar.LENGTH_SHORT)
+                            FenixSnackbar.make(view = it,
+                                duration = Snackbar.LENGTH_SHORT,
+                                isDisplayedOnBrowserFragment = false
+                            )
                                 .setText(it.context.getString(R.string.onboarding_firefox_account_sync_is_on))
                                 .setAnchorView(toolbarLayout)
                                 .show()
@@ -589,7 +592,7 @@ class HomeFragment : Fragment() {
         nav(R.id.homeFragment, directions)
     }
 
-    @SuppressWarnings("ComplexMethod")
+    @SuppressWarnings("ComplexMethod", "LongMethod")
     private fun createHomeMenu(context: Context, menuButtonView: WeakReference<MenuButton>) = HomeMenu(
         this,
         context,
@@ -649,7 +652,11 @@ class HomeFragment : Fragment() {
                     deleteAndQuit(
                         activity,
                         lifecycleScope,
-                        view?.let { view -> FenixSnackbar.makeWithToolbarPadding(view) }
+                        view?.let { view -> FenixSnackbar.make(
+                            view = view,
+                            isDisplayedOnBrowserFragment = false
+                        )
+                        }
                     )
                 }
                 HomeMenu.Item.Sync -> {
@@ -876,7 +883,10 @@ class HomeFragment : Fragment() {
                 } else {
                     R.string.create_collection_tab_saved
                 }
-                FenixSnackbar.make(view, Snackbar.LENGTH_LONG)
+                FenixSnackbar.make(view = view,
+                    duration = Snackbar.LENGTH_LONG,
+                    isDisplayedOnBrowserFragment = false
+                )
                     .setText(view.context.getString(stringRes))
                     .setAnchorView(snackbarAnchorView)
                     .show()
@@ -887,7 +897,11 @@ class HomeFragment : Fragment() {
     private fun showRenamedSnackbar() {
         view?.let { view ->
             val string = view.context.getString(R.string.snackbar_collection_renamed)
-            FenixSnackbar.make(view, Snackbar.LENGTH_LONG)
+            FenixSnackbar.make(
+                view = view,
+                duration = Snackbar.LENGTH_LONG,
+                isDisplayedOnBrowserFragment = false
+            )
                 .setText(string)
                 .setAnchorView(snackbarAnchorView)
                 .show()

app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingAutomaticSignInViewHolder.kt

@@ -39,7 +39,11 @@ class OnboardingAutomaticSignInViewHolder(private val view: View) : RecyclerView
                             R.string.onboarding_firefox_account_auto_signin_confirm
                         )
                         it.turn_on_sync_button.isEnabled = true
-                        FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText(
+                        FenixSnackbar.make(
+                            view = it,
+                            duration = Snackbar.LENGTH_SHORT,
+                            isDisplayedOnBrowserFragment = false
+                        ).setText(
                             it.context.getString(R.string.onboarding_firefox_account_automatic_signin_failed)
                         ).show()
                     }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -88,7 +88,11 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
             bookmarksController = DefaultBookmarkController(
                 context = context!!,
                 navController = findNavController(),
-                snackbar = FenixSnackbar.make(view, FenixSnackbar.LENGTH_LONG),
+                snackbar = FenixSnackbar.make(
+                    view = view,
+                    duration = FenixSnackbar.LENGTH_LONG,
+                    isDisplayedOnBrowserFragment = false
+                ),
                 deleteBookmarkNodes = ::deleteMulti,
                 invokePendingDeletion = ::invokePendingDeletion
             ),

app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt

@@ -164,7 +164,10 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) {
                                 .popBackStack()
 
                             bookmarkNode?.let { bookmark ->
-                                FenixSnackbar.makeWithToolbarPadding(activity.getRootView()!!)
+                                FenixSnackbar.make(
+                                    view = activity.getRootView()!!,
+                                    isDisplayedOnBrowserFragment = true
+                                )
                                     .setText(
                                         getString(
                                             R.string.bookmark_deletion_snackbar_message,

app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt

@@ -64,7 +64,11 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
             historyStore,
             findNavController(),
             resources,
-            FenixSnackbar.make(view, FenixSnackbar.LENGTH_LONG),
+            FenixSnackbar.make(
+                view = view,
+                duration = FenixSnackbar.LENGTH_LONG,
+                isDisplayedOnBrowserFragment = false
+            ),
             activity?.getSystemService(CLIPBOARD_SERVICE) as ClipboardManager,
             ::openItem,
             ::displayDeleteAllDialog,

app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt

@@ -310,7 +310,11 @@ class AccountSettingsFragment : PreferenceFragmentCompat() {
     private fun getChangeListenerForDeviceName(): Preference.OnPreferenceChangeListener {
         return Preference.OnPreferenceChangeListener { _, newValue ->
             accountSettingsInteractor.onChangeDeviceName(newValue as String) {
-                FenixSnackbar.make(view!!, FenixSnackbar.LENGTH_LONG)
+                FenixSnackbar.make(
+                    view = view!!,
+                    duration = FenixSnackbar.LENGTH_LONG,
+                    isDisplayedOnBrowserFragment = false
+                )
                     .setText(getString(R.string.empty_device_name_error))
                     .show()
             }

app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt

@@ -81,11 +81,19 @@ class TurnOnSyncFragment : Fragment(), AccountObserver {
         // 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)
+            FenixSnackbar.make(
+                view = requireView(),
+                duration = snackbarLength,
+                isDisplayedOnBrowserFragment = true
+            )
                 .setText(snackbarText)
                 .show()
         } else {
-            FenixSnackbar.make(requireView(), snackbarLength)
+            FenixSnackbar.make(
+                view = requireView(),
+                duration = snackbarLength,
+                isDisplayedOnBrowserFragment = false
+            )
                 .setText(snackbarText)
                 .show()
         }

app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt

@@ -136,7 +136,11 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da
 
         updateItemCounts()
 
-        FenixSnackbar.makeWithToolbarPadding(requireView(), FenixSnackbar.LENGTH_SHORT)
+        FenixSnackbar.make(
+            view = requireView(),
+            duration = FenixSnackbar.LENGTH_SHORT,
+            isDisplayedOnBrowserFragment = true
+        )
             .setText(resources.getString(R.string.preferences_delete_browsing_data_snackbar))
             .show()
 

app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginSiteInfoFragment.kt

@@ -181,7 +181,11 @@ class SavedLoginSiteInfoFragment : Fragment(R.layout.fragment_saved_login_site_i
 
         private fun showCopiedSnackbar(copiedItem: String) {
             view?.let {
-                FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText(copiedItem).show()
+                FenixSnackbar.make(
+                    view = it,
+                    duration = Snackbar.LENGTH_SHORT,
+                    isDisplayedOnBrowserFragment = false
+                ).setText(copiedItem).show()
             }
         }
     }

app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt

@@ -208,7 +208,10 @@ class AddSearchEngineFragment : Fragment(), CompoundButton.OnCheckedChangeListen
                         .getString(R.string.search_add_custom_engine_success_message, name)
 
                     view?.also {
-                        FenixSnackbar.make(it, FenixSnackbar.LENGTH_SHORT)
+                        FenixSnackbar.make(view = it,
+                            duration = FenixSnackbar.LENGTH_SHORT,
+                            isDisplayedOnBrowserFragment = false
+                        )
                             .setText(successMessage)
                             .show()
                     }

app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt

@@ -154,7 +154,11 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng
                         .getString(R.string.search_edit_custom_engine_success_message, name)
 
                     view?.also {
-                        FenixSnackbar.make(it, FenixSnackbar.LENGTH_SHORT)
+                        FenixSnackbar.make(
+                            view = it,
+                            duration = FenixSnackbar.LENGTH_SHORT,
+                            isDisplayedOnBrowserFragment = false
+                        )
                             .setText(successMessage)
                             .show()
                     }

app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt

@@ -67,7 +67,10 @@ class ShareFragment : AppCompatDialogFragment() {
             DefaultShareController(
                 context = requireContext(),
                 shareData = shareData,
-                snackbar = FenixSnackbar.makeWithToolbarPadding(requireActivity().getRootView()!!),
+                snackbar = FenixSnackbar.make(
+                    view = requireActivity().getRootView()!!,
+                    isDisplayedOnBrowserFragment = true
+                ),
                 navController = findNavController(),
                 sendTabUseCases = SendTabUseCases(accountManager),
                 recentAppsStorage = RecentAppsStorage(requireContext()),