Bug 1842245 - Refactor history fragment item click handlers to lib-state

fenix/117.0
MatthewTighe 11 months ago committed by mergify[bot]
parent 31b5683a76
commit 7e97ccb6f1

@ -16,6 +16,7 @@ import org.mozilla.fenix.selection.SelectionHolder
*/
class HistoryAdapter(
private val historyInteractor: HistoryInteractor,
private val store: HistoryFragmentStore,
private val onEmptyStateChanged: (Boolean) -> Unit,
) : PagingDataAdapter<History, HistoryListItemViewHolder>(historyDiffCallback),
SelectionHolder<History> {
@ -38,7 +39,12 @@ class HistoryAdapter(
override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): HistoryListItemViewHolder {
val view = LayoutInflater.from(parent.context).inflate(viewType, parent, false)
return HistoryListItemViewHolder(view, historyInteractor, this)
return HistoryListItemViewHolder(
view = view,
historyInteractor = historyInteractor,
selectionHolder = this,
store = store,
)
}
fun updateMode(mode: HistoryFragmentState.Mode) {

@ -41,6 +41,7 @@ import mozilla.components.support.base.feature.UserInteractionHandler
import mozilla.components.support.ktx.kotlin.toShortUrl
import mozilla.telemetry.glean.private.NoExtras
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.NavHostActivity
import org.mozilla.fenix.R
@ -55,7 +56,10 @@ import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.runIfFragmentIsAttached
import org.mozilla.fenix.ext.setTextColor
import org.mozilla.fenix.home.Mode
import org.mozilla.fenix.library.LibraryPageFragment
import org.mozilla.fenix.library.history.state.HistoryNavigationMiddleware
import org.mozilla.fenix.library.history.state.HistoryTelemetryMiddleware
import org.mozilla.fenix.tabstray.Page
import org.mozilla.fenix.utils.allowUndo
import org.mozilla.fenix.GleanMetrics.History as GleanHistory
@ -92,12 +96,15 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
val view = binding.root
historyStore = StoreProvider.get(this) {
HistoryFragmentStore(
HistoryFragmentState(
items = listOf(),
mode = HistoryFragmentState.Mode.Normal,
pendingDeletionItems = emptySet(),
isEmpty = false,
isDeletingItems = false,
initialState = HistoryFragmentState.initial,
middleware = listOf(
HistoryNavigationMiddleware(
navController = findNavController(),
openToBrowser = ::openItem,
),
HistoryTelemetryMiddleware(
isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private,
),
),
)
}
@ -128,6 +135,7 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
HistoryFragmentAction.ChangeEmptyState(isEmpty = true),
)
},
store = historyStore,
onEmptyStateChanged = {
historyStore.dispatch(
HistoryFragmentAction.ChangeEmptyState(it),
@ -353,13 +361,16 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
}
private fun openItem(item: History.Regular) {
GleanHistory.openedItem.record(
GleanHistory.OpenedItemExtra(
isRemote = item.isRemote,
timeGroup = item.historyTimeGroup.toString(),
isPrivate = (activity as HomeActivity).browsingModeManager.mode == BrowsingMode.Private,
),
)
// This telemetry is recorded by the middleware if the refactor is enabled
if (!FeatureFlags.historyFragmentLibStateRefactor) {
GleanHistory.openedItem.record(
GleanHistory.OpenedItemExtra(
isRemote = item.isRemote,
timeGroup = item.historyTimeGroup.toString(),
isPrivate = (activity as HomeActivity).browsingModeManager.mode == BrowsingMode.Private,
),
)
}
(activity as HomeActivity).openToBrowserAndLoad(
searchTermOrURL = item.url,

@ -9,6 +9,7 @@ import kotlinx.parcelize.Parcelize
import mozilla.components.concept.storage.HistoryMetadata
import mozilla.components.concept.storage.HistoryMetadataKey
import mozilla.components.lib.state.Action
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.State
import mozilla.components.lib.state.Store
import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
@ -110,8 +111,11 @@ fun HistoryMetadata.toHistoryMetadata(position: Int): History.Metadata {
/**
* The [Store] for holding the [HistoryFragmentState] and applying [HistoryFragmentAction]s.
*/
class HistoryFragmentStore(initialState: HistoryFragmentState) :
Store<HistoryFragmentState, HistoryFragmentAction>(initialState, ::historyStateReducer)
class HistoryFragmentStore(
initialState: HistoryFragmentState,
middleware: List<Middleware<HistoryFragmentState, HistoryFragmentAction>> = listOf(),
) :
Store<HistoryFragmentState, HistoryFragmentAction>(initialState, ::historyStateReducer, middleware)
/**
* Actions to dispatch through the `HistoryStore` to modify `HistoryState` through the reducer.
@ -121,6 +125,20 @@ sealed class HistoryFragmentAction : Action {
data class AddItemForRemoval(val item: History) : HistoryFragmentAction()
data class RemoveItemForRemoval(val item: History) : HistoryFragmentAction()
/**
* A [History] item has been clicked by a user.
*
* @property item The history item clicked.
*/
data class HistoryItemClicked(val item: History) : HistoryFragmentAction()
/**
* A [History] item has been long-clicked by a user.
*
* @property item The history item long-clicked.
*/
data class HistoryItemLongClicked(val item: History) : HistoryFragmentAction()
/**
* Updates the empty state of [org.mozilla.fenix.library.history.HistoryView].
*/
@ -157,6 +175,16 @@ data class HistoryFragmentState(
object Syncing : Mode()
data class Editing(override val selectedItems: Set<History>) : Mode()
}
companion object {
val initial = HistoryFragmentState(
items = listOf(),
mode = Mode.Normal,
pendingDeletionItems = emptySet(),
isEmpty = false,
isDeletingItems = false,
)
}
}
/**
@ -188,5 +216,38 @@ private fun historyStateReducer(
is HistoryFragmentAction.UpdatePendingDeletionItems -> state.copy(
pendingDeletionItems = action.pendingDeletionItems,
)
is HistoryFragmentAction.HistoryItemClicked -> {
if (state.mode.selectedItems.isEmpty() || state.mode is HistoryFragmentState.Mode.Syncing) {
state
} else {
if (state.mode.selectedItems.contains(action.item)) {
val selected = state.mode.selectedItems - action.item
state.copy(
mode = if (selected.isEmpty()) {
HistoryFragmentState.Mode.Normal
} else {
HistoryFragmentState.Mode.Editing(selected)
},
)
} else {
state.copy(
mode = HistoryFragmentState.Mode.Editing(
state.mode.selectedItems + action.item,
),
)
}
}
}
is HistoryFragmentAction.HistoryItemLongClicked -> {
if (state.mode == HistoryFragmentState.Mode.Syncing) {
state
} else {
state.copy(
mode = HistoryFragmentState.Mode.Editing(
state.mode.selectedItems + action.item,
),
)
}
}
}
}

@ -24,6 +24,7 @@ import org.mozilla.fenix.theme.ThemeManager
class HistoryView(
container: ViewGroup,
val interactor: HistoryInteractor,
val store: HistoryFragmentStore,
val onZeroItemsLoaded: () -> Unit,
val onEmptyStateChanged: (Boolean) -> Unit,
) : LibraryPageView(container), UserInteractionHandler {
@ -37,7 +38,7 @@ class HistoryView(
var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
private set
val historyAdapter = HistoryAdapter(interactor) { isEmpty ->
val historyAdapter = HistoryAdapter(interactor, store) { isEmpty ->
onEmptyStateChanged(isEmpty)
}.apply {
addLoadStateListener {

@ -0,0 +1,66 @@
/* 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
import androidx.navigation.NavController
import androidx.navigation.NavOptions
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.MiddlewareContext
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
/**
* A [Middleware] for initiating navigation events based on [HistoryFragmentAction]s that are
* dispatched to the [HistoryFragmentStore].
*
* @property navController [NavController] for handling navigation events
* @property openToBrowser Callback to open history items in a browser window.
*/
class HistoryNavigationMiddleware(
private val navController: NavController,
private val openToBrowser: (item: History.Regular) -> Unit,
private val scope: CoroutineScope = CoroutineScope(Dispatchers.Main),
) : Middleware<HistoryFragmentState, HistoryFragmentAction> {
override fun invoke(
context: MiddlewareContext<HistoryFragmentState, HistoryFragmentAction>,
next: (HistoryFragmentAction) -> Unit,
action: HistoryFragmentAction,
) {
// Read the current state before letting the chain process the action, so that clicks are
// treated correctly in reference to the number of selected items.
val currentState = context.state
next(action)
scope.launch {
when (action) {
is HistoryFragmentAction.HistoryItemClicked -> {
if (currentState.mode.selectedItems.isEmpty()) {
when (val item = action.item) {
is History.Regular -> openToBrowser(item)
is History.Group -> {
navController.navigate(
HistoryFragmentDirections.actionGlobalHistoryMetadataGroup(
title = item.title,
historyMetadataItems = item.items.toTypedArray(),
),
NavOptions.Builder()
.setPopUpTo(R.id.historyMetadataGroupFragment, true)
.build(),
)
}
else -> Unit
}
}
}
else -> Unit
}
}
}
}

@ -0,0 +1,52 @@
/* 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
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.MiddlewareContext
import mozilla.components.service.glean.private.NoExtras
import org.mozilla.fenix.library.history.History
import org.mozilla.fenix.library.history.HistoryFragmentAction
import org.mozilla.fenix.library.history.HistoryFragmentState
import org.mozilla.fenix.GleanMetrics.History as GleanHistory
/**
* A [Middleware] for recording telemetry based on [HistoryFragmentAction]s that are dispatched to
* the [HistoryFragmentStore].
*
* @property isInPrivateMode Whether the app is currently in private browsing mode.
*/
class HistoryTelemetryMiddleware(
private val isInPrivateMode: Boolean,
) : Middleware<HistoryFragmentState, HistoryFragmentAction> {
override fun invoke(
context: MiddlewareContext<HistoryFragmentState, HistoryFragmentAction>,
next: (HistoryFragmentAction) -> Unit,
action: HistoryFragmentAction,
) {
val currentState = context.state
next(action)
when (action) {
is HistoryFragmentAction.HistoryItemClicked -> {
if (currentState.mode.selectedItems.isEmpty()) {
when (val item = action.item) {
is History.Regular -> {
GleanHistory.openedItem.record(
GleanHistory.OpenedItemExtra(
isRemote = item.isRemote,
timeGroup = item.historyTimeGroup.toString(),
isPrivate = isInPrivateMode,
),
)
}
is History.Group -> GleanHistory.searchTermGroupTapped.record(NoExtras())
else -> Unit
}
}
}
else -> Unit
}
}
}

@ -7,13 +7,16 @@ package org.mozilla.fenix.library.history.viewholders
import android.view.View
import androidx.core.view.isVisible
import androidx.recyclerview.widget.RecyclerView
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.R
import org.mozilla.fenix.databinding.HistoryListItemBinding
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.hideAndDisable
import org.mozilla.fenix.ext.showAndEnable
import org.mozilla.fenix.library.history.History
import org.mozilla.fenix.library.history.HistoryFragmentAction
import org.mozilla.fenix.library.history.HistoryFragmentState
import org.mozilla.fenix.library.history.HistoryFragmentStore
import org.mozilla.fenix.library.history.HistoryInteractor
import org.mozilla.fenix.library.history.HistoryItemTimeGroup
import org.mozilla.fenix.selection.SelectionHolder
@ -22,6 +25,7 @@ class HistoryListItemViewHolder(
view: View,
private val historyInteractor: HistoryInteractor,
private val selectionHolder: SelectionHolder<History>,
private val store: HistoryFragmentStore,
) : RecyclerView.ViewHolder(view) {
private var item: History? = null
@ -85,7 +89,18 @@ class HistoryListItemViewHolder(
val headerText = timeGroup?.humanReadable(itemView.context)
toggleHeader(headerText)
binding.historyLayout.setSelectionInteractor(item, selectionHolder, historyInteractor)
if (FeatureFlags.historyFragmentLibStateRefactor) {
binding.historyLayout.setOnClickListener {
store.dispatch(HistoryFragmentAction.HistoryItemClicked(item))
}
binding.historyLayout.setOnLongClickListener {
store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(item))
true
}
} else {
binding.historyLayout.setSelectionInteractor(item, selectionHolder, historyInteractor)
}
binding.historyLayout.changeSelected(item in selectionHolder.selectedItems)
if (item is History.Regular &&

@ -5,6 +5,7 @@
package org.mozilla.fenix.library.history
import kotlinx.coroutines.test.runTest
import mozilla.components.support.test.ext.joinBlocking
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotSame
@ -103,6 +104,87 @@ class HistoryFragmentStoreTest {
assertEquals(emptySet<PendingDeletionHistory>(), store.state.pendingDeletionItems)
}
@Test
fun `GIVEN items have been selected WHEN selected item is clicked THEN item is unselected`() = runTest {
val store = HistoryFragmentStore(twoItemEditState())
store.dispatch(HistoryFragmentAction.HistoryItemClicked(historyItem)).joinBlocking()
assertEquals(1, store.state.mode.selectedItems.size)
assertEquals(newHistoryItem, store.state.mode.selectedItems.first())
}
@Test
fun `GIVEN items have been selected WHEN unselected item is clicked THEN item is selected`() {
val initialState = oneItemEditState().copy(items = listOf(newHistoryItem))
val store = HistoryFragmentStore(initialState)
store.dispatch(HistoryFragmentAction.HistoryItemClicked(newHistoryItem)).joinBlocking()
assertEquals(2, store.state.mode.selectedItems.size)
assertTrue(store.state.mode.selectedItems.contains(newHistoryItem))
}
@Test
fun `GIVEN items have been selected WHEN last selected item is clicked THEN editing mode is exited`() {
val store = HistoryFragmentStore(oneItemEditState())
store.dispatch(HistoryFragmentAction.HistoryItemClicked(historyItem)).joinBlocking()
assertEquals(0, store.state.mode.selectedItems.size)
assertTrue(store.state.mode is HistoryFragmentState.Mode.Normal)
}
@Test
fun `GIVEN items have not been selected WHEN item is clicked THEN state is unchanged`() {
val store = HistoryFragmentStore(emptyDefaultState())
store.dispatch(HistoryFragmentAction.HistoryItemClicked(historyItem)).joinBlocking()
assertEquals(0, store.state.mode.selectedItems.size)
assertTrue(store.state.mode is HistoryFragmentState.Mode.Normal)
}
@Test
fun `GIVEN mode is syncing WHEN item is clicked THEN state is unchanged`() {
val store = HistoryFragmentStore(emptyDefaultState().copy(mode = HistoryFragmentState.Mode.Syncing))
store.dispatch(HistoryFragmentAction.HistoryItemClicked(historyItem)).joinBlocking()
assertEquals(0, store.state.mode.selectedItems.size)
assertTrue(store.state.mode is HistoryFragmentState.Mode.Syncing)
}
@Test
fun `GIVEN mode is syncing WHEN item is long-clicked THEN state is unchanged`() {
val store = HistoryFragmentStore(emptyDefaultState().copy(mode = HistoryFragmentState.Mode.Syncing))
store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(historyItem)).joinBlocking()
assertEquals(0, store.state.mode.selectedItems.size)
assertTrue(store.state.mode is HistoryFragmentState.Mode.Syncing)
}
@Test
fun `GIVEN mode is not syncing WHEN item is long-clicked THEN mode becomes editing`() {
val store = HistoryFragmentStore(oneItemEditState())
store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(newHistoryItem)).joinBlocking()
assertEquals(2, store.state.mode.selectedItems.size)
assertTrue(store.state.mode.selectedItems.contains(newHistoryItem))
}
@Test
fun `GIVEN mode is not syncing WHEN item is long-clicked THEN item is selected`() {
val store = HistoryFragmentStore(emptyDefaultState())
store.dispatch(HistoryFragmentAction.HistoryItemLongClicked(historyItem)).joinBlocking()
assertEquals(1, store.state.mode.selectedItems.size)
assertTrue(store.state.mode.selectedItems.contains(historyItem))
}
private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState(
items = listOf(),
mode = HistoryFragmentState.Mode.Normal,

@ -0,0 +1,97 @@
package org.mozilla.fenix.library.history.state
import androidx.navigation.NavController
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import mozilla.components.support.test.any
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.mock
import mozilla.components.support.test.rule.MainCoroutineRule
import mozilla.components.support.test.whenever
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.mockito.Mockito.verify
import org.mozilla.fenix.library.history.History
import org.mozilla.fenix.library.history.HistoryFragmentAction
import org.mozilla.fenix.library.history.HistoryFragmentState
import org.mozilla.fenix.library.history.HistoryFragmentStore
import org.mozilla.fenix.library.history.HistoryItemTimeGroup
class HistoryNavigationMiddlewareTest {
@get:Rule
val coroutinesTestRule = MainCoroutineRule()
@Test
fun `WHEN regular history item clicked THEN item is opened in browser`() = runTest {
var openedInBrowser = false
val url = "url"
val history = History.Regular(0, "title", url, 0, HistoryItemTimeGroup.timeGroupForTimestamp(0))
val middleware = HistoryNavigationMiddleware(
navController = mock(),
openToBrowser = { item ->
if (item.url == url) {
openedInBrowser = true
}
},
scope = this,
)
val store =
HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware))
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
advanceUntilIdle()
assertTrue(openedInBrowser)
}
@Test
fun `GIVEN selected items WHEN the last selected item is clicked THEN last item is not opened`() = runTest {
var openedInBrowser = false
val url = "url"
val history = History.Regular(0, "title", url, 0, HistoryItemTimeGroup.timeGroupForTimestamp(0))
val middleware = HistoryNavigationMiddleware(
navController = mock(),
openToBrowser = { item ->
if (item.url == url) {
openedInBrowser = true
}
},
scope = this,
)
val state = HistoryFragmentState.initial.copy(
mode = HistoryFragmentState.Mode.Editing(selectedItems = setOf(history)),
)
val store =
HistoryFragmentStore(initialState = state, middleware = listOf(middleware))
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
advanceUntilIdle()
assertFalse(openedInBrowser)
}
@Test
fun `WHEN group history item clicked THEN navigate to history metadata fragment`() = runTest {
val title = "title"
val history = History.Group(0, title, 0, HistoryItemTimeGroup.timeGroupForTimestamp(0), listOf())
val navController = mock<NavController>()
whenever(navController.navigate(directions = any(), navOptions = any())).thenAnswer { }
val middleware = HistoryNavigationMiddleware(
navController = navController,
openToBrowser = { },
scope = this,
)
val store =
HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware))
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
advanceUntilIdle()
verify(navController).navigate(
directions = any(),
navOptions = any(),
)
}
}

@ -0,0 +1,65 @@
package org.mozilla.fenix.library.history.state
import mozilla.components.service.glean.testing.GleanTestRule
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.library.history.History
import org.mozilla.fenix.library.history.HistoryFragmentAction
import org.mozilla.fenix.library.history.HistoryFragmentState
import org.mozilla.fenix.library.history.HistoryFragmentStore
import org.mozilla.fenix.library.history.HistoryItemTimeGroup
import org.robolectric.RobolectricTestRunner
import org.mozilla.fenix.GleanMetrics.History as GleanHistory
@RunWith(RobolectricTestRunner::class)
class HistoryTelemetryMiddlewareTest {
@get:Rule
val gleanTestRule = GleanTestRule(testContext)
private val middleware = HistoryTelemetryMiddleware(isInPrivateMode = false)
@Test
fun `GIVEN no items selected WHEN regular history item clicked THEN telemetry recorded`() {
val history = History.Regular(0, "title", "url", 0, HistoryItemTimeGroup.timeGroupForTimestamp(0))
val store = HistoryFragmentStore(
initialState = HistoryFragmentState.initial,
middleware = listOf(middleware),
)
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
assertNotNull(GleanHistory.openedItem.testGetValue())
}
@Test
fun `GIVEN items selected WHEN regular history item clicked THEN no telemetry recorded`() {
val history = History.Regular(0, "title", "url", 0, HistoryItemTimeGroup.timeGroupForTimestamp(0))
val state = HistoryFragmentState.initial.copy(
mode = HistoryFragmentState.Mode.Editing(selectedItems = setOf(history)),
)
val store = HistoryFragmentStore(
initialState = state,
middleware = listOf(middleware),
)
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
assertNull(GleanHistory.openedItem.testGetValue())
}
@Test
fun `WHEN group history item clicked THEN record telemetry`() {
val history = History.Group(0, "title", 0, HistoryItemTimeGroup.timeGroupForTimestamp(0), listOf())
val store =
HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware))
store.dispatch(HistoryFragmentAction.HistoryItemClicked(history)).joinBlocking()
assertNotNull(GleanHistory.searchTermGroupTapped.testGetValue())
}
}
Loading…
Cancel
Save