From 1d795b4372ee6bacc36c6b4fd0e2f8c8d6acca22 Mon Sep 17 00:00:00 2001 From: mike a Date: Tue, 5 Mar 2024 10:37:33 -0800 Subject: [PATCH] Bug 1879377 - Hide nav bar when the toolbar becomes focused --- .../fenix/browser/BaseBrowserFragment.kt | 34 ++++++++++++------- .../fenix/components/appstate/AppAction.kt | 6 ++++ .../fenix/components/appstate/AppState.kt | 3 ++ .../components/appstate/AppStoreReducer.kt | 1 + .../navbar/BottomToolbarContainerView.kt | 9 ++--- .../toolbar/navbar/NavbarIntegration.kt | 19 +++++++++++ .../customtabs/ExternalAppBrowserFragment.kt | 4 +-- .../org/mozilla/fenix/home/HomeFragment.kt | 23 ++++++++++++- .../fenix/search/SearchDialogFragment.kt | 9 +++++ .../appstate/AppStoreReducerTest.kt | 21 ++++++++++++ .../toolbar/NavbarIntegrationTest.kt | 3 ++ 11 files changed, 110 insertions(+), 22 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 1add7a940..576a57958 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -60,7 +60,6 @@ import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.content.DownloadState import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.thumbnails.BrowserThumbnails -import mozilla.components.browser.toolbar.BrowserToolbar import mozilla.components.concept.base.crash.Breadcrumb import mozilla.components.concept.engine.permission.SitePermissions import mozilla.components.concept.engine.prompt.ShareData @@ -210,7 +209,9 @@ abstract class BaseBrowserFragment : internal val browserToolbarView: BrowserToolbarView get() = _browserToolbarView!! - internal lateinit var browserToolbar: BrowserToolbar + private var _bottomToolbarContainerView: BottomToolbarContainerView? = null + private val bottomToolbarContainerView: BottomToolbarContainerView + get() = _bottomToolbarContainerView!! protected val readerViewFeature = ViewBoundFeatureWrapper() protected val thumbnailsFeature = ViewBoundFeatureWrapper() @@ -441,9 +442,7 @@ abstract class BaseBrowserFragment : interactor = browserToolbarInteractor, customTabSession = customTabSessionId?.let { store.state.findCustomTab(it) }, lifecycleOwner = viewLifecycleOwner, - ).also { - browserToolbar = it.view - } + ) if (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) { val isToolbarAtBottom = context.components.settings.toolbarPosition == ToolbarPosition.BOTTOM @@ -452,6 +451,7 @@ abstract class BaseBrowserFragment : // We should remove it and add the view to the navigation bar container. // Should refactor this so there is no added view to remove to begin with: // https://bugzilla.mozilla.org/show_bug.cgi?id=1870976 + val browserToolbar = browserToolbarView.view if (isToolbarAtBottom) { binding.browserLayout.removeView(browserToolbar) } @@ -467,19 +467,26 @@ abstract class BaseBrowserFragment : ) } - BottomToolbarContainerView( + _bottomToolbarContainerView = BottomToolbarContainerView( context = context, parent = binding.browserLayout, androidToolbarView = if (isToolbarAtBottom) browserToolbar else null, menuButton = menuButton, isPrivateMode = activity.browsingModeManager.mode.isPrivate, - ).also { - navbarIntegration.set( - feature = it.navbarIntegration, - owner = this, - view = view, - ) - } + ) + + navbarIntegration.set( + feature = NavbarIntegration( + toolbar = bottomToolbarContainerView.toolbarContainerView, + store = requireComponents.core.store, + appStore = requireComponents.appStore, + viewLifecycleOwner = viewLifecycleOwner, + bottomToolbarContainerView = bottomToolbarContainerView, + sessionId = customTabSessionId, + ), + owner = this, + view = view, + ) } toolbarIntegration.set( @@ -1660,6 +1667,7 @@ abstract class BaseBrowserFragment : binding.engineView.setActivityContext(null) requireContext().accessibilityManager.removeAccessibilityStateChangeListener(this) + _bottomToolbarContainerView = null _browserToolbarView = null _browserToolbarInteractor = null _binding = null diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index a2089f351..dbdeb831e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -26,6 +26,7 @@ import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem import org.mozilla.fenix.library.history.PendingDeletionHistory import org.mozilla.fenix.messaging.MessagingState +import org.mozilla.fenix.search.SearchDialogFragment import org.mozilla.fenix.wallpapers.Wallpaper /** @@ -38,6 +39,11 @@ sealed class AppAction : Action { * Updates whether the first frame of the homescreen has been [drawn]. */ data class UpdateFirstFrameDrawn(val drawn: Boolean) : AppAction() + + /** + * Updates whether the [SearchDialogFragment] is visible. + */ + data class UpdateSearchDialogVisibility(val isVisible: Boolean) : AppAction() data class AddNonFatalCrash(val crash: NativeCodeCrash) : AppAction() data class RemoveNonFatalCrash(val crash: NativeCodeCrash) : AppAction() object RemoveAllNonFatalCrashes : AppAction() diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt index 30e141a27..e118dca12 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt @@ -24,6 +24,7 @@ import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem import org.mozilla.fenix.library.history.PendingDeletionHistory import org.mozilla.fenix.messaging.MessagingState +import org.mozilla.fenix.search.SearchDialogFragment import org.mozilla.fenix.wallpapers.WallpaperState /** @@ -33,6 +34,7 @@ import org.mozilla.fenix.wallpapers.WallpaperState * @property inactiveTabsExpanded A flag to know if the Inactive Tabs section of the Tabs Tray * should be expanded when the tray is opened. * @property firstFrameDrawn Flag indicating whether the first frame of the homescreen has been drawn. + * @property isSearchDialogVisible Flag indicating whether the user is interacting with the [SearchDialogFragment]. * @property nonFatalCrashes List of non-fatal crashes that allow the app to continue being used. * @property collections The list of [TabCollection] to display in the [HomeFragment]. * @property expandedCollections A set containing the ids of the [TabCollection] that are expanded @@ -63,6 +65,7 @@ data class AppState( val isForeground: Boolean = true, val inactiveTabsExpanded: Boolean = false, val firstFrameDrawn: Boolean = false, + val isSearchDialogVisible: Boolean = false, val nonFatalCrashes: List = emptyList(), val collections: List = emptyList(), val expandedCollections: Set = emptySet(), diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index deef63f40..a61468998 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -243,6 +243,7 @@ internal object AppStoreReducer { is AppAction.TabStripAction.UpdateLastTabClosed -> state.copy( wasLastTabClosedPrivate = action.private, ) + is AppAction.UpdateSearchDialogVisibility -> state.copy(isSearchDialogVisible = action.isVisible) } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt index 6d20f170a..5f37d54ce 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/BottomToolbarContainerView.kt @@ -35,7 +35,6 @@ 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 customTabSessionId Custom tab session ID. * * Defaults to [NavigationItems.defaultItems] which provides a standard set of navigation items. */ @@ -46,15 +45,13 @@ class BottomToolbarContainerView( androidToolbarView: View? = null, menuButton: MenuButton, isPrivateMode: Boolean = false, - customTabSessionId: String? = null, ) { - private val toolbarContainerView = ToolbarContainerView(context) - val navbarIntegration = - NavbarIntegration(toolbarContainerView, parent.context.components.core.store, customTabSessionId) + val toolbarContainerView = ToolbarContainerView(context) + val composeView: ComposeView init { - ComposeView(parent.context).apply { + composeView = ComposeView(context).apply { setContent { val tabCount = context.components.core.store.observeAsState(initialValue = 0) { browserState -> if (isPrivateMode) { diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavbarIntegration.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavbarIntegration.kt index c07d1dc3a..bdeaad456 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavbarIntegration.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/navbar/NavbarIntegration.kt @@ -5,10 +5,17 @@ package org.mozilla.fenix.components.toolbar.navbar import androidx.annotation.VisibleForTesting +import androidx.core.view.isVisible +import androidx.lifecycle.LifecycleOwner +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.distinctUntilChangedBy import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.toolbar.ScrollableToolbar import mozilla.components.feature.toolbar.ToolbarBehaviorController +import mozilla.components.lib.state.ext.flowScoped import mozilla.components.support.base.feature.LifecycleAwareFeature +import org.mozilla.fenix.components.AppStore /** * The feature responsible for scrolling behaviour of the navigation bar. @@ -18,17 +25,29 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature class NavbarIntegration( val toolbar: ScrollableToolbar, val store: BrowserStore, + val appStore: AppStore, + val viewLifecycleOwner: LifecycleOwner, + val bottomToolbarContainerView: BottomToolbarContainerView, sessionId: String?, ) : LifecycleAwareFeature { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) var toolbarController = ToolbarBehaviorController(toolbar, store, sessionId) + var scope: CoroutineScope? = null override fun start() { toolbarController.start() + + scope = appStore.flowScoped(viewLifecycleOwner) { flow -> + flow.distinctUntilChangedBy { it.isSearchDialogVisible } + .collect { state -> + bottomToolbarContainerView.composeView.isVisible = !state.isSearchDialogVisible + } + } } override fun stop() { toolbarController.stop() + scope?.cancel() } } diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt index 8e8f905a6..7f2a77a3e 100644 --- a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt @@ -66,7 +66,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment() { feature = CustomTabsIntegration( store = requireComponents.core.store, useCases = requireComponents.useCases.customTabsUseCases, - toolbar = browserToolbar, + toolbar = browserToolbarView.view, sessionId = customTabSessionId, activity = activity, onItemTapped = { browserToolbarInteractor.onBrowserToolbarMenuItemTapped(it) }, @@ -102,7 +102,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment() { } }, owner = this, - view = browserToolbar, + view = browserToolbarView.view, ) if (manifest != null) { diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index df62b5456..42baeb3f2 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -76,6 +76,7 @@ import mozilla.components.feature.top.sites.TopSitesFrecencyConfig import mozilla.components.feature.top.sites.TopSitesProviderConfig import mozilla.components.lib.state.ext.consumeFlow import mozilla.components.lib.state.ext.consumeFrom +import mozilla.components.lib.state.ext.flow import mozilla.components.service.glean.private.NoExtras import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.ui.colors.PhotonColors @@ -95,6 +96,7 @@ import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.components.toolbar.navbar.BottomToolbarContainerView +import org.mozilla.fenix.components.toolbar.navbar.NavbarIntegration import org.mozilla.fenix.databinding.FragmentHomeBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.containsQueryParameters @@ -156,6 +158,10 @@ class HomeFragment : Fragment() { ToolbarPosition.TOP -> null } + private var _bottomToolbarContainerView: BottomToolbarContainerView? = null + private val bottomToolbarContainerView: BottomToolbarContainerView + get() = _bottomToolbarContainerView!! + private val searchSelectorMenu by lazy { SearchSelectorMenu( context = requireContext(), @@ -223,6 +229,7 @@ class HomeFragment : Fragment() { private val historyMetadataFeature = ViewBoundFeatureWrapper() private val searchSelectorBinding = ViewBoundFeatureWrapper() private val searchSelectorMenuBinding = ViewBoundFeatureWrapper() + private val navbarIntegration = ViewBoundFeatureWrapper() override fun onCreate(savedInstanceState: Bundle?) { // DO NOT ADD ANYTHING ABOVE THIS getProfilerTime CALL! @@ -456,13 +463,26 @@ class HomeFragment : Fragment() { menuButton = WeakReference(menuButton), ).also { it.build() } - BottomToolbarContainerView( + _bottomToolbarContainerView = BottomToolbarContainerView( context = requireContext(), parent = binding.homeLayout, androidToolbarView = if (isToolbarAtBottom) binding.toolbarLayout else null, menuButton = menuButton, isPrivateMode = activity.browsingModeManager.mode.isPrivate, ) + + navbarIntegration.set( + feature = NavbarIntegration( + toolbar = bottomToolbarContainerView.toolbarContainerView, + store = requireComponents.core.store, + appStore = requireComponents.appStore, + viewLifecycleOwner = viewLifecycleOwner, + bottomToolbarContainerView = bottomToolbarContainerView, + sessionId = null, + ), + owner = this, + view = binding.root, + ) } sessionControlView = SessionControlView( @@ -802,6 +822,7 @@ class HomeFragment : Fragment() { sessionControlView = null tabCounterView = null toolbarView = null + _bottomToolbarContainerView = null _binding = null bundleArgs.clear() diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index 80f0e310a..aa07bb96e 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -82,6 +82,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.Core.Companion.BOOKMARKS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.Core.Companion.HISTORY_SEARCH_ENGINE_ID import org.mozilla.fenix.components.Core.Companion.TABS_SEARCH_ENGINE_ID +import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.databinding.FragmentSearchDialogBinding import org.mozilla.fenix.databinding.SearchSuggestionsHintBinding @@ -171,6 +172,10 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { toolbarView.view.showKeyboard() } } + + requireComponents.appStore.dispatch( + AppAction.UpdateSearchDialogVisibility(isVisible = true), + ) } override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { @@ -597,6 +602,10 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { if (!dialogHandledAction) { requireComponents.core.store.dispatch(AwesomeBarAction.EngagementFinished(abandoned = true)) } + + requireComponents.appStore.dispatch( + AppAction.UpdateSearchDialogVisibility(isVisible = false), + ) } override fun onBackPressed(): Boolean { diff --git a/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt b/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt index 2997e14c0..f1b784936 100644 --- a/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt @@ -116,4 +116,25 @@ class AppStoreReducerTest { assertFalse(updatedState.mode.isPrivate) } + + @Test + fun `WHEN UpdateSearchDialogVisibility is called THEN isSearchDialogVisible gets updated`() { + val initialState = AppState() + + assertFalse(initialState.isSearchDialogVisible) + + var updatedState = AppStoreReducer.reduce( + initialState, + AppAction.UpdateSearchDialogVisibility(isVisible = true), + ) + + assertTrue(updatedState.isSearchDialogVisible) + + updatedState = AppStoreReducer.reduce( + initialState, + AppAction.UpdateSearchDialogVisibility(isVisible = false), + ) + + assertFalse(updatedState.isSearchDialogVisible) + } } diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/NavbarIntegrationTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/NavbarIntegrationTest.kt index ba308bcea..a620417fa 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/NavbarIntegrationTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/NavbarIntegrationTest.kt @@ -21,6 +21,9 @@ class NavbarIntegrationTest { feature = NavbarIntegration( toolbar = mockk(), store = mockk(), + appStore = mockk(), + viewLifecycleOwner = mockk(), + bottomToolbarContainerView = mockk(), sessionId = null, ).apply { toolbarController = mockk(relaxed = true)