Commit 3311e68d authored by Mugurell's avatar Mugurell
Browse files

For #17686 - Use a custom behavior to scroll InfoBanner with the top toolbar

Previously when the toolbar was on top the banner was inflated in the toolbar's parent - an AppBarLayout.
After migrating to use a custom behavior for scrolling the toolbar and not use
anymore the AppbarLayout for this we needed a new solution.
Using a new behavior to keep this banner in sync with the y translation of the
toolbar gives us most of the old behavior back.
parent d0fd3e82
......@@ -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
......
......@@ -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(
......
/* 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
)
}
}
}
/* 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<View>(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
}
}
......@@ -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() }
......
......@@ -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
......
......@@ -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
......
/* 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 }
}
}
/* 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)
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment