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 a2f3db7d9..6d0cdb926 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -137,7 +137,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { navController = findNavController(), settings = context.settings(), appLinksUseCases = context.components.useCases.appLinksUseCases, - container = browserLayout as ViewGroup + container = browserLayout as ViewGroup, + shouldScrollWithTopToolbar = !context.settings().shouldUseBottomToolbar ), owner = this, view = view diff --git a/app/src/main/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserver.kt b/app/src/main/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserver.kt index 65cb98795..9205a26bd 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserver.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserver.kt @@ -22,6 +22,8 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged import org.mozilla.fenix.R +import org.mozilla.fenix.browser.infobanner.DynamicInfoBanner +import org.mozilla.fenix.browser.infobanner.InfoBanner import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event.BannerOpenInAppGoToSettings import org.mozilla.fenix.ext.components @@ -40,7 +42,9 @@ class OpenInAppOnboardingObserver( private val navController: NavController, private val settings: Settings, private val appLinksUseCases: AppLinksUseCases, - private val container: ViewGroup + private val container: ViewGroup, + @VisibleForTesting + internal val shouldScrollWithTopToolbar: Boolean = false ) : LifecycleAwareFeature { private var scope: CoroutineScope? = null private var currentUrl: String? = null @@ -93,13 +97,14 @@ class OpenInAppOnboardingObserver( } @VisibleForTesting - internal fun createInfoBanner(): InfoBanner { - return InfoBanner( + internal fun createInfoBanner(): DynamicInfoBanner { + return DynamicInfoBanner( context = context, message = context.getString(R.string.open_in_app_cfr_info_message), dismissText = context.getString(R.string.open_in_app_cfr_negative_button_text), actionText = context.getString(R.string.open_in_app_cfr_positive_button_text), container = container, + shouldScrollWithTopToolbar = shouldScrollWithTopToolbar, dismissAction = ::dismissAction ) { val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment( diff --git a/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBanner.kt b/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBanner.kt new file mode 100644 index 000000000..83e11465c --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBanner.kt @@ -0,0 +1,42 @@ +/* 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.browser.infobanner + +import android.content.Context +import android.view.ViewGroup +import androidx.annotation.VisibleForTesting +import androidx.coordinatorlayout.widget.CoordinatorLayout + +/** + * [InfoBanner] that will automatically scroll with the top [BrowserToolbar]. + * Only to be used with [BrowserToolbar]s placed at the top of the screen. + * + * @param shouldScrollWithTopToolbar whether to follow the Y translation of the top toolbar or not + */ +@Suppress("LongParameterList") +class DynamicInfoBanner( + private val context: Context, + container: ViewGroup, + @VisibleForTesting + internal val shouldScrollWithTopToolbar: Boolean = false, + message: String, + dismissText: String, + actionText: String? = null, + dismissByHiding: Boolean = false, + dismissAction: (() -> Unit)? = null, + actionToPerform: (() -> Unit)? = null +) : InfoBanner( + context, container, message, dismissText, actionText, dismissByHiding, dismissAction, actionToPerform +) { + override fun showBanner() { + super.showBanner() + + if (shouldScrollWithTopToolbar) { + (bannerLayout.layoutParams as CoordinatorLayout.LayoutParams).behavior = DynamicInfoBannerBehavior( + context, null + ) + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehavior.kt b/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehavior.kt new file mode 100644 index 000000000..752826248 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehavior.kt @@ -0,0 +1,50 @@ +/* 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.browser.infobanner + +import android.content.Context +import android.util.AttributeSet +import android.view.View +import androidx.annotation.VisibleForTesting +import androidx.coordinatorlayout.widget.CoordinatorLayout +import mozilla.components.browser.toolbar.BrowserToolbar + +/** + * A [CoordinatorLayout.Behavior] implementation to be used when placing [InfoBanner] + * below the BrowserToolbar with which is has to scroll. + * + * This Behavior will keep the Y translations of [InfoBanner] and the top [BrowserToolbar] in sync + * so that the banner will be shown between: + * - the top of the container, being translated over the initial toolbar height (toolbar fully collapsed) + * - immediately below the toolbar (toolbar fully expanded). + */ +class DynamicInfoBannerBehavior( + context: Context?, + attrs: AttributeSet? +) : CoordinatorLayout.Behavior(context, attrs) { + @VisibleForTesting + internal var toolbarHeight: Int = 0 + + override fun layoutDependsOn(parent: CoordinatorLayout, child: View, dependency: View): Boolean { + if (dependency::class == BrowserToolbar::class) { + toolbarHeight = dependency.height + setBannerYTranslation(child, dependency.translationY) + return true + } + + return super.layoutDependsOn(parent, child, dependency) + } + + override fun onDependentViewChanged(parent: CoordinatorLayout, child: View, dependency: View): Boolean { + setBannerYTranslation(child, dependency.translationY) + + return true + } + + @VisibleForTesting + internal fun setBannerYTranslation(banner: View, newYTranslation: Float) { + banner.translationY = toolbarHeight + newYTranslation + } +} diff --git a/app/src/main/java/org/mozilla/fenix/browser/InfoBanner.kt b/app/src/main/java/org/mozilla/fenix/browser/infobanner/InfoBanner.kt similarity index 85% rename from app/src/main/java/org/mozilla/fenix/browser/InfoBanner.kt rename to app/src/main/java/org/mozilla/fenix/browser/infobanner/InfoBanner.kt index d7f4187cc..1d1ba4041 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/InfoBanner.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/infobanner/InfoBanner.kt @@ -2,15 +2,14 @@ * 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.browser +package org.mozilla.fenix.browser.infobanner import android.annotation.SuppressLint import android.content.Context import android.view.LayoutInflater import android.view.View.GONE import android.view.ViewGroup -import android.view.ViewGroup.LayoutParams.MATCH_PARENT -import android.view.ViewGroup.LayoutParams.WRAP_CONTENT +import androidx.annotation.VisibleForTesting import kotlinx.android.synthetic.main.info_banner.view.* import org.mozilla.fenix.R import org.mozilla.fenix.ext.settings @@ -27,7 +26,7 @@ import org.mozilla.fenix.ext.settings * @param actionToPerform - The action to be performed on action button press */ @SuppressWarnings("LongParameterList") -class InfoBanner( +open class InfoBanner( private val context: Context, private val container: ViewGroup, private val message: String, @@ -38,10 +37,11 @@ class InfoBanner( private val actionToPerform: (() -> Unit)? = null ) { @SuppressLint("InflateParams") - private val bannerLayout = LayoutInflater.from(context) + @VisibleForTesting + internal val bannerLayout = LayoutInflater.from(context) .inflate(R.layout.info_banner, null) - internal fun showBanner() { + internal open fun showBanner() { bannerLayout.banner_info_message.text = message bannerLayout.dismiss.text = dismissText @@ -53,10 +53,6 @@ class InfoBanner( container.addView(bannerLayout) - val params = bannerLayout.layoutParams as ViewGroup.LayoutParams - params.height = WRAP_CONTENT - params.width = MATCH_PARENT - bannerLayout.dismiss.setOnClickListener { dismissAction?.invoke() if (dismissByHiding) { bannerLayout.visibility = GONE } else { dismiss() } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index bf97ba942..a748291ee 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -46,10 +46,10 @@ import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.support.ktx.android.util.dpToPx import mozilla.components.ui.tabcounter.TabCounter.Companion.INFINITE_CHAR_PADDING_BOTTOM import org.mozilla.fenix.R -import org.mozilla.fenix.browser.InfoBanner import org.mozilla.fenix.components.metrics.Event import mozilla.components.ui.tabcounter.TabCounter.Companion.MAX_VISIBLE_TABS import mozilla.components.ui.tabcounter.TabCounter.Companion.SO_MANY_TABS_OPEN +import org.mozilla.fenix.browser.infobanner.InfoBanner import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.updateAccessibilityCollectionInfo diff --git a/app/src/test/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserverTest.kt b/app/src/test/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserverTest.kt index ec8405f3f..3645bf604 100644 --- a/app/src/test/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserverTest.kt +++ b/app/src/test/java/org/mozilla/fenix/browser/OpenInAppOnboardingObserverTest.kt @@ -24,12 +24,16 @@ import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.app.links.AppLinksUseCases import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.browser.infobanner.DynamicInfoBanner import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings @@ -46,7 +50,7 @@ class OpenInAppOnboardingObserverTest { private lateinit var appLinksUseCases: AppLinksUseCases private lateinit var context: Context private lateinit var container: ViewGroup - private lateinit var infoBanner: InfoBanner + private lateinit var infoBanner: DynamicInfoBanner private val testDispatcher = TestCoroutineDispatcher() @@ -76,7 +80,8 @@ class OpenInAppOnboardingObserverTest { navController = navigationController, settings = settings, appLinksUseCases = appLinksUseCases, - container = container + container = container, + shouldScrollWithTopToolbar = true )) every { openInAppOnboardingObserver.createInfoBanner() } returns infoBanner } @@ -157,6 +162,24 @@ class OpenInAppOnboardingObserverTest { verify(exactly = 1) { infoBanner.dismiss() } } + @Test + fun `GIVEN a observer WHEN createInfoBanner() THEN the scrollWithTopToolbar is passed to the DynamicInfoBanner`() { + // Mockk currently doesn't support verifying constructor parameters + // But we can check the values found in the constructed objects + + openInAppOnboardingObserver = spyk(OpenInAppOnboardingObserver( + testContext, mockk(), mockk(), mockk(), mockk(), mockk(), mockk(), shouldScrollWithTopToolbar = true + )) + val banner1 = openInAppOnboardingObserver.createInfoBanner() + assertTrue(banner1.shouldScrollWithTopToolbar) + + openInAppOnboardingObserver = spyk(OpenInAppOnboardingObserver( + testContext, mockk(), mockk(), mockk(), mockk(), mockk(), mockk(), shouldScrollWithTopToolbar = false + )) + val banner2 = openInAppOnboardingObserver.createInfoBanner() + assertFalse(banner2.shouldScrollWithTopToolbar) + } + internal class MockedLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner { val lifecycleRegistry = LifecycleRegistry(this).apply { currentState = initialState diff --git a/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehaviorTest.kt b/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehaviorTest.kt new file mode 100644 index 000000000..69fd5d8ca --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerBehaviorTest.kt @@ -0,0 +1,72 @@ +/* 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.browser.infobanner + +import android.view.View +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import mozilla.components.browser.toolbar.BrowserToolbar +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class DynamicInfoBannerBehaviorTest { + @Test + fun `layoutDependsOn should not do anything if not for BrowserToolbar as a dependency`() { + val behavior = spyk(DynamicInfoBannerBehavior(mockk(), null)) + + behavior.layoutDependsOn(mockk(), mockk(), mockk()) + + verify(exactly = 0) { behavior.toolbarHeight } + verify(exactly = 0) { behavior.toolbarHeight = any() } + verify(exactly = 0) { behavior.setBannerYTranslation(any(), any()) } + } + + @Test + fun `layoutDependsOn should update toolbarHeight and translate the banner`() { + val behavior = spyk(DynamicInfoBannerBehavior(mockk(), null)) + val banner: View = mockk(relaxed = true) + val toolbar: BrowserToolbar = mockk { + every { height } returns 99 + every { translationY } returns -33f + } + + assertEquals(0, behavior.toolbarHeight) + + behavior.layoutDependsOn(mockk(), banner, toolbar) + + assertEquals(99, behavior.toolbarHeight) + verify { behavior.setBannerYTranslation(banner, -33f) } + } + + @Test + fun `onDependentViewChanged should translate the banner`() { + val behavior = spyk(DynamicInfoBannerBehavior(mockk(), null)) + val banner: View = mockk(relaxed = true) + val toolbar: BrowserToolbar = mockk { + every { height } returns 50 + every { translationY } returns -23f + } + + behavior.layoutDependsOn(mockk(), banner, toolbar) + + verify { behavior.setBannerYTranslation(banner, -23f) } + } + + @Test + fun `setBannerYTranslation should set banner translation to be toolbarHeight + it's translation`() { + val behavior = spyk(DynamicInfoBannerBehavior(mockk(), null)) + val banner: View = mockk(relaxed = true) + behavior.toolbarHeight = 30 + + behavior.setBannerYTranslation(banner, -20f) + + verify { banner.translationY = 10f } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerTest.kt b/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerTest.kt new file mode 100644 index 000000000..d894071ab --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/browser/infobanner/DynamicInfoBannerTest.kt @@ -0,0 +1,39 @@ +/* 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.browser.infobanner + +import androidx.coordinatorlayout.widget.CoordinatorLayout +import io.mockk.spyk +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class DynamicInfoBannerTest { + @Test + fun `showBanner should set DynamicInfoBannerBehavior as behavior if scrollWithTopToolbar`() { + val banner = spyk(DynamicInfoBanner( + testContext, CoordinatorLayout(testContext), true, "", "" + )) + + banner.showBanner() + + assertTrue((banner.bannerLayout.layoutParams as CoordinatorLayout.LayoutParams).behavior is DynamicInfoBannerBehavior) + } + + @Test + fun `showBanner should not set a behavior if not scrollWithTopToolbar`() { + val banner = spyk(DynamicInfoBanner( + testContext, CoordinatorLayout(testContext), false, "", "" + )) + + banner.showBanner() + + assertNull((banner.bannerLayout.layoutParams as CoordinatorLayout.LayoutParams).behavior) + } +}