Closes #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"
This commit is contained in:
parent
88fbcc05b7
commit
233f0a8a1d
|
@ -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…
Reference in New Issue
Block a user