From 11404a576be777fe5ef782a59876f364b6033436 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 18 Jan 2023 13:04:44 +0200 Subject: [PATCH] [fenix] Bug 1812518 - Control the snackbar positioning from Fenix Previously Android-Components - BrowserToolbarBehavior would be responsible for positioning the snackbar above the toolbar. With that responsibility removed we can handle in Fenix positioning the snackbar depending on the toolbar and many more cases - like positioning it depending on the download dialogs. --- .../mozilla/fenix/browser/BaseBrowserFragment.kt | 18 +- .../org/mozilla/fenix/browser/BrowserFragment.kt | 2 +- .../org/mozilla/fenix/components/FenixSnackbar.kt | 14 ++ .../fenix/components/FenixSnackbarBehavior.kt | 76 +++++++++ .../app/src/main/res/layout/fragment_browser.xml | 5 + .../fenix/components/FenixSnackbarBehaviorTest.kt | 187 +++++++++++++++++++++ .../fenix/tabstray/ext/FenixSnackbarKtTest.kt | 33 ++++ 7 files changed, 325 insertions(+), 10 deletions(-) create mode 100644 mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt create mode 100644 mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/FenixSnackbarBehaviorTest.kt diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index a886d9793d70..3937c5b5b93c 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -359,7 +359,7 @@ abstract class BaseBrowserFragment : } viewLifecycleOwner.lifecycleScope.allowUndo( - binding.browserLayout, + binding.dynamicSnackbarContainer, snackbarMessage, requireContext().getString(R.string.snackbar_deleted_undo), { @@ -438,7 +438,7 @@ abstract class BaseBrowserFragment : feature = ContextMenuFeature( fragmentManager = parentFragmentManager, store = store, - candidates = getContextMenuCandidates(context, binding.browserLayout), + candidates = getContextMenuCandidates(context, binding.dynamicSnackbarContainer), engineView = binding.engineView, useCases = context.components.useCases.contextMenuUseCases, tabId = customTabSessionId, @@ -527,7 +527,7 @@ abstract class BaseBrowserFragment : didFail = downloadJobStatus == DownloadState.Status.FAILED, tryAgain = downloadFeature::tryAgain, onCannotOpenFile = { - showCannotOpenFileError(binding.browserLayout, context, it) + showCannotOpenFileError(binding.dynamicSnackbarContainer, context, it) }, binding = binding.viewDynamicDownloadDialog, toolbarHeight = toolbarHeight, @@ -971,7 +971,7 @@ abstract class BaseBrowserFragment : didFail = savedDownloadState.second, tryAgain = onTryAgain, onCannotOpenFile = { - showCannotOpenFileError(binding.browserLayout, context, it) + showCannotOpenFileError(binding.dynamicSnackbarContainer, context, it) }, binding = binding.viewDynamicDownloadDialog, toolbarHeight = toolbarHeight, @@ -1297,7 +1297,7 @@ abstract class BaseBrowserFragment : withContext(Main) { view?.let { FenixSnackbar.make( - view = binding.browserLayout, + view = binding.dynamicSnackbarContainer, duration = FenixSnackbar.LENGTH_LONG, isDisplayedWithBrowserToolbar = true, ) @@ -1318,7 +1318,7 @@ abstract class BaseBrowserFragment : withContext(Main) { view?.let { FenixSnackbar.make( - view = binding.browserLayout, + view = binding.dynamicSnackbarContainer, duration = FenixSnackbar.LENGTH_LONG, isDisplayedWithBrowserToolbar = true, ) @@ -1361,7 +1361,7 @@ abstract class BaseBrowserFragment : // Close find in page bar if opened findInPageIntegration.onBackPressed() FenixSnackbar.make( - view = binding.browserLayout, + view = binding.dynamicSnackbarContainer, duration = Snackbar.LENGTH_SHORT, isDisplayedWithBrowserToolbar = false, ) @@ -1442,12 +1442,12 @@ abstract class BaseBrowserFragment : } private fun showCannotOpenFileError( - view: View, + container: ViewGroup, context: Context, downloadState: DownloadState, ) { FenixSnackbar.make( - view = view, + view = container, duration = Snackbar.LENGTH_SHORT, isDisplayedWithBrowserToolbar = true, ).setText(DynamicDownloadDialog.getCannotOpenFileErrorMessage(context, downloadState)) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 57b267608a2c..378d91e32d5c 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -442,7 +442,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { } } FenixSnackbar.make( - view = binding.browserLayout, + view = binding.dynamicSnackbarContainer, duration = Snackbar.LENGTH_SHORT, isDisplayedWithBrowserToolbar = true, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt index 73a4fad72396..b6914e1a3559 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt @@ -141,10 +141,20 @@ class FenixSnackbar private constructor( 0 }, ) + + if (parent.id == R.id.dynamicSnackbarContainer) { + (parent.layoutParams as? CoordinatorLayout.LayoutParams)?.apply { + behavior = FenixSnackbarBehavior( + context = view.context, + toolbarPosition = view.context.settings().toolbarPosition, + ) + } + } } } // Use the same implementation of `Snackbar` + @Suppress("ReturnCount") private fun findSuitableParent(_view: View?): ViewGroup? { var view = _view var fallback: ViewGroup? = null @@ -159,6 +169,10 @@ class FenixSnackbar private constructor( return view } + if (view.id == R.id.dynamicSnackbarContainer) { + return view + } + fallback = view } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt new file mode 100644 index 000000000000..7ee4dd54f727 --- /dev/null +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt @@ -0,0 +1,76 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components + +import android.content.Context +import android.view.Gravity +import android.view.View +import androidx.annotation.VisibleForTesting +import androidx.coordinatorlayout.widget.CoordinatorLayout +import androidx.core.view.children +import androidx.core.view.isVisible +import org.mozilla.fenix.R +import org.mozilla.fenix.components.toolbar.ToolbarPosition + +/** + * [CoordinatorLayout.Behavior] to be used by a snackbar that want to ensure it it always positioned + * such that it will be shown on top (vertically) of other siblings that may obstruct it's view. + * + * @param context [Context] used for various system interactions. + * @param toolbarPosition Where the toolbar is positioned on the screen. + * Depending on it's position (top / bottom) the snackbar will be shown below / above the toolbar. + */ +class FenixSnackbarBehavior( + context: Context, + @get:VisibleForTesting internal val toolbarPosition: ToolbarPosition, +) : CoordinatorLayout.Behavior(context, null) { + + private val dependenciesIds = listOf( + R.id.viewDynamicDownloadDialog, + R.id.toolbar, + ) + + private var currentAnchorId = View.NO_ID + + override fun layoutDependsOn( + parent: CoordinatorLayout, + child: V, + dependency: View, + ): Boolean { + val anchorId = dependenciesIds + .intersect(parent.children.filter { it.isVisible }.map { it.id }.toSet()) + .firstOrNull() + + // It is possible that previous anchor's visibility is changed. + // The layout is updated and layoutDependsOn is called but onDependentViewChanged not. + // We have to check here if a new anchor is available and reparent the snackbar. + // This check also ensures we are not positioning the snackbar multiple times for the same anchor. + return if (anchorId != currentAnchorId) { + positionSnackbar(child, parent.children.firstOrNull { it.id == anchorId }) + true + } else { + false + } + } + + private fun positionSnackbar(child: View, dependency: View?) { + currentAnchorId = dependency?.id ?: View.NO_ID + val params = child.layoutParams as CoordinatorLayout.LayoutParams + + if (dependency == null || (dependency.id == R.id.toolbar && toolbarPosition == ToolbarPosition.TOP)) { + // Position the snackbar at the bottom of the screen. + params.anchorId = View.NO_ID + params.anchorGravity = Gravity.NO_GRAVITY + params.gravity = Gravity.BOTTOM or Gravity.CENTER_HORIZONTAL + } else { + // Position the snackbar just above the anchor. + params.anchorId = dependency.id + params.anchorGravity = Gravity.TOP or Gravity.CENTER_HORIZONTAL + params.gravity = Gravity.TOP or Gravity.CENTER_HORIZONTAL + } + + child.layoutParams = params + } +} diff --git a/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml b/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml index f337cb15421d..a8b3b29a00cb 100644 --- a/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml +++ b/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml @@ -66,6 +66,11 @@ android:layout_height="match_parent" android:visibility="gone" /> + + (testContext, ToolbarPosition.BOTTOM) + + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + } + + @Test + fun `GIVEN the dynamic download dialog is shown WHEN the snackbar is shown THEN place the snackbar above the dialog`() { + dependency.id = R.id.viewDynamicDownloadDialog + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.BOTTOM) + + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + + assertSnackbarPlacementAboveAnchor() + } + + @Test + fun `GIVEN a bottom toolbar is shown WHEN the snackbar is shown THEN place the snackbar above the toolbar`() { + dependency.id = R.id.toolbar + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.BOTTOM) + + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + + assertSnackbarPlacementAboveAnchor() + } + + @Test + fun `GIVEN a toolbar and a dynamic download dialog are shown WHEN the snackbar is shown THEN place the snackbar above the dialog`() { + listOf(R.id.viewDynamicDownloadDialog, R.id.toolbar).forEach { + parent.addView(View(testContext).apply { id = it }) + } + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.BOTTOM) + + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + + assertSnackbarPlacementAboveAnchor(parent.findViewById(R.id.viewDynamicDownloadDialog)) + } + + @Test + fun `GIVEN the snackbar is anchored to the dynamic download dialog and a bottom toolbar is shown WHEN the dialog is not shown anymore THEN place the snackbar above the toolbar`() { + val dialog = View(testContext) + .apply { id = R.id.viewDynamicDownloadDialog } + .also { parent.addView(it) } + val toolbar = View(testContext) + .apply { id = R.id.toolbar } + .also { parent.addView(it) } + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.BOTTOM) + + // Test the scenario where the dialog is invisible. + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(dialog) + dialog.visibility = View.GONE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(toolbar) + + // Test the scenario where the dialog is removed from parent. + dialog.visibility = View.VISIBLE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(dialog) + parent.removeView(dialog) + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(toolbar) + } + + @Test + fun `GIVEN the snackbar is anchored to the bottom toolbar WHEN the toolbar is not shown anymore THEN place the snackbar at the bottom`() { + val toolbar = View(testContext) + .apply { id = R.id.toolbar } + .also { parent.addView(it) } + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.BOTTOM) + + // Test the scenario where the toolbar is invisible. + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(toolbar) + toolbar.visibility = View.GONE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + + // Test the scenario where the toolbar is removed from parent. + toolbar.visibility = View.VISIBLE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(toolbar) + parent.removeView(toolbar) + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + } + + @Test + fun `GIVEN the snackbar is anchored to the dynamic download dialog and a top toolbar is shown WHEN the dialog is not shown anymore THEN place the snackbar to the bottom`() { + val dialog = View(testContext) + .apply { id = R.id.viewDynamicDownloadDialog } + .also { parent.addView(it) } + View(testContext) + .apply { id = R.id.toolbar } + .also { parent.addView(it) } + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.TOP) + + // Test the scenario where the dialog is invisible. + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(dialog) + dialog.visibility = View.GONE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + + // Test the scenario where the dialog is removed from parent. + dialog.visibility = View.VISIBLE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarPlacementAboveAnchor(dialog) + parent.removeView(dialog) + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + } + + @Test + fun `GIVEN the snackbar is anchored based on a top toolbar WHEN the toolbar is not shown anymore THEN place the snackbar at the bottom`() { + val toolbar = View(testContext) + .apply { id = R.id.toolbar } + .also { parent.addView(it) } + val behavior = FenixSnackbarBehavior(testContext, ToolbarPosition.TOP) + + // Test the scenario where the toolbar is invisible. + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + toolbar.visibility = View.GONE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + + // Test the scenario where the toolbar is removed from parent. + toolbar.visibility = View.VISIBLE + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + parent.removeView(toolbar) + behavior.layoutDependsOn(parent, snackbarContainer, dependency) + assertSnackbarIsPlacedAtTheBottomOfTheScreen() + } + + private fun assertSnackbarPlacementAboveAnchor(anchor: View = dependency) { + assertEquals(anchor.id, snackbarContainer.params.anchorId) + assertEquals(Gravity.TOP or Gravity.CENTER_HORIZONTAL, snackbarContainer.params.anchorGravity) + assertEquals(Gravity.TOP or Gravity.CENTER_HORIZONTAL, snackbarContainer.params.gravity) + } + + private fun assertSnackbarIsPlacedAtTheBottomOfTheScreen() { + assertEquals(View.NO_ID, snackbarContainer.params.anchorId) + assertEquals(Gravity.NO_GRAVITY, snackbarContainer.params.anchorGravity) + assertEquals(Gravity.BOTTOM or Gravity.CENTER_HORIZONTAL, snackbarContainer.params.gravity) + } + + private val FrameLayout.params + get() = layoutParams as CoordinatorLayout.LayoutParams +} diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/ext/FenixSnackbarKtTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/ext/FenixSnackbarKtTest.kt index e2bdf4e3e365..a8ae568af1a6 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/ext/FenixSnackbarKtTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/ext/FenixSnackbarKtTest.kt @@ -6,16 +6,29 @@ package org.mozilla.fenix.tabstray.ext import android.content.Context import android.view.View +import android.widget.FrameLayout +import androidx.coordinatorlayout.widget.CoordinatorLayout import io.mockk.every import io.mockk.mockk +import io.mockk.mockkStatic import io.mockk.verifyOrder +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test +import org.junit.runner.RunWith import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.components.FenixSnackbarBehavior +import org.mozilla.fenix.components.toolbar.ToolbarPosition +import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.MockkRetryTestRule import org.mozilla.fenix.tabstray.TabsTrayFragment.Companion.ELEVATION +import org.mozilla.fenix.utils.Settings +@RunWith(FenixRobolectricTestRunner::class) class FenixSnackbarKtTest { @get:Rule @@ -94,4 +107,24 @@ class FenixSnackbarKtTest { snackbar.setAction("test1", any()) } } + + @Test + fun `GIVEN the snackbar is a child of dynamic container WHEN it is shown THEN enable the dynamic behavior`() { + val container = FrameLayout(testContext).apply { + id = R.id.dynamicSnackbarContainer + layoutParams = CoordinatorLayout.LayoutParams(0, 0) + } + val settings: Settings = mockk(relaxed = true) { + every { toolbarPosition } returns ToolbarPosition.BOTTOM + } + mockkStatic("org.mozilla.fenix.ext.ContextKt") { + every { any().settings() } returns settings + + FenixSnackbar.make(view = container) + + val behavior = (container.layoutParams as? CoordinatorLayout.LayoutParams)?.behavior + assertTrue(behavior is FenixSnackbarBehavior) + assertEquals(ToolbarPosition.BOTTOM, (behavior as? FenixSnackbarBehavior)?.toolbarPosition) + } + } } -- 2.11.4.GIT