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 ce42a1c24..5aa5ac704 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.browser import android.content.Context +import android.content.res.Configuration import android.os.StrictMode import android.view.View import android.view.ViewGroup @@ -52,6 +53,11 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { private var readerModeAvailable = false private var pwaOnboardingObserver: PwaOnboardingObserver? = null + private var forwardAction: BrowserToolbar.TwoStateButton? = null + private var backAction: BrowserToolbar.TwoStateButton? = null + private var refreshAction: BrowserToolbar.TwoStateButton? = null + private var isTablet: Boolean = false + @Suppress("LongMethod") override fun initializeUI(view: View, tab: SessionState) { super.initializeUI(view, tab) @@ -84,86 +90,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { browserToolbarView.view.addNavigationAction(homeAction) - if (resources.getBoolean(R.bool.tablet)) { - val enableTint = ThemeManager.resolveAttribute(R.attr.textPrimary, context) - val disableTint = ThemeManager.resolveAttribute(R.attr.textDisabled, context) - val backAction = BrowserToolbar.TwoStateButton( - primaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_back, - )!!, - primaryContentDescription = context.getString(R.string.browser_menu_back), - primaryImageTintResource = enableTint, - isInPrimaryState = { getCurrentTab()?.content?.canGoBack ?: false }, - secondaryImageTintResource = disableTint, - disableInSecondaryState = true, - longClickListener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Back(viewHistory = true), - ) - }, - listener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Back(viewHistory = false), - ) - }, - ) - browserToolbarView.view.addNavigationAction(backAction) - val forwardAction = BrowserToolbar.TwoStateButton( - primaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_forward, - )!!, - primaryContentDescription = context.getString(R.string.browser_menu_forward), - primaryImageTintResource = enableTint, - isInPrimaryState = { getCurrentTab()?.content?.canGoForward ?: false }, - secondaryImageTintResource = disableTint, - disableInSecondaryState = true, - longClickListener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Forward(viewHistory = true), - ) - }, - listener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Forward(viewHistory = false), - ) - }, - ) - browserToolbarView.view.addNavigationAction(forwardAction) - val refreshAction = BrowserToolbar.TwoStateButton( - primaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_refresh, - )!!, - primaryContentDescription = context.getString(R.string.browser_menu_refresh), - primaryImageTintResource = enableTint, - isInPrimaryState = { - getCurrentTab()?.content?.loading == false - }, - secondaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_stop, - )!!, - secondaryContentDescription = context.getString(R.string.browser_menu_stop), - disableInSecondaryState = false, - longClickListener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Reload(bypassCache = true), - ) - }, - listener = { - if (getCurrentTab()?.content?.loading == true) { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped(ToolbarMenu.Item.Stop) - } else { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Reload(bypassCache = false), - ) - } - }, - ) - browserToolbarView.view.addNavigationAction(refreshAction) - } + setScreenSize(isTablet = resources.getBoolean(R.bool.tablet)) val readerModeAction = BrowserToolbar.ToggleButton( @@ -243,6 +170,130 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { } } + private fun setScreenSize(isTablet: Boolean) { + if (isTablet == this.isTablet) return + + if (isTablet) { + addTabletActions(requireContext()) + } else { + removeTabletActions() + } + + this.isTablet = isTablet + } + + @Suppress("LongMethod") + private fun addTabletActions(context: Context) { + val enableTint = ThemeManager.resolveAttribute(R.attr.textPrimary, context) + val disableTint = ThemeManager.resolveAttribute(R.attr.textDisabled, context) + + if (backAction == null) { + backAction = BrowserToolbar.TwoStateButton( + primaryImage = AppCompatResources.getDrawable( + context, + R.drawable.mozac_ic_back, + )!!, + primaryContentDescription = context.getString(R.string.browser_menu_back), + primaryImageTintResource = enableTint, + isInPrimaryState = { getCurrentTab()?.content?.canGoBack ?: false }, + secondaryImageTintResource = disableTint, + disableInSecondaryState = true, + longClickListener = { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Back(viewHistory = true), + ) + }, + listener = { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Back(viewHistory = false), + ) + }, + ) + } + + backAction?.let { + browserToolbarView.view.addNavigationAction(it) + } + + if (forwardAction == null) { + forwardAction = BrowserToolbar.TwoStateButton( + primaryImage = AppCompatResources.getDrawable( + context, + R.drawable.mozac_ic_forward, + )!!, + primaryContentDescription = context.getString(R.string.browser_menu_forward), + primaryImageTintResource = enableTint, + isInPrimaryState = { getCurrentTab()?.content?.canGoForward ?: false }, + secondaryImageTintResource = disableTint, + disableInSecondaryState = true, + longClickListener = { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Forward(viewHistory = true), + ) + }, + listener = { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Forward(viewHistory = false), + ) + }, + ) + } + + forwardAction?.let { + browserToolbarView.view.addNavigationAction(it) + } + + if (refreshAction == null) { + refreshAction = BrowserToolbar.TwoStateButton( + primaryImage = AppCompatResources.getDrawable( + context, + R.drawable.mozac_ic_refresh, + )!!, + primaryContentDescription = context.getString(R.string.browser_menu_refresh), + primaryImageTintResource = enableTint, + isInPrimaryState = { + getCurrentTab()?.content?.loading == false + }, + secondaryImage = AppCompatResources.getDrawable( + context, + R.drawable.mozac_ic_stop, + )!!, + secondaryContentDescription = context.getString(R.string.browser_menu_stop), + disableInSecondaryState = false, + longClickListener = { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Reload(bypassCache = true), + ) + }, + listener = { + if (getCurrentTab()?.content?.loading == true) { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped(ToolbarMenu.Item.Stop) + } else { + browserToolbarInteractor.onBrowserToolbarMenuItemTapped( + ToolbarMenu.Item.Reload(bypassCache = false), + ) + } + }, + ) + } + + refreshAction?.let { + browserToolbarView.view.addNavigationAction(it) + } + } + + private fun removeTabletActions() { + forwardAction?.let { + browserToolbarView.view.removeNavigationAction(it) + } + backAction?.let { + browserToolbarView.view.removeNavigationAction(it) + } + refreshAction?.let { + browserToolbarView.view.removeNavigationAction(it) + } + } + override fun onStart() { super.onStart() val context = requireContext() @@ -396,4 +447,9 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { internal fun updateLastBrowseActivity() { requireContext().settings().lastBrowseActivity = System.currentTimeMillis() } + + override fun onConfigurationChanged(newConfig: Configuration) { + super.onConfigurationChanged(newConfig) + setScreenSize(isTablet = resources.getBoolean(R.bool.tablet)) + } } diff --git a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt index 2e34a9f79..f79056199 100644 --- a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt @@ -6,13 +6,18 @@ package org.mozilla.fenix.browser import android.content.Context import android.view.View +import androidx.appcompat.content.res.AppCompatResources import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry import androidx.navigation.NavController import io.mockk.every import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic import io.mockk.spyk +import io.mockk.unmockkObject +import io.mockk.unmockkStatic import io.mockk.verify import mozilla.components.browser.state.action.RestoreCompleteAction import mozilla.components.browser.state.action.TabListAction @@ -39,6 +44,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.onboarding.FenixOnboarding +import org.mozilla.fenix.theme.ThemeManager import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) @@ -317,6 +323,75 @@ class BrowserFragmentTest { verify(exactly = 1) { browserToolbarView.dismissMenu() } } + @Test + fun `WHEN fragment configuration screen size changes between tablet and mobile size THEN tablet action items added and removed`() { + val browserToolbar: BrowserToolbar = mockk(relaxed = true) + every { browserFragment.browserToolbarView.view } returns browserToolbar + + mockkObject(ThemeManager.Companion) + every { ThemeManager.resolveAttribute(any(), context) } returns mockk(relaxed = true) + + mockkStatic(AppCompatResources::class) + every { AppCompatResources.getDrawable(context, any()) } returns mockk() + + every { browserFragment.resources.getBoolean(R.bool.tablet) } returns true + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 3) { browserToolbar.addNavigationAction(any()) } + + every { browserFragment.resources.getBoolean(R.bool.tablet) } returns false + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 3) { browserToolbar.removeNavigationAction(any()) } + + unmockkObject(ThemeManager.Companion) + unmockkStatic(AppCompatResources::class) + } + + @Test + fun `WHEN fragment configuration change enables tablet size twice THEN tablet action items are only added once`() { + val browserToolbar: BrowserToolbar = mockk(relaxed = true) + every { browserFragment.browserToolbarView.view } returns browserToolbar + + mockkObject(ThemeManager.Companion) + every { ThemeManager.resolveAttribute(any(), context) } returns mockk(relaxed = true) + + mockkStatic(AppCompatResources::class) + every { AppCompatResources.getDrawable(context, any()) } returns mockk() + + every { browserFragment.resources.getBoolean(R.bool.tablet) } returns true + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 3) { browserToolbar.addNavigationAction(any()) } + + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 3) { browserToolbar.addNavigationAction(any()) } + + unmockkObject(ThemeManager.Companion) + unmockkStatic(AppCompatResources::class) + } + + @Test + fun `WHEN fragment configuration change sets mobile size twice THEN tablet action items are not added or removed`() { + val browserToolbar: BrowserToolbar = mockk(relaxed = true) + every { browserFragment.browserToolbarView.view } returns browserToolbar + + mockkObject(ThemeManager.Companion) + every { ThemeManager.resolveAttribute(any(), context) } returns mockk(relaxed = true) + + mockkStatic(AppCompatResources::class) + every { AppCompatResources.getDrawable(context, any()) } returns mockk() + + every { browserFragment.resources.getBoolean(R.bool.tablet) } returns false + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 0) { browserToolbar.addNavigationAction(any()) } + verify(exactly = 0) { browserToolbar.removeNavigationAction(any()) } + + browserFragment.onConfigurationChanged(mockk(relaxed = true)) + verify(exactly = 0) { browserToolbar.addNavigationAction(any()) } + verify(exactly = 0) { browserToolbar.removeNavigationAction(any()) } + + unmockkObject(ThemeManager.Companion) + unmockkStatic(AppCompatResources::class) + } + private fun addAndSelectTab(tab: TabSessionState) { store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() store.dispatch(TabListAction.SelectTabAction(tab.id)).joinBlocking()