[fenix] Closes https://github.com/mozilla-mobile/fenix/issues/21871 - Eagerly update UI state after search group removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
pull/600/head
Grisha Kruglov 3 years ago committed by Christian Sadilek
parent 0751190582
commit eef2ad800a

@ -9,12 +9,15 @@ import androidx.annotation.VisibleForTesting.PRIVATE
import androidx.navigation.NavController
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import mozilla.components.browser.state.action.HistoryMetadataAction
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.storage.HistoryMetadataStorage
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.BrowserFragmentDirections
import org.mozilla.fenix.historymetadata.HistoryMetadataGroup
import org.mozilla.fenix.historymetadata.interactor.HistoryMetadataInteractor
import org.mozilla.fenix.home.HomeFragmentAction
import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.home.HomeFragmentStore
import org.mozilla.fenix.library.history.toHistoryMetadata
/**
@ -42,6 +45,8 @@ interface HistoryMetadataController {
* The default implementation of [HistoryMetadataController].
*/
class DefaultHistoryMetadataController(
private val store: BrowserStore,
private val homeStore: HomeFragmentStore,
private val navController: NavController,
private val storage: HistoryMetadataStorage,
private val scope: CoroutineScope
@ -65,12 +70,15 @@ class DefaultHistoryMetadataController(
}
override fun handleRemoveGroup(searchTerm: String) {
// We want to update the UI right away in response to user action without waiting for the IO.
// First, dispatch actions that will clean up search groups in the two stores that have
// metadata-related state.
store.dispatch(HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = searchTerm))
homeStore.dispatch(HomeFragmentAction.DisbandSearchGroupAction(searchTerm = searchTerm))
// Then, perform the expensive IO work of removing search groups from storage.
scope.launch {
storage.deleteHistoryMetadata(searchTerm)
}
navController.navigate(
BrowserFragmentDirections.actionGlobalHome()
)
}
@VisibleForTesting(otherwise = PRIVATE)

@ -349,8 +349,10 @@ class HomeFragment : Fragment() {
),
historyMetadataController = DefaultHistoryMetadataController(
navController = findNavController(),
homeStore = homeFragmentStore,
storage = components.core.historyStorage,
scope = viewLifecycleOwner.lifecycleScope
scope = viewLifecycleOwner.lifecycleScope,
store = components.core.store
),
pocketStoriesController = DefaultPocketStoriesController(
homeActivity = activity,

@ -97,6 +97,7 @@ sealed class HomeFragmentAction : Action {
data class RecentTabsChange(val recentTabs: List<RecentTab>) : HomeFragmentAction()
data class RecentBookmarksChange(val recentBookmarks: List<BookmarkNode>) : HomeFragmentAction()
data class HistoryMetadataChange(val historyMetadata: List<HistoryMetadataGroup>) : HomeFragmentAction()
data class DisbandSearchGroupAction(val searchTerm: String) : HomeFragmentAction()
data class SelectPocketStoriesCategory(val categoryName: String) : HomeFragmentAction()
data class DeselectPocketStoriesCategory(val categoryName: String) : HomeFragmentAction()
data class PocketStoriesShown(val storiesShown: List<PocketRecommendedStory>) : HomeFragmentAction()
@ -150,6 +151,9 @@ private fun homeFragmentStateReducer(
is HomeFragmentAction.RecentTabsChange -> state.copy(recentTabs = action.recentTabs)
is HomeFragmentAction.RecentBookmarksChange -> state.copy(recentBookmarks = action.recentBookmarks)
is HomeFragmentAction.HistoryMetadataChange -> state.copy(historyMetadata = action.historyMetadata)
is HomeFragmentAction.DisbandSearchGroupAction -> state.copy(
historyMetadata = state.historyMetadata.filter { it.title.lowercase() != action.searchTerm.lowercase() }
)
is HomeFragmentAction.SelectPocketStoriesCategory -> {
val updatedCategoriesState = state.copy(
pocketStoriesCategoriesSelections =

@ -128,6 +128,18 @@ class HomeFragmentStoreTest {
assertEquals(historyMetadata, homeFragmentStore.state.historyMetadata)
}
@Test
fun `Test disbanding search group in HomeFragmentStore`() = runBlocking {
val g1 = HistoryMetadataGroup(title = "test One")
val g2 = HistoryMetadataGroup(title = "test two")
val historyMetadata: List<HistoryMetadataGroup> = listOf(g1, g2)
homeFragmentStore.dispatch(HomeFragmentAction.HistoryMetadataChange(historyMetadata)).join()
assertEquals(historyMetadata, homeFragmentStore.state.historyMetadata)
homeFragmentStore.dispatch(HomeFragmentAction.DisbandSearchGroupAction("Test one")).join()
assertEquals(listOf(g2), homeFragmentStore.state.historyMetadata)
}
@Test
fun `Test changing hiding collections placeholder`() = runBlocking {
assertTrue(homeFragmentStore.state.showCollectionPlaceholder)

@ -3,5 +3,5 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
object AndroidComponents {
const val VERSION = "95.0.20211013154351"
const val VERSION = "95.0.20211013172120"
}

Loading…
Cancel
Save