From b36431a6df7bdbe15d3176d20d7302f0989cfa29 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Tue, 30 Mar 2021 11:48:49 +0300 Subject: [PATCH] For #18616 - Update browser and toolbar layout when toolbar is at top FindInPageIntegration which already updated the toolbar to make room for the find in page bar now receives more data based on which it will be able to better update the layout of BrowserFragment to to support showing the find in page bar. --- .../fenix/browser/BaseBrowserFragment.kt | 8 +- .../fenix/components/FindInPageIntegration.kt | 75 +++++- .../components/FindInPageIntegrationTest.kt | 255 ++++++++++++++++++ 3 files changed, 331 insertions(+), 7 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/components/FindInPageIntegrationTest.kt 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 78eb832706..7d80abbddb 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -371,8 +371,12 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit store = store, sessionId = customTabSessionId, stub = view.stubFindInPage, - engineView = view.engineView, - toolbar = browserToolbarView.view + engineView = engineView, + toolbarInfo = FindInPageIntegration.ToolbarInfo( + browserToolbarView.view, + !context.settings().shouldUseFixedTopToolbar && context.settings().isDynamicToolbarEnabled, + !context.settings().shouldUseBottomToolbar + ) ), owner = this, view = view diff --git a/app/src/main/java/org/mozilla/fenix/components/FindInPageIntegration.kt b/app/src/main/java/org/mozilla/fenix/components/FindInPageIntegration.kt index 1f03bb8d4a..bf3f6cab5a 100644 --- a/app/src/main/java/org/mozilla/fenix/components/FindInPageIntegration.kt +++ b/app/src/main/java/org/mozilla/fenix/components/FindInPageIntegration.kt @@ -5,7 +5,10 @@ package org.mozilla.fenix.components import android.view.View +import android.view.ViewGroup.MarginLayoutParams import android.view.ViewStub +import androidx.annotation.VisibleForTesting +import androidx.core.view.isVisible import mozilla.components.browser.state.selector.findCustomTabOrSelectedTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.toolbar.BrowserToolbar @@ -13,30 +16,92 @@ import mozilla.components.concept.engine.EngineView import mozilla.components.feature.findinpage.FindInPageFeature import mozilla.components.feature.findinpage.view.FindInPageView import mozilla.components.support.base.feature.LifecycleAwareFeature +import org.mozilla.fenix.components.FindInPageIntegration.ToolbarInfo import org.mozilla.fenix.utils.Mockable +/** + * BrowserFragment delegate to handle all layout updates needed to show or hide the find in page bar. + * + * @param store [BrowserStore] + * @param sessionId ID of the [store] session in which the query will be performed. + * @param engineView the browser in which the queries will be made and which needs to be better positioned + * to suit the find in page bar. + * @param toolbarInfo [ToolbarInfo] used to configure the [BrowserToolbar] while the find in page bar is shown. + */ @Mockable class FindInPageIntegration( private val store: BrowserStore, private val sessionId: String? = null, stub: ViewStub, private val engineView: EngineView, - private val toolbar: BrowserToolbar + private val toolbarInfo: ToolbarInfo ) : InflationAwareFeature(stub) { override fun onViewInflated(view: View): LifecycleAwareFeature { return FindInPageFeature(store, view as FindInPageView, engineView) { - toolbar.visibility = View.VISIBLE + restorePreviousLayout() + view.visibility = View.GONE } } override fun onLaunch(view: View, feature: LifecycleAwareFeature) { store.state.findCustomTabOrSelectedTab(sessionId)?.let { tab -> - // Always hide the toolbar and display find in page query - toolbar.visibility = View.GONE + prepareLayoutForFindBar() + view.visibility = View.VISIBLE (feature as FindInPageFeature).bind(tab) - view.layoutParams.height = toolbar.height + view.layoutParams.height = toolbarInfo.toolbar.height } } + + @VisibleForTesting + internal fun restorePreviousLayout() { + toolbarInfo.toolbar.isVisible = true + + val engineViewParent = getEngineViewParent() + val engineViewParentParams = getEngineViewsParentLayoutParams() + if (toolbarInfo.isToolbarPlacedAtTop) { + if (toolbarInfo.isToolbarDynamic) { + engineViewParent.translationY = toolbarInfo.toolbar.height.toFloat() + engineViewParentParams.bottomMargin = 0 + } else { + engineViewParent.translationY = 0f + } + } + } + + @VisibleForTesting + internal fun prepareLayoutForFindBar() { + toolbarInfo.toolbar.isVisible = false + + val engineViewParent = getEngineViewParent() + val engineViewParentParams = getEngineViewsParentLayoutParams() + if (toolbarInfo.isToolbarPlacedAtTop) { + if (toolbarInfo.isToolbarDynamic) { + // With a dynamic toolbar the EngineView extends to the entire (top and bottom) of the screen. + // And now with the toolbar expanded it is translated down immediately below the toolbar. + engineViewParent.translationY = 0f + engineViewParentParams.bottomMargin = toolbarInfo.toolbar.height + } else { + // With a fixed toolbar the EngineView is anchored below the toolbar with 0 Y translation. + engineViewParent.translationY = -toolbarInfo.toolbar.height.toFloat() + } + } + } + + @VisibleForTesting + internal fun getEngineViewParent() = engineView.asView().parent as View + + @VisibleForTesting + internal fun getEngineViewsParentLayoutParams() = getEngineViewParent().layoutParams as MarginLayoutParams + + /** + * Holder of all details needed about the Toolbar. + * Used to modify the layout of BrowserToolbar while the find in page bar is shown. + */ + data class ToolbarInfo( + val toolbar: BrowserToolbar, + val isToolbarDynamic: Boolean, + val isToolbarPlacedAtTop: Boolean + ) } diff --git a/app/src/test/java/org/mozilla/fenix/components/FindInPageIntegrationTest.kt b/app/src/test/java/org/mozilla/fenix/components/FindInPageIntegrationTest.kt new file mode 100644 index 0000000000..d1fbaab3aa --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/components/FindInPageIntegrationTest.kt @@ -0,0 +1,255 @@ +/* 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.view.View +import android.view.ViewGroup +import android.widget.FrameLayout +import androidx.core.view.isVisible +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import mozilla.components.browser.engine.gecko.GeckoEngineView +import mozilla.components.browser.toolbar.BrowserToolbar +import mozilla.components.concept.engine.EngineView +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Test + +class FindInPageIntegrationTest { + // For ease of tests naming "find in page bar" is referred to as FIPB. + + @Test + fun `GIVEN FIPB not shown WHEN prepareLayoutForFindBar is called for a dynamic top toolbar THEN toolbar is hidden and browser translated up`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = true, + isToolbarPlacedAtTop = true + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.prepareLayoutForFindBar() + + verify { toolbar.isVisible = false } + verify { engineViewParent.translationY = 0f } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(123, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB not shown WHEN prepareLayoutForFindBar is called for a fixed top toolbar THEN toolbar is hidden and browser translated up`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = false, + isToolbarPlacedAtTop = true + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.prepareLayoutForFindBar() + + verify { toolbar.isVisible = false } + verify { engineViewParent.translationY = -123f } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So a real instance is used to then assert on the value actually set. + assertEquals(0, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB shown WHEN restorePreviousLayout is called for a dynamic top toolbar THEN toolbar is shown and browser translated down`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = true, + isToolbarPlacedAtTop = true + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.restorePreviousLayout() + + verify { toolbar.isVisible = true } + verify { engineViewParent.translationY = 123f } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(0, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB shown WHEN restorePreviousLayout is called for a fixed top toolbar THEN toolbar is shown and browser translated down`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = false, + isToolbarPlacedAtTop = true + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.restorePreviousLayout() + + verify { toolbar.isVisible = true } + verify { engineViewParent.translationY = 0f } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(0, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB not shown WHEN prepareLayoutForFindBar is called for a dynamic bottom toolbar THEN toolbar is hidden and browser is made smaller`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = true, + isToolbarPlacedAtTop = false + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.prepareLayoutForFindBar() + + verify { toolbar.isVisible = false } + verify(exactly = 0) { engineViewParent.translationY = any() } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(123, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB not shown WHEN prepareLayoutForFindBar is called for a fixed bottom toolbar THEN toolbar is hidden and browser remains the same`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = false, + isToolbarPlacedAtTop = false + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.prepareLayoutForFindBar() + + verify { toolbar.isVisible = false } + verify(exactly = 0) { engineViewParent.translationY = any() } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(123, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB shown WHEN restorePreviousLayout is called for a dynamic bottom toolbar THEN toolbar is shown and browser is made bigger`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = true, + isToolbarPlacedAtTop = false + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.restorePreviousLayout() + + verify { toolbar.isVisible = true } + verify(exactly = 0) { engineViewParent.translationY = any() } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(0, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FIPB shown WHEN restorePreviousLayout is called for a fixed bottom toolbar THEN toolbar is shown and browser remains the same`() { + val toolbar: BrowserToolbar = mockk(relaxed = true) { + every { height } returns 123 + } + val engineViewParent: FrameLayout = mockk(relaxed = true) + val engineViewParentParams = ViewGroup.MarginLayoutParams(100, 100) + val toolbarInfo = FindInPageIntegration.ToolbarInfo( + toolbar = toolbar, + isToolbarDynamic = true, + isToolbarPlacedAtTop = false + ) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo)) { + every { getEngineViewsParentLayoutParams() } returns engineViewParentParams + every { getEngineViewParent() } returns engineViewParent + } + + feature.restorePreviousLayout() + + verify { toolbar.isVisible = true } + verify(exactly = 0) { engineViewParent.translationY = any() } + // MockKException: Missing calls inside verify { ... } block if verifying the bottomMargin setter on a mockk + // So I used a real instance and assert on the value actually set. + assertEquals(0, engineViewParentParams.bottomMargin) + } + + @Test + fun `GIVEN FindInPageIntegration WHEN getEngineViewParent is called THEN it returns EngineView's layout parent`() { + val parent: FrameLayout = mockk() + val engineView: GeckoEngineView = mockk(relaxed = true) + every { (engineView as EngineView).asView().parent } returns parent + + val feature = FindInPageIntegration(mockk(), null, mockk(), engineView, mockk()) + + assertSame(parent as View, feature.getEngineViewParent()) + } + + @Test + fun `GIVEN FindInPageIntegration WHEN getEngineViewsParentLayoutParams is called THEN it returns EngineView's layout parent MarginLayoutParams`() { + val parent: FrameLayout = mockk(relaxed = true) { + every { layoutParams } returns mockk(relaxed = true) + } + val engineView: GeckoEngineView = mockk(relaxed = true) + val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), engineView, mockk())) + every { feature.getEngineViewParent() } returns parent + + assertSame(parent.layoutParams, feature.getEngineViewsParentLayoutParams()) + } +}