diff --git a/fenix/.buildconfig.yml b/fenix/.buildconfig.yml index 10c28a1ff7e1..11c87288484b 100644 --- a/fenix/.buildconfig.yml +++ b/fenix/.buildconfig.yml @@ -83,6 +83,7 @@ projects: - support-rusthttp - support-rustlog - support-test + - support-test-fakes - support-test-libstate - support-utils - support-webextensions diff --git a/fenix/app/build.gradle b/fenix/app/build.gradle index e1b5608406c7..ce2174c4d8b4 100644 --- a/fenix/app/build.gradle +++ b/fenix/app/build.gradle @@ -621,6 +621,7 @@ dependencies { implementation project(':lib-push-firebase') implementation project(':lib-state') implementation project(':lib-dataprotect') + testImplementation project(':support-test-fakes') debugImplementation ComponentsDependencies.leakcanary debugImplementation ComponentsDependencies.androidx_compose_ui_tooling diff --git a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt index 05eeb740b6eb..657b1287fb15 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt @@ -108,7 +108,7 @@ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, User tryAgain = downloadsFeature.get()!!::tryAgain, onCannotOpenFile = onCannotOpenFile, binding = getDownloadDialogLayoutBinding(), - toolbarHeight = 0, + bottomToolbarHeight = 0, ) {}.show() }, ) diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 8ac5638f7d29..fdf51f484d36 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -111,7 +111,6 @@ import mozilla.components.support.ktx.android.view.hideKeyboard import mozilla.components.support.ktx.kotlin.getOrigin import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged import mozilla.components.support.locale.ActivityContextWrapper -import mozilla.components.ui.widgets.behavior.EngineViewClippingBehavior import mozilla.components.ui.widgets.withCenterAlignedButtons import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.FeatureFlags @@ -138,9 +137,12 @@ import org.mozilla.fenix.components.toolbar.DefaultBrowserToolbarMenuController import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature import org.mozilla.fenix.components.toolbar.ToolbarIntegration import org.mozilla.fenix.components.toolbar.ToolbarPosition +import org.mozilla.fenix.components.toolbar.getBottomToolbarHeight +import org.mozilla.fenix.components.toolbar.getTopToolbarHeight import org.mozilla.fenix.components.toolbar.interactor.BrowserToolbarInteractor import org.mozilla.fenix.components.toolbar.interactor.DefaultBrowserToolbarInteractor import org.mozilla.fenix.components.toolbar.navbar.BottomToolbarContainerView +import org.mozilla.fenix.components.toolbar.navbar.EngineViewClippingBehavior import org.mozilla.fenix.components.toolbar.navbar.NavbarIntegration import org.mozilla.fenix.crashes.CrashContentIntegration import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity @@ -175,7 +177,6 @@ import org.mozilla.fenix.utils.allowUndo import org.mozilla.fenix.wifi.SitePermissionsWifiIntegration import java.lang.ref.WeakReference import kotlin.coroutines.cancellation.CancellationException -import mozilla.components.ui.widgets.behavior.ToolbarPosition as MozacToolbarPosition /** * Base fragment extended by [BrowserFragment]. @@ -357,8 +358,6 @@ abstract class BaseBrowserFragment : val store = context.components.core.store val activity = requireActivity() as HomeActivity - val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) - browserAnimator = BrowserAnimator( fragment = WeakReference(this), engineView = WeakReference(binding.engineView), @@ -469,12 +468,15 @@ abstract class BaseBrowserFragment : ) } + val shouldHideOnScroll = + !context.settings().shouldUseFixedTopToolbar && context.settings().isDynamicToolbarEnabled _bottomToolbarContainerView = BottomToolbarContainerView( context = context, parent = binding.browserLayout, androidToolbarView = if (isToolbarAtBottom) browserToolbar else null, menuButton = menuButton, isPrivateMode = activity.browsingModeManager.mode.isPrivate, + hideOnScroll = shouldHideOnScroll, ) navbarIntegration.set( @@ -637,6 +639,8 @@ abstract class BaseBrowserFragment : }, ) + val bottomToolbarHeight = context.getBottomToolbarHeight() + downloadFeature.onDownloadStopped = { downloadState, _, downloadJobStatus -> val onCannotOpenFile: (DownloadState) -> Unit = { FenixSnackbar.make( @@ -671,7 +675,7 @@ abstract class BaseBrowserFragment : tryAgain = downloadFeature::tryAgain, onCannotOpenFile = onCannotOpenFile, binding = binding.viewDynamicDownloadDialog, - toolbarHeight = toolbarHeight, + bottomToolbarHeight = bottomToolbarHeight, ) { sharedViewModel.downloadDialogState.remove(downloadState.sessionId) }.show() @@ -684,7 +688,7 @@ abstract class BaseBrowserFragment : getCurrentTab()?.id, store, context, - toolbarHeight, + bottomToolbarHeight, ) shareDownloadsFeature.set( @@ -1017,7 +1021,11 @@ abstract class BaseBrowserFragment : owner = this, view = view, ) - initializeEngineView(toolbarHeight) + + initializeEngineView( + topToolbarHeight = context.getTopToolbarHeight(), + bottomToolbarHeight = context.getBottomToolbarHeight(), + ) } protected fun showUndoSnackbar(message: String) { @@ -1153,7 +1161,7 @@ abstract class BaseBrowserFragment : sessionId: String?, store: BrowserStore, context: Context, - toolbarHeight: Int, + bottomToolbarHeight: Int, ) { val savedDownloadState = sharedViewModel.downloadDialogState[sessionId] @@ -1191,7 +1199,7 @@ abstract class BaseBrowserFragment : .show() }, binding = binding.viewDynamicDownloadDialog, - toolbarHeight = toolbarHeight, + bottomToolbarHeight = bottomToolbarHeight, onDismiss = onDismiss, ).show() @@ -1206,36 +1214,31 @@ abstract class BaseBrowserFragment : } @VisibleForTesting - internal fun initializeEngineView(toolbarHeight: Int) { + internal fun initializeEngineView( + topToolbarHeight: Int, + bottomToolbarHeight: Int, + ) { val context = requireContext() if (!context.settings().shouldUseFixedTopToolbar && context.settings().isDynamicToolbarEnabled) { - getEngineView().setDynamicToolbarMaxHeight(toolbarHeight) + getEngineView().setDynamicToolbarMaxHeight(topToolbarHeight + bottomToolbarHeight) - val toolbarPosition = when (context.settings().toolbarPosition) { - ToolbarPosition.BOTTOM -> MozacToolbarPosition.BOTTOM - ToolbarPosition.TOP -> MozacToolbarPosition.TOP - } (getSwipeRefreshLayout().layoutParams as CoordinatorLayout.LayoutParams).behavior = EngineViewClippingBehavior( context, null, getSwipeRefreshLayout(), - toolbarHeight, - toolbarPosition, + topToolbarHeight, + !requireComponents.settings.shouldUseBottomToolbar, ) } else { // Ensure webpage's bottom elements are aligned to the very bottom of the engineView. getEngineView().setDynamicToolbarMaxHeight(0) - // Effectively place the engineView on top/below of the toolbar if that is not dynamic. - val swipeRefreshParams = - getSwipeRefreshLayout().layoutParams as CoordinatorLayout.LayoutParams - if (context.settings().toolbarPosition == ToolbarPosition.TOP) { - swipeRefreshParams.topMargin = toolbarHeight - } else { - swipeRefreshParams.bottomMargin = toolbarHeight - } + // Effectively place the engineView on top/below of the toolbars if that is not dynamic. + val swipeRefreshParams = getSwipeRefreshLayout().layoutParams as CoordinatorLayout.LayoutParams + swipeRefreshParams.topMargin = topToolbarHeight + swipeRefreshParams.bottomMargin = bottomToolbarHeight } } @@ -1313,9 +1316,9 @@ abstract class BaseBrowserFragment : fullScreenChanged(false) browserToolbarView.expand() - val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) val context = requireContext() - resumeDownloadDialogState(selectedTab.id, context.components.core.store, context, toolbarHeight) + val bottomToolbarHeight = context.getBottomToolbarHeight() + resumeDownloadDialogState(selectedTab.id, context.components.core.store, context, bottomToolbarHeight) it.announceForAccessibility(selectedTab.toDisplayTitle()) } } else { @@ -1638,8 +1641,10 @@ abstract class BaseBrowserFragment : } if (webAppToolbarShouldBeVisible) { browserToolbarView.view.isVisible = true - val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) - initializeEngineView(toolbarHeight) + initializeEngineView( + topToolbarHeight = requireContext().getTopToolbarHeight(), + bottomToolbarHeight = requireContext().getBottomToolbarHeight(), + ) browserToolbarView.expand() } if (customTabSessionId == null && requireContext().settings().isTabletAndTabStripEnabled) { diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt index e16afc38761a..b6f22f0ccbd3 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt @@ -66,6 +66,7 @@ class TabPreview @JvmOverloads constructor( parent = this, androidToolbarView = if (!isToolbarAtTop) binding.fakeToolbar else null, menuButton = MenuButton(context), + hideOnScroll = false, ) removeView(binding.fakeToolbar) } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/RedesignToolbarFeature.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/RedesignToolbarFeature.kt index 2529b038bf47..6f3813f25ac8 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/RedesignToolbarFeature.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/RedesignToolbarFeature.kt @@ -4,6 +4,9 @@ package org.mozilla.fenix.components.toolbar +import android.content.Context +import org.mozilla.fenix.R +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.utils.Settings /** @@ -40,3 +43,38 @@ class IncompleteRedesignToolbarFeature( override val isEnabled: Boolean get() = settings.enableIncompleteToolbarRedesign } + +/** + * Returns the height of the bottom toolbar element. + * + * The bottom toolbar can consist of a navigation bar, + * of a combination of a navigation and address bar or + * be absent. + */ +fun Context.getBottomToolbarHeight(): Int { + val isNavBarEnabled = IncompleteRedesignToolbarFeature(settings()).isEnabled + val isToolbarAtBottom = settings().shouldUseBottomToolbar + val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) + val navbarHeight = resources.getDimensionPixelSize(R.dimen.browser_navbar_height) + + return when { + isNavBarEnabled && isToolbarAtBottom -> toolbarHeight + navbarHeight + isNavBarEnabled -> navbarHeight + isToolbarAtBottom -> toolbarHeight + else -> 0 + } +} + +/** + * Returns the height of the top toolbar element. + */ +fun Context.getTopToolbarHeight(): Int { + val isToolbarAtTop = !settings().shouldUseBottomToolbar + val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) + + return if (isToolbarAtTop) { + toolbarHeight + } else { + 0 + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt index 5f37d54ce4d2..9a0162d7213b 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt @@ -35,6 +35,7 @@ import org.mozilla.fenix.theme.FirefoxTheme * @param androidToolbarView An option toolbar view that will be added atop of the navigation bar. * @param menuButton A [MenuButton] to be used for [ItemType.MENU]. * @param isPrivateMode If browsing in [BrowsingMode.Private]. + * @param hideOnScroll If the container should react to the [EngineView] content being scrolled. * * Defaults to [NavigationItems.defaultItems] which provides a standard set of navigation items. */ @@ -45,6 +46,7 @@ class BottomToolbarContainerView( androidToolbarView: View? = null, menuButton: MenuButton, isPrivateMode: Boolean = false, + hideOnScroll: Boolean, ) { val toolbarContainerView = ToolbarContainerView(context) @@ -86,7 +88,9 @@ class BottomToolbarContainerView( CoordinatorLayout.LayoutParams.WRAP_CONTENT, ).apply { gravity = Gravity.BOTTOM - behavior = EngineViewScrollingBehavior(parent.context, null, ViewPosition.BOTTOM) + if (hideOnScroll) { + behavior = EngineViewScrollingBehavior(parent.context, null, ViewPosition.BOTTOM) + } } parent.addView(toolbarContainerView) diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/EngineViewClippingBehavior.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/EngineViewClippingBehavior.kt new file mode 100644 index 000000000000..e6061f303b1b --- /dev/null +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/EngineViewClippingBehavior.kt @@ -0,0 +1,99 @@ +/* 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.toolbar.navbar + +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 +import mozilla.components.concept.engine.EngineView +import mozilla.components.concept.toolbar.ScrollableToolbar +import mozilla.components.support.ktx.android.view.findViewInHierarchy + +/** + * A [CoordinatorLayout.Behavior] implementation that allows the [EngineView] to automatically + * size its content and itself in relation to the Y translation of one or multiple [ScrollableToolbar]s. + * + * This is useful in combination with [ScrollableToolbar] ensuring the web content is displayed + * immediately below / above the toolbar even when it is being animated. + * + * @param context [Context] used for various Android interactions. + * @param attrs XML set attributes configuring this. + * @param engineViewParent the parent [View] of the [EngineView]. + * @param topToolbarHeight size of [ScrollableToolbar] when it is placed above the [EngineView]. + * @param hasTopToolbar whether [EngineView] has a toolbar at the top. + */ +class EngineViewClippingBehavior( + context: Context?, + attrs: AttributeSet?, + private val engineViewParent: View, + private val topToolbarHeight: Int, + private val hasTopToolbar: Boolean, +) : CoordinatorLayout.Behavior(context, attrs) { + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var engineView = engineViewParent.findViewInHierarchy { it is EngineView } as EngineView? + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var recentBottomToolbarTranslation = 0f + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var recentTopToolbarTranslation = 0f + + override fun layoutDependsOn(parent: CoordinatorLayout, child: View, dependency: View): Boolean { + if (dependency is ScrollableToolbar) { + return true + } + + return super.layoutDependsOn(parent, child, dependency) + } + + override fun onDependentViewChanged(parent: CoordinatorLayout, child: View, dependency: View): Boolean { + // I can't come up with a scenario where the translationY + // would return a NaN, but given that it was added to the + // previous version of the behaviour and there was a test + // written for it, I played it safe. + if (dependency.translationY.isNaN()) { + return true + } + + when (dependency) { + is BrowserToolbar -> { + if (hasTopToolbar) { + recentTopToolbarTranslation = dependency.translationY + } else { + recentBottomToolbarTranslation = dependency.translationY + } + } + is ToolbarContainerView -> recentBottomToolbarTranslation = dependency.translationY + } + + engineView?.let { + if (hasTopToolbar) { + // Here we are adjusting the vertical position of + // the engine view container to be directly under + // the toolbar. The top toolbar is shifting up, so + // its translation will be either negative or zero. + // It might be safe to use the child view here, but the original + // implementation was adjusting the size of the parent passed + // to the class directly, and I feel cautious to change that + // considering possible side effects. + engineViewParent.translationY = recentTopToolbarTranslation + topToolbarHeight + } + + // We want to position the engine view popup content + // right above the bottom toolbar when the toolbar + // is being shifted down. The top of the bottom toolbar + // is either positive or zero, but for clipping + // the values should be negative because the baseline + // for clipping is bottom toolbar height. + val contentBottomClipping = recentTopToolbarTranslation - recentBottomToolbarTranslation + it.setVerticalClipping(contentBottomClipping.toInt()) + } + return true + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavigationBar.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavigationBar.kt index e6bc1194a208..ec0aeef0e5d0 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavigationBar.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavigationBar.kt @@ -19,6 +19,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.res.dimensionResource import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview @@ -49,7 +50,7 @@ fun NavigationBar( Box( modifier = Modifier .background(FirefoxTheme.colors.layer1) - .height(48.dp) + .height(dimensionResource(id = R.dimen.browser_navbar_height)) .fillMaxWidth(), ) { Row( diff --git a/fenix/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt b/fenix/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt index 6d7e00a2ceeb..fee4711c29dc 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt @@ -30,7 +30,7 @@ class DynamicDownloadDialog( private val tryAgain: (String) -> Unit, private val onCannotOpenFile: (DownloadState) -> Unit, private val binding: DownloadDialogLayoutBinding, - private val toolbarHeight: Int, + private val bottomToolbarHeight: Int, private val onDismiss: () -> Unit, ) { @@ -49,7 +49,7 @@ class DynamicDownloadDialog( DynamicDownloadDialogBehavior( context, null, - toolbarHeight.toFloat(), + bottomToolbarHeight.toFloat(), ) } } @@ -58,7 +58,7 @@ class DynamicDownloadDialog( if (settings.shouldUseBottomToolbar) { val params: ViewGroup.MarginLayoutParams = binding.root.layoutParams as ViewGroup.MarginLayoutParams - params.bottomMargin = toolbarHeight + params.bottomMargin = bottomToolbarHeight } if (didFail) { diff --git a/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index a847f75085f0..bb59c58e4e75 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -469,6 +469,7 @@ class HomeFragment : Fragment() { androidToolbarView = if (isToolbarAtBottom) binding.toolbarLayout else null, menuButton = menuButton, isPrivateMode = activity.browsingModeManager.mode.isPrivate, + hideOnScroll = false, ) navbarIntegration.set( diff --git a/fenix/app/src/main/res/values/dimens.xml b/fenix/app/src/main/res/values/dimens.xml index 8e990dae92ee..32003466d935 100644 --- a/fenix/app/src/main/res/values/dimens.xml +++ b/fenix/app/src/main/res/values/dimens.xml @@ -70,6 +70,7 @@ 56dp + 48dp 0dp diff --git a/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt index 4a7e37e36c10..aa98ad6e9081 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt @@ -17,9 +17,9 @@ import mozilla.components.concept.engine.EngineView import mozilla.components.concept.engine.permission.SitePermissions import mozilla.components.feature.contextmenu.ContextMenuCandidate import mozilla.components.ui.widgets.VerticalSwipeRefreshLayout -import mozilla.components.ui.widgets.behavior.EngineViewClippingBehavior import org.junit.Before import org.junit.Test +import org.mozilla.fenix.components.toolbar.navbar.EngineViewClippingBehavior import org.mozilla.fenix.ext.components import org.mozilla.fenix.utils.Settings @@ -52,7 +52,7 @@ class BaseBrowserFragmentTest { every { settings.shouldUseBottomToolbar } returns false every { settings.shouldUseFixedTopToolbar } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 0) verify { engineView.setDynamicToolbarMaxHeight(0) } } @@ -62,17 +62,18 @@ class BaseBrowserFragmentTest { every { settings.shouldUseBottomToolbar } returns true every { settings.shouldUseFixedTopToolbar } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 13) verify { engineView.setDynamicToolbarMaxHeight(0) } } + // TODO this should also cover the cases where the navigation bar is enabled @Test fun `initializeEngineView should setDynamicToolbarMaxHeight to toolbar height if dynamic toolbar is enabled`() { every { settings.shouldUseFixedTopToolbar } returns false every { settings.isDynamicToolbarEnabled } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 0) verify { engineView.setDynamicToolbarMaxHeight(13) } } @@ -82,7 +83,7 @@ class BaseBrowserFragmentTest { every { settings.shouldUseFixedTopToolbar } returns false every { settings.isDynamicToolbarEnabled } returns false - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 0) verify { engineView.setDynamicToolbarMaxHeight(0) } } @@ -96,7 +97,7 @@ class BaseBrowserFragmentTest { every { swipeRefreshLayout.layoutParams } returns params val behavior = slot() - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 0) // EngineViewClippingBehavior constructor parameters are not properties, we cannot check them. // Ensure just that the right behavior is set. @@ -109,7 +110,7 @@ class BaseBrowserFragmentTest { every { settings.isDynamicToolbarEnabled } returns false every { settings.shouldUseBottomToolbar } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(0, 13) verify { (swipeRefreshLayout.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = 13 } } @@ -119,7 +120,7 @@ class BaseBrowserFragmentTest { every { settings.shouldUseBottomToolbar } returns false every { settings.shouldUseFixedTopToolbar } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(13, 0) verify { (swipeRefreshLayout.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = 13 } } @@ -130,7 +131,7 @@ class BaseBrowserFragmentTest { every { settings.shouldUseBottomToolbar } returns true every { settings.shouldUseFixedTopToolbar } returns true - fragment.initializeEngineView(13) + fragment.initializeEngineView(0, 13) verify { (swipeRefreshLayout.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = 13 } } diff --git a/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/EngineViewClippingBehaviorTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/EngineViewClippingBehaviorTest.kt new file mode 100644 index 000000000000..26c2e68a15e6 --- /dev/null +++ b/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/EngineViewClippingBehaviorTest.kt @@ -0,0 +1,294 @@ +package org.mozilla.fenix.components.toolbar + +import android.view.View +import android.widget.EditText +import android.widget.ImageView +import android.widget.TextView +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.toolbar.BrowserToolbar +import mozilla.components.concept.engine.EngineView +import mozilla.components.support.test.fakes.engine.FakeEngineView +import mozilla.components.support.test.mock +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito +import org.mozilla.fenix.components.toolbar.navbar.EngineViewClippingBehavior +import org.mozilla.fenix.components.toolbar.navbar.ToolbarContainerView + +@RunWith(AndroidJUnit4::class) +class EngineViewClippingBehaviorTest { + + // Bottom toolbar position tests + @Test + fun `GIVEN the toolbar is at the bottom WHEN toolbar is being shifted THEN EngineView adjusts bottom clipping && EngineViewParent position doesn't change`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: BrowserToolbar = mock() + Mockito.doReturn(Y_DOWN_TRANSITION).`when`(toolbar).translationY + + Assert.assertEquals(0f, engineParentView.translationY) + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = false, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + } + + // We want to position the engine view popup content + // right above the bottom toolbar when the toolbar + // is being shifted down. The top of the bottom toolbar + // is either positive or zero, but for clipping + // the values should be negative because the baseline + // for clipping is bottom toolbar height. + val bottomClipping = -Y_DOWN_TRANSITION.toInt() + Mockito.verify(engineView).setVerticalClipping(bottomClipping) + + Assert.assertEquals(0f, engineParentView.translationY) + } + + @Test + fun `GIVEN the toolbar is at the bottom && the navbar is enabled WHEN toolbar is being shifted THEN EngineView adjusts bottom clipping && EngineViewParent position doesn't change`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: ToolbarContainerView = mock() + Mockito.doReturn(Y_DOWN_TRANSITION).`when`(toolbar).translationY + + Assert.assertEquals(0f, engineParentView.translationY) + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = false, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + } + + // We want to position the engine view popup content + // right above the bottom toolbar when the toolbar + // is being shifted down. The top of the bottom toolbar + // is either positive or zero, but for clipping + // the values should be negative because the baseline + // for clipping is bottom toolbar height. + val bottomClipping = -Y_DOWN_TRANSITION.toInt() + Mockito.verify(engineView).setVerticalClipping(bottomClipping) + + Assert.assertEquals(0f, engineParentView.translationY) + } + + // Top toolbar position tests + @Test + fun `GIVEN the toolbar is at the top WHEN toolbar is being shifted THEN EngineView adjusts bottom clipping && EngineViewParent shifts as well`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: BrowserToolbar = mock() + Mockito.doReturn(Y_UP_TRANSITION).`when`(toolbar).translationY + + Assert.assertEquals(0f, engineParentView.translationY) + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = true, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + } + + Mockito.verify(engineView).setVerticalClipping(Y_UP_TRANSITION.toInt()) + + // Here we are adjusting the vertical position of + // the engine view container to be directly under + // the toolbar. The top toolbar is shifting up, so + // its translation will be either negative or zero. + val bottomClipping = Y_UP_TRANSITION + TOOLBAR_HEIGHT + Assert.assertEquals(bottomClipping, engineParentView.translationY) + } + + // Combined toolbar position tests + @Test + fun `WHEN both of the toolbars are being shifted GIVEN the toolbar is at the top && the navbar is enabled THEN EngineView adjusts bottom clipping`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: BrowserToolbar = mock() + val toolbarContainerView: ToolbarContainerView = mock() + Mockito.doReturn(Y_UP_TRANSITION).`when`(toolbar).translationY + Mockito.doReturn(Y_DOWN_TRANSITION).`when`(toolbarContainerView).translationY + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = true, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + onDependentViewChanged(mock(), mock(), toolbarContainerView) + } + + val doubleClipping = Y_UP_TRANSITION - Y_DOWN_TRANSITION + Mockito.verify(engineView).setVerticalClipping(doubleClipping.toInt()) + } + + @Test + fun `WHEN both of the toolbars are being shifted GIVEN the toolbar is at the top && the navbar is enabled THEN EngineViewParent shifts as well`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: BrowserToolbar = mock() + val toolbarContainerView: ToolbarContainerView = mock() + Mockito.doReturn(Y_UP_TRANSITION).`when`(toolbar).translationY + Mockito.doReturn(Y_DOWN_TRANSITION).`when`(toolbarContainerView).translationY + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = true, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + onDependentViewChanged(mock(), mock(), toolbarContainerView) + } + + // The top of the parent should be positioned right below the toolbar, + // so when we are given the new Y position of the top of the toolbar, + // which is always negative as the element is being "scrolled" out of + // the screen, the bottom of the toolbar is just a toolbar height away + // from it. + val parentTranslation = Y_UP_TRANSITION + TOOLBAR_HEIGHT + Assert.assertEquals(parentTranslation, engineParentView.translationY) + } + + // Edge cases + @Test + fun `GIVEN top toolbar is much bigger than bottom WHEN bottom stopped shifting && top is shifting THEN bottom clipping && engineParentView shifting is still accurate`() { + val largeYUpTransition = -500f + val largeTopToolbarHeight = 500 + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: BrowserToolbar = mock() + Mockito.doReturn(largeYUpTransition).`when`(toolbar).translationY + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = largeTopToolbarHeight, + hasTopToolbar = true, + ).apply { + this.engineView = engineView + this.recentBottomToolbarTranslation = Y_DOWN_TRANSITION + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + } + + val doubleClipping = largeYUpTransition - Y_DOWN_TRANSITION + Mockito.verify(engineView).setVerticalClipping(doubleClipping.toInt()) + + val parentTranslation = largeYUpTransition + largeTopToolbarHeight + Assert.assertEquals(parentTranslation, engineParentView.translationY) + } + + @Test + fun `GIVEN bottom toolbar is much bigger than top WHEN top stopped shifting && bottom is shifting THEN bottom clipping && engineParentView shifting is still accurate`() { + val largeYBottomTransition = 500f + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbarContainerView: ToolbarContainerView = mock() + Mockito.doReturn(largeYBottomTransition).`when`(toolbarContainerView).translationY + + EngineViewClippingBehavior( + context = mock(), + attrs = null, + engineViewParent = engineParentView, + topToolbarHeight = TOOLBAR_HEIGHT.toInt(), + hasTopToolbar = true, + ).apply { + this.engineView = engineView + this.recentTopToolbarTranslation = Y_UP_TRANSITION + }.run { + onDependentViewChanged(mock(), mock(), toolbarContainerView) + } + + val doubleClipping = Y_UP_TRANSITION - largeYBottomTransition + Mockito.verify(engineView).setVerticalClipping(doubleClipping.toInt()) + + val parentTranslation = Y_UP_TRANSITION + TOOLBAR_HEIGHT + Assert.assertEquals(parentTranslation, engineParentView.translationY) + } + + @Test + fun `GIVEN a bottom toolbar WHEN translation returns NaN THEN no exception thrown`() { + val engineView: EngineView = Mockito.spy(FakeEngineView(testContext)) + val engineParentView: View = Mockito.spy(View(testContext)) + val toolbar: View = mock() + Mockito.doReturn(100).`when`(toolbar).height + Mockito.doReturn(Float.NaN).`when`(toolbar).translationY + + EngineViewClippingBehavior( + mock(), + null, + engineParentView, + toolbar.height, + hasTopToolbar = false, + ).apply { + this.engineView = engineView + }.run { + onDependentViewChanged(mock(), mock(), toolbar) + } + + Assert.assertEquals(0f, engineView.asView().translationY) + } + + // General tests + @Test + fun `WHEN layoutDependsOn receives a class that isn't a ScrollableToolbar THEN it ignores it`() { + val behavior = EngineViewClippingBehavior( + mock(), + null, + mock(), + 0, + hasTopToolbar = false, + ) + + Assert.assertFalse(behavior.layoutDependsOn(mock(), mock(), TextView(testContext))) + Assert.assertFalse(behavior.layoutDependsOn(mock(), mock(), EditText(testContext))) + Assert.assertFalse(behavior.layoutDependsOn(mock(), mock(), ImageView(testContext))) + } + + @Test + fun `WHEN layoutDependsOn receives a class that is a ScrollableToolbar THEN it recognizes it as a dependency`() { + val behavior = EngineViewClippingBehavior( + mock(), + null, + mock(), + 0, + hasTopToolbar = false, + ) + + Assert.assertTrue(behavior.layoutDependsOn(mock(), mock(), BrowserToolbar(testContext))) + Assert.assertTrue(behavior.layoutDependsOn(mock(), mock(), ToolbarContainerView(testContext))) + } +} + +const val TOOLBAR_HEIGHT = 100f +const val Y_UP_TRANSITION = -42f +const val Y_DOWN_TRANSITION = -42f