diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index 6ca7ed709..35018c5db 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -63,6 +63,7 @@ import org.mozilla.fenix.library.history.state.HistoryNavigationMiddleware import org.mozilla.fenix.library.history.state.HistoryStorageMiddleware import org.mozilla.fenix.library.history.state.HistorySyncMiddleware import org.mozilla.fenix.library.history.state.HistoryTelemetryMiddleware +import org.mozilla.fenix.library.history.state.bindings.MenuBinding import org.mozilla.fenix.library.history.state.bindings.PendingDeletionBinding import org.mozilla.fenix.tabstray.Page import org.mozilla.fenix.utils.allowUndo @@ -95,6 +96,13 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, PendingDeletionBinding(requireContext().components.appStore, historyView) } + private val menuBinding by lazy { + MenuBinding( + store = historyStore, + invalidateOptionsMenu = { activity?.invalidateOptionsMenu() }, + ) + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -110,6 +118,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, HistoryNavigationMiddleware( navController = findNavController(), openToBrowser = ::openItem, + onBackPressed = requireActivity().onBackPressedDispatcher::onBackPressed, ), HistoryTelemetryMiddleware( isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private, @@ -263,10 +272,12 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, private fun startStateBindings() { pendingDeletionBinding.start() + menuBinding.start() } private fun stopStateBindings() { pendingDeletionBinding.stop() + menuBinding.stop() } private fun updateDeleteMenuItemView(isEnabled: Boolean) { @@ -414,8 +425,12 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, } } - override fun onBackPressed(): Boolean { - return historyView.onBackPressed() + override fun onBackPressed() = if (FeatureFlags.historyFragmentLibStateRefactor) { + // The state needs to be updated accordingly if Edit mode is active + historyStore.dispatch(HistoryFragmentAction.BackPressed) + true + } else { + historyView.onBackPressed() } override fun onDestroyView() { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt index 00d727fcd..6927a4f53 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt @@ -149,6 +149,16 @@ sealed class HistoryFragmentAction : Action { */ data class DeleteItems(val items: Set) : HistoryFragmentAction() + /** + * The user has clicked to enter the Recently Closed fragment. + */ + object EnterRecentlyClosed : HistoryFragmentAction() + + /** + * A back press event has been dispatched. + */ + object BackPressed : HistoryFragmentAction() + /** * Updates the empty state of [org.mozilla.fenix.library.history.HistoryView]. */ @@ -259,10 +269,18 @@ private fun historyStateReducer( ) } } + is HistoryFragmentAction.BackPressed -> { + if (state.mode is HistoryFragmentState.Mode.Editing) { + state.copy(mode = HistoryFragmentState.Mode.Normal) + } else { + state + } + } // For deletion actions: the item list is handled through storage. // Updates from storage are dispatched directly to the view. is HistoryFragmentAction.DeleteItems, is HistoryFragmentAction.DeleteTimeRange, + is HistoryFragmentAction.EnterRecentlyClosed, -> state } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt index d454027c8..1a958fd57 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt @@ -99,7 +99,7 @@ class HistoryView( val last = layoutManager.findLastVisibleItemPosition() + 1 historyAdapter.notifyItemRangeChanged(first, last - first) - if (state.mode::class != oldMode::class) { + if (state.mode::class != oldMode::class && !FeatureFlags.historyFragmentLibStateRefactor) { interactor.onModeSwitched() } @@ -135,7 +135,11 @@ class HistoryView( with(binding.recentlyClosedNavEmpty) { recentlyClosedNav.setOnClickListener { - interactor.onRecentlyClosedClicked() + if (FeatureFlags.historyFragmentLibStateRefactor) { + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) + } else { + interactor.onRecentlyClosedClicked() + } } val numRecentTabs = recentlyClosedNav.context.components.core.store.state.closedTabs.size recentlyClosedTabsDescription.text = String.format( diff --git a/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddleware.kt b/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddleware.kt index b492cd936..13fd0386a 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddleware.kt @@ -27,6 +27,7 @@ import org.mozilla.fenix.library.history.HistoryFragmentState class HistoryNavigationMiddleware( private val navController: NavController, private val openToBrowser: (item: History.Regular) -> Unit, + private val onBackPressed: () -> Unit, private val scope: CoroutineScope = CoroutineScope(Dispatchers.Main), ) : Middleware { override fun invoke( @@ -40,6 +41,18 @@ class HistoryNavigationMiddleware( next(action) scope.launch { when (action) { + is HistoryFragmentAction.EnterRecentlyClosed -> { + navController.navigate( + HistoryFragmentDirections.actionGlobalRecentlyClosed(), + NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(), + ) + } + is HistoryFragmentAction.BackPressed -> { + // When editing, we override the back pressed event to update the mode. + if (currentState.mode !is HistoryFragmentState.Mode.Editing) { + onBackPressed() + } + } is HistoryFragmentAction.HistoryItemClicked -> { if (currentState.mode.selectedItems.isEmpty()) { when (val item = action.item) { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddleware.kt index 8ac7b7905..3d4f4fe91 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddleware.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.library.history.state import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext import mozilla.components.service.glean.private.NoExtras +import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryFragmentAction import org.mozilla.fenix.library.history.HistoryFragmentState @@ -30,6 +31,9 @@ class HistoryTelemetryMiddleware( val currentState = context.state next(action) when (action) { + is HistoryFragmentAction.EnterRecentlyClosed -> { + Events.recentlyClosedTabsOpened.record(NoExtras()) + } is HistoryFragmentAction.HistoryItemClicked -> { if (currentState.mode.selectedItems.isEmpty()) { when (val item = action.item) { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/state/bindings/MenuBinding.kt b/app/src/main/java/org/mozilla/fenix/library/history/state/bindings/MenuBinding.kt new file mode 100644 index 000000000..dd792a727 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/history/state/bindings/MenuBinding.kt @@ -0,0 +1,24 @@ +/* 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.library.history.state.bindings + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChangedBy +import mozilla.components.lib.state.helpers.AbstractBinding +import org.mozilla.fenix.library.history.HistoryFragmentState +import org.mozilla.fenix.library.history.HistoryFragmentStore + +/** + * A binding to map state updates to menu updates. + */ +class MenuBinding( + store: HistoryFragmentStore, + val invalidateOptionsMenu: () -> Unit, +) : AbstractBinding(store) { + override suspend fun onState(flow: Flow) { + flow.distinctUntilChangedBy { it.mode } + .collect { invalidateOptionsMenu() } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt index 3da3aeaf1..9842cd9c7 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt @@ -33,7 +33,11 @@ class HistoryListItemViewHolder( init { binding.recentlyClosedNavEmpty.recentlyClosedNav.setOnClickListener { - historyInteractor.onRecentlyClosedClicked() + if (FeatureFlags.historyFragmentLibStateRefactor) { + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed) + } else { + historyInteractor.onRecentlyClosedClicked() + } } binding.historyLayout.overflowView.apply { diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt index 40c259bf1..dae2a0598 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt @@ -185,6 +185,30 @@ class HistoryFragmentStoreTest { assertTrue(store.state.mode.selectedItems.contains(historyItem)) } + @Test + fun `GIVEN mode is editing WHEN back pressed THEN mode becomes normal`() { + val store = HistoryFragmentStore( + emptyDefaultState().copy( + mode = HistoryFragmentState.Mode.Editing( + setOf(), + ), + ), + ) + + store.dispatch(HistoryFragmentAction.BackPressed).joinBlocking() + + assertEquals(HistoryFragmentState.Mode.Normal, store.state.mode) + } + + @Test + fun `GIVEN mode is not editing WHEN back pressed THEN does not change`() { + val store = HistoryFragmentStore(emptyDefaultState().copy(mode = HistoryFragmentState.Mode.Syncing)) + + store.dispatch(HistoryFragmentAction.BackPressed).joinBlocking() + + assertEquals(HistoryFragmentState.Mode.Syncing, store.state.mode) + } + private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Normal, diff --git a/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddlewareTest.kt index d2d5c38f1..42d6e2488 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddlewareTest.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.library.history.state import androidx.navigation.NavController +import androidx.navigation.NavOptions import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import mozilla.components.support.test.any @@ -17,8 +18,10 @@ import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.mockito.Mockito.verify +import org.mozilla.fenix.R import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryFragmentAction +import org.mozilla.fenix.library.history.HistoryFragmentDirections import org.mozilla.fenix.library.history.HistoryFragmentState import org.mozilla.fenix.library.history.HistoryFragmentStore import org.mozilla.fenix.library.history.HistoryItemTimeGroup @@ -39,6 +42,7 @@ class HistoryNavigationMiddlewareTest { openedInBrowser = true } }, + onBackPressed = { }, scope = this, ) val store = @@ -62,6 +66,7 @@ class HistoryNavigationMiddlewareTest { openedInBrowser = true } }, + onBackPressed = { }, scope = this, ) val state = HistoryFragmentState.initial.copy( @@ -85,6 +90,7 @@ class HistoryNavigationMiddlewareTest { val middleware = HistoryNavigationMiddleware( navController = navController, openToBrowser = { }, + onBackPressed = { }, scope = this, ) val store = @@ -98,4 +104,68 @@ class HistoryNavigationMiddlewareTest { navOptions = any(), ) } + + @Test + fun `WHEN recently closed is requested to be entered THEN nav controller navigates to it`() = runTest { + val navController = mock() + val middleware = HistoryNavigationMiddleware( + navController = navController, + openToBrowser = { }, + onBackPressed = { }, + scope = this, + ) + val store = + HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware)) + + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed).joinBlocking() + advanceUntilIdle() + + verify(navController).navigate( + HistoryFragmentDirections.actionGlobalRecentlyClosed(), + NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(), + ) + } + + @Test + fun `GIVEN mode is editing WHEN back pressed THEN no navigation happens`() = runTest { + var onBackPressed = false + val middleware = HistoryNavigationMiddleware( + navController = mock(), + openToBrowser = { }, + onBackPressed = { onBackPressed = true }, + scope = this, + ) + val store = + HistoryFragmentStore( + HistoryFragmentState.initial.copy( + mode = HistoryFragmentState.Mode.Editing( + setOf(), + ), + ), + middleware = listOf(middleware), + ) + + store.dispatch(HistoryFragmentAction.BackPressed).joinBlocking() + advanceUntilIdle() + + assertFalse(onBackPressed) + } + + @Test + fun `GIVEN mode is not editing WHEN back pressed THEN onBackPressed callback invoked`() = runTest { + var onBackPressed = false + val middleware = HistoryNavigationMiddleware( + navController = mock(), + openToBrowser = { }, + onBackPressed = { onBackPressed = true }, + scope = this, + ) + val store = + HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware)) + + store.dispatch(HistoryFragmentAction.BackPressed).joinBlocking() + advanceUntilIdle() + + assertTrue(onBackPressed) + } } diff --git a/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddlewareTest.kt index 4872d471c..d4f5a02a1 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddlewareTest.kt @@ -12,6 +12,7 @@ import org.junit.Assert.assertNull import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryFragmentAction import org.mozilla.fenix.library.history.HistoryFragmentState @@ -108,4 +109,16 @@ class HistoryTelemetryMiddlewareTest { assertNotNull(GleanHistory.removedAll.testGetValue()) } + + @Test + fun `GIVEN recently closed is requested to be entered THEN telemetry recorded`() { + val store = HistoryFragmentStore( + initialState = HistoryFragmentState.initial, + middleware = listOf(middleware), + ) + + store.dispatch(HistoryFragmentAction.EnterRecentlyClosed).joinBlocking() + + assertNotNull(Events.recentlyClosedTabsOpened.testGetValue()) + } } diff --git a/app/src/test/java/org/mozilla/fenix/library/history/state/bindings/MenuBindingTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/state/bindings/MenuBindingTest.kt new file mode 100644 index 000000000..4ea81c20f --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/history/state/bindings/MenuBindingTest.kt @@ -0,0 +1,34 @@ +/* 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.library.history.state.bindings + +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.rule.MainCoroutineRule +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.mozilla.fenix.library.history.HistoryFragmentAction +import org.mozilla.fenix.library.history.HistoryFragmentState +import org.mozilla.fenix.library.history.HistoryFragmentStore + +class MenuBindingTest { + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + + @Test + fun `WHEN the mode is updated THEN the menu is invalidated`() { + var menuInvalidated = false + val store = HistoryFragmentStore(HistoryFragmentState.initial.copy(mode = HistoryFragmentState.Mode.Syncing)) + val binding = MenuBinding( + store = store, + invalidateOptionsMenu = { menuInvalidated = true }, + ) + + binding.start() + store.dispatch(HistoryFragmentAction.FinishSync).joinBlocking() + + assertTrue(menuInvalidated) + } +}