From 75aa2d413adc44fa5d247fdf90f03530a2141683 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Fri, 11 Sep 2020 18:13:02 +0300 Subject: [PATCH] For #14974 - Ensure website bottom elements stay at bottom engineView.setDynamicToolbarMaxHeight(0) vs engineView.setDynamicToolbarMaxHeight(toolbarHeight) ensures webpage's bottom elements are aligned to the bottom of the browser. We also need to make sure that when the toolbar is static it does not cover the bottom of the page - something desired when the toolbar was dynamic. For this the engineView will have a toolbarHeight bottom margin. --- .../fenix/browser/BaseBrowserFragment.kt | 41 +++++++++++++------ .../toolbar/BrowserToolbarController.kt | 5 ++- .../DefaultBrowserToolbarControllerTest.kt | 14 ++++++- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 315f20704..c05a5bf8a 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -770,20 +770,35 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } private fun initializeEngineView(toolbarHeight: Int) { - engineView.setDynamicToolbarMaxHeight(toolbarHeight) - val context = requireContext() - val behavior = when (context.settings().toolbarPosition) { - ToolbarPosition.BOTTOM -> EngineViewBottomBehavior(context, null) - ToolbarPosition.TOP -> SwipeRefreshScrollingViewBehavior( - context, - null, - engineView, - browserToolbarView - ) - } - (swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams).behavior = behavior + if (context.settings().isDynamicToolbarEnabled) { + engineView.setDynamicToolbarMaxHeight(toolbarHeight) + + val behavior = when (context.settings().toolbarPosition) { + // Set engineView dynamic vertical clipping depending on the toolbar position. + ToolbarPosition.BOTTOM -> EngineViewBottomBehavior(context, null) + // Set scroll flags depending on if if the browser or the website is doing the scroll. + ToolbarPosition.TOP -> SwipeRefreshScrollingViewBehavior( + context, + null, + engineView, + browserToolbarView + ) + } + + (swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams).behavior = behavior + } else { + // Ensure webpage's bottom elements are aligned to the very bottom of the engineView. + engineView.setDynamicToolbarMaxHeight(0) + + // Effectively place the engineView on top of the toolbar if that is not dynamic. + if (context.settings().shouldUseBottomToolbar) { + val browserEngine = swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams + browserEngine.bottomMargin = + requireContext().resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) + } + } } /** @@ -1097,7 +1112,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session if (webAppToolbarShouldBeVisible) { browserToolbarView.view.isVisible = true val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) - engineView.setDynamicToolbarMaxHeight(toolbarHeight) + initializeEngineView(toolbarHeight) } } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index 86f32927c..5b70eae28 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -22,6 +22,7 @@ import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.sessionsOfType +import org.mozilla.fenix.ext.settings /** * An interface that handles the view manipulation of the BrowserToolbar, triggered by the Interactor @@ -156,7 +157,9 @@ class DefaultBrowserToolbarController( } override fun handleScroll(offset: Int) { - engineView.setVerticalClipping(offset) + if (activity.settings().isDynamicToolbarEnabled) { + engineView.setVerticalClipping(offset) + } } companion object { diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index a25367093..124cf312a 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt @@ -37,6 +37,7 @@ import org.mozilla.fenix.browser.readermode.ReaderModeController import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) @@ -241,12 +242,23 @@ class DefaultBrowserToolbarControllerTest { } @Test - fun handleScroll() { + fun `handleScroll for dynamic toolbars`() { val controller = createController() + every { activity.settings().isDynamicToolbarEnabled } returns true + controller.handleScroll(10) verify { engineView.setVerticalClipping(10) } } + @Test + fun `handleScroll for static toolbars`() { + val controller = createController() + every { activity.settings().isDynamicToolbarEnabled } returns false + + controller.handleScroll(10) + verify(exactly = 0) { engineView.setVerticalClipping(10) } + } + private fun createController( activity: HomeActivity = this.activity, customTabSession: Session? = null,