commit 457417d3eb61669e6495f6fc09a00d76fdb5a1ed parent 7becdd1b439e7fd963556c970461eeeec2f19988 Author: Cristian Tuns <ctuns@mozilla.com> Date: Tue, 4 Nov 2025 00:13:56 -0500 Revert "Bug 1994279, Bug 1994285 - Part 2: Add “Clear Recent Searches” button to Settings Search r=android-reviewers,petru,android-l10n-reviewers,delphine" for causing fenix failures in StudiesViewTest This reverts commit 1eb7755a40e30bdbbd1c8598f7b50fca4404b762. Revert "Bug 1994279 - Part 1: Recent Searches in Settings Search r=android-reviewers,petru" This reverts commit dccdbb4eaa7d0fc643cf1e20f3c05ab8717a84e0. Revert "Bug 1994285 - Change divider and hint text for Settings Search bar. Fix Landscape padding. r=android-reviewers,android-l10n-reviewers,delphine,petru" This reverts commit 7024be5d788f3044b7b7d2534e26c586f78af31b. Diffstat:
15 files changed, 114 insertions(+), 741 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/FenixRecentSettingsSearchesRepository.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/FenixRecentSettingsSearchesRepository.kt @@ -1,84 +0,0 @@ -/* 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.settings.settingssearch - -import android.content.Context -import androidx.datastore.core.DataStore -import androidx.datastore.dataStore -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.map -import org.mozilla.fenix.settings.datastore.RecentSettingsSearchItem -import org.mozilla.fenix.settings.datastore.RecentSettingsSearches -import org.mozilla.fenix.settings.settingssearch.DefaultFenixSettingsIndexer.Companion.preferenceFileInformationList - -private val Context.recentSearchesDataStore: DataStore<RecentSettingsSearches> by dataStore( - fileName = "recent_searches.pb", - serializer = RecentSettingsSearchesSerializer, -) - -/** - * Repository for recent searches. - * - * @param context The application context. - */ -class FenixRecentSettingsSearchesRepository( - private val context: Context, -) : RecentSettingsSearchesRepository { - - override val recentSearches: Flow<List<SettingsSearchItem>> = - context.recentSearchesDataStore.data.map { protoResult -> - protoResult.itemsList.mapNotNull { protoItem -> - val prefInfo = preferenceFileInformationList.find { - it.xmlResourceId == protoItem.xmlResourceId - } ?: return@mapNotNull null - - SettingsSearchItem( - preferenceKey = protoItem.preferenceKey, - title = protoItem.title, - summary = protoItem.summary, - breadcrumbs = protoItem.breadcrumbsList, - preferenceFileInformation = prefInfo, - ) - } - } - - /** - * Adds a new recent search item to the repository. - * - * @param item The [SettingsSearchItem] to add. - */ - override suspend fun addRecentSearchItem(item: SettingsSearchItem) { - context.recentSearchesDataStore.updateData { currentRecents -> - val currentItems = currentRecents.itemsList.toMutableList() - - currentItems.removeIf { it.preferenceKey == item.preferenceKey } - - val newProtoItem = RecentSettingsSearchItem.newBuilder() - .setPreferenceKey(item.preferenceKey) - .setTitle(item.title) - .setSummary(item.summary) - .addAllBreadcrumbs(item.breadcrumbs) - .setXmlResourceId(item.preferenceFileInformation.xmlResourceId) - .build() - currentItems.add(0, newProtoItem) - - val updatedItems = currentItems.take(MAX_RECENTS) - currentRecents.toBuilder().clearItems().addAllItems(updatedItems).build() - } - } - - /** - * Clears all recent search items from the repository. - */ - override suspend fun clearRecentSearches() { - context.recentSearchesDataStore.updateData { - it.toBuilder().clearItems().build() - } - } - - companion object { - private const val MAX_RECENTS = 5 - } -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/RecentSettingsSearchesRepository.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/RecentSettingsSearchesRepository.kt @@ -1,31 +0,0 @@ -/* 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.settings.settingssearch - -import kotlinx.coroutines.flow.Flow - -/** - * A repository for recent search items. - */ -interface RecentSettingsSearchesRepository { - - /** - * A flow that emits the list of recent search items whenever it changes. - */ - val recentSearches: Flow<List<SettingsSearchItem>> - - /** - * Adds a [SettingsSearchItem] to the list of recent search items. - * If the item already exits, it is moved to the top. - * - * @param item The item to add. - */ - suspend fun addRecentSearchItem(item: SettingsSearchItem) - - /** - * Clears the list of recent search items. - */ - suspend fun clearRecentSearches() -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/RecentSettingsSearchesSerializer.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/RecentSettingsSearchesSerializer.kt @@ -1,29 +0,0 @@ -/* 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.settings.settingssearch - -import androidx.datastore.core.CorruptionException -import androidx.datastore.core.Serializer -import com.google.protobuf.InvalidProtocolBufferException -import org.mozilla.fenix.settings.datastore.RecentSettingsSearches -import java.io.InputStream -import java.io.OutputStream - -/** - * DataStore serializer for Recent Settings Searches. - */ -object RecentSettingsSearchesSerializer : Serializer<RecentSettingsSearches> { - override val defaultValue: RecentSettingsSearches = RecentSettingsSearches.getDefaultInstance() - - override suspend fun readFrom(input: InputStream): RecentSettingsSearches { - try { - return RecentSettingsSearches.parseFrom(input) - } catch (exception: InvalidProtocolBufferException) { - throw CorruptionException("Cannot read proto.", exception) - } - } - - override suspend fun writeTo(t: RecentSettingsSearches, output: OutputStream) = t.writeTo(output) -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchAction.kt @@ -10,24 +10,6 @@ import mozilla.components.lib.state.Action * Actions for the settings search screen. */ sealed interface SettingsSearchAction : Action { - - /** - * User has started a search. - */ - data object Init : SettingsSearchAction - - /** - * Signals a new valid [SettingsSearchEnvironment] has been set. - * - * @property environment New [SettingsSearchEnvironment]. - */ - data class EnvironmentRehydrated(val environment: SettingsSearchEnvironment) : SettingsSearchAction - - /** - * Signals that the current [SettingsSearchEnvironment] has been cleared. - */ - data object EnvironmentCleared : SettingsSearchAction - /** * User has updated the search query in the search bar. * @@ -59,16 +41,4 @@ sealed interface SettingsSearchAction : Action { * @property item [SettingsSearchItem] that was clicked. */ data class ResultItemClicked(val item: SettingsSearchItem) : SettingsSearchAction - - /** - * Recent Searches have been updated. - * - * @property recentSearches List of [SettingsSearchItem]s that represent the recent searches. - */ - data class RecentSearchesUpdated(val recentSearches: List<SettingsSearchItem>) : SettingsSearchAction - - /** - * User has clicked on the clear recent searches button. - */ - data object ClearRecentSearchesClicked : SettingsSearchAction } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchBar.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchBar.kt @@ -6,7 +6,7 @@ package org.mozilla.fenix.settings.settingssearch import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.TopAppBar @@ -16,13 +16,11 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import mozilla.components.compose.base.button.IconButton import mozilla.components.compose.base.textfield.TextField -import mozilla.components.compose.base.textfield.TextFieldColors import mozilla.components.lib.state.ext.observeAsComposableState import org.mozilla.fenix.R import org.mozilla.fenix.theme.FirefoxTheme @@ -44,8 +42,6 @@ fun SettingsSearchBar( var searchQuery by remember { mutableStateOf(state.searchQuery) } TopAppBar( - modifier = Modifier - .height(72.dp), title = { TextField( value = searchQuery, @@ -54,15 +50,11 @@ fun SettingsSearchBar( store.dispatch(SettingsSearchAction.SearchQueryUpdated(value)) }, modifier = Modifier - .fillMaxWidth(), + .fillMaxWidth() + .padding(end = 8.dp), placeholder = stringResource(R.string.settings_search_title), singleLine = true, errorText = stringResource(R.string.settings_search_error_message), - colors = TextFieldColors.default( - focusedIndicatorColor = Color.Transparent, - unfocusedIndicatorColor = Color.Transparent, - errorIndicatorColor = Color.Transparent, - ), trailingIcons = { when (state) { is SettingsSearchState.SearchInProgress, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchEnvironment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchEnvironment.kt @@ -1,24 +0,0 @@ -/* 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.settings.settingssearch - -import android.content.Context -import androidx.fragment.app.Fragment -import androidx.navigation.NavController - -/** - * Environment for the [SettingsSearchStore]. - * - * @property navController [NavController] used for navigation. - * @property fragment [Fragment] used for lifecycle owner and context for UI operations. - * @property context [Context] used for various system interactions - * @property recentSettingsSearchesRepository [RecentSettingsSearchesRepository] used for storing recent searches. - */ -data class SettingsSearchEnvironment( - val navController: NavController, - val fragment: Fragment, - val context: Context, - val recentSettingsSearchesRepository: RecentSettingsSearchesRepository, -) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchFragment.kt @@ -14,7 +14,6 @@ import androidx.appcompat.app.AppCompatActivity import androidx.compose.material3.ExperimentalMaterial3Api import androidx.fragment.app.Fragment import androidx.fragment.compose.content -import androidx.lifecycle.DefaultLifecycleObserver import androidx.navigation.fragment.findNavController import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.ext.components @@ -50,36 +49,18 @@ class SettingsSearchFragment : Fragment() { } private fun buildSettingsSearchStore(): SettingsSearchStore { - val recentSettingsSearchesRepository = FenixRecentSettingsSearchesRepository(requireContext()) - return StoreProvider.get(this) { SettingsSearchStore( - initialState = SettingsSearchState.Default(emptyList()), + initialState = SettingsSearchState.Default, middleware = listOf( SettingsSearchMiddleware( + SettingsSearchMiddleware.Companion.Dependencies( + navController = findNavController(), + ), fenixSettingsIndexer = requireContext().components.settingsIndexer, ), ), ) - }.also { - it.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = this, - navController = findNavController(), - context = requireContext(), - recentSettingsSearchesRepository = recentSettingsSearchesRepository, - ), - ), - ) - - viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: androidx.lifecycle.LifecycleOwner) { - it.dispatch(SettingsSearchAction.EnvironmentCleared) - } - }, - ) } } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchMiddleware.kt @@ -5,10 +5,7 @@ package org.mozilla.fenix.settings.settingssearch import androidx.core.os.bundleOf -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle -import kotlinx.coroutines.CoroutineDispatcher +import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -18,14 +15,18 @@ import mozilla.components.lib.state.MiddlewareContext /** * [Middleware] for the settings search screen. * + * @param initialDependencies [Dependencies] to use for navigation. * @property fenixSettingsIndexer [SettingsIndexer] to use for indexing and querying settings. - * @property dispatcher [CoroutineDispatcher] to use for performing background tasks. */ class SettingsSearchMiddleware( + initialDependencies: Dependencies, val fenixSettingsIndexer: SettingsIndexer, - val dispatcher: CoroutineDispatcher = Dispatchers.IO, ) : Middleware<SettingsSearchState, SettingsSearchAction> { - internal var environment: SettingsSearchEnvironment? = null + var dependencies = initialDependencies + + init { + fenixSettingsIndexer.indexAllSettings() + } override fun invoke( context: MiddlewareContext<SettingsSearchState, SettingsSearchAction>, @@ -34,24 +35,9 @@ class SettingsSearchMiddleware( ) { val store = context.store as SettingsSearchStore when (action) { - is SettingsSearchAction.Init -> { - next(action) - fenixSettingsIndexer.indexAllSettings() - } - is SettingsSearchAction.EnvironmentRehydrated -> { - next(action) - environment = action.environment - environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch { - observeRecentSearches(store) - } - } - is SettingsSearchAction.EnvironmentCleared -> { - next(action) - environment = null - } is SettingsSearchAction.SearchQueryUpdated -> { next(action) - CoroutineScope(dispatcher).launch { + CoroutineScope(Dispatchers.Main).launch { val results = fenixSettingsIndexer.getSettingsWithQuery(action.query) if (results.isEmpty()) { store.dispatch(SettingsSearchAction.NoResultsFound(action.query)) @@ -72,18 +58,8 @@ class SettingsSearchMiddleware( "search_in_progress" to true, ) val fragmentId = searchItem.preferenceFileInformation.fragmentId - CoroutineScope(dispatcher).launch { - environment?.recentSettingsSearchesRepository?.addRecentSearchItem(searchItem) - } CoroutineScope(Dispatchers.Main).launch { - environment?.navController?.navigate(fragmentId, bundle) - } - next(action) - } - is SettingsSearchAction.ClearRecentSearchesClicked -> { - next(action) - CoroutineScope(Dispatchers.IO).launch { - environment?.recentSettingsSearchesRepository?.clearRecentSearches() + dependencies.navController.navigate(fragmentId, bundle) } } else -> { @@ -93,20 +69,9 @@ class SettingsSearchMiddleware( } } - /** - * Observes the recent searches repository and updates the store when the list of recent searches changes. - * - * @param store The [SettingsSearchStore] to dispatch the updates to. - */ - private fun observeRecentSearches(store: SettingsSearchStore) { - environment?.fragment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - environment?.recentSettingsSearchesRepository?.recentSearches?.collect { recents -> - store.dispatch(SettingsSearchAction.RecentSearchesUpdated(recents)) - } - } - } - } + companion object { + data class Dependencies( + val navController: NavController, + ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchScreen.kt @@ -4,13 +4,8 @@ package org.mozilla.fenix.settings.settingssearch -import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.material3.HorizontalDivider @@ -18,14 +13,11 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.PreviewLightDark -import androidx.compose.ui.unit.dp -import mozilla.components.compose.base.button.TextButton import mozilla.components.lib.state.ext.observeAsComposableState import org.mozilla.fenix.R import org.mozilla.fenix.theme.FirefoxTheme @@ -44,53 +36,52 @@ fun SettingsSearchScreen( val state by store.observeAsComposableState { it } Scaffold( topBar = { - Column { - SettingsSearchBar( - store = store, - onBackClick = onBackClick, - ) - HorizontalDivider() - } + SettingsSearchBar( + store = store, + onBackClick = onBackClick, + ) }, ) { paddingValues -> - val topPadding = remember(paddingValues) { - paddingValues.calculateTopPadding() - } - when (state) { is SettingsSearchState.Default -> { - if (state.recentSearches.isNotEmpty()) { - RecentSearchesContent( - store = store, - recentSearches = state.recentSearches, - modifier = Modifier - .padding(top = topPadding) - .fillMaxSize(), - ) - } else { - SettingsSearchMessageContent( - modifier = Modifier - .padding(top = topPadding) - .fillMaxSize(), - ) - } + SettingsSearchMessageContent( + modifier = Modifier + .padding(paddingValues) + .fillMaxSize(), + ) } is SettingsSearchState.NoSearchResults -> { SettingsSearchMessageContent( modifier = Modifier - .padding(top = topPadding) + .padding(paddingValues) .fillMaxSize(), currentUserQuery = state.searchQuery, ) } is SettingsSearchState.SearchInProgress -> { - SearchResults( - store = store, - searchItems = state.searchResults, + LazyColumn( modifier = Modifier - .padding(top = topPadding) + .padding(paddingValues) .fillMaxSize(), - ) + ) { + items(state.searchResults.size) { index -> + val settingsSearchItem = state.searchResults[index] + if (index != 0) { + HorizontalDivider() + } + SettingsSearchResultItem( + item = settingsSearchItem, + query = state.searchQuery, + onClick = { + store.dispatch( + SettingsSearchAction.ResultItemClicked( + settingsSearchItem, + ), + ) + }, + ) + } + } } } } @@ -105,7 +96,7 @@ private fun SettingsSearchMessageContent( stringResource(R.string.settings_search_empty_query_placeholder) } else { stringResource( - R.string.settings_search_no_results_found_message, + R.string.setttings_search_no_results_found_message, currentUserQuery, ) } @@ -122,69 +113,6 @@ private fun SettingsSearchMessageContent( } } -@Composable -private fun SearchResults( - store: SettingsSearchStore, - searchItems: List<SettingsSearchItem>, - modifier: Modifier = Modifier, -) { - val state by store.observeAsComposableState { it } - - LazyColumn( - modifier = modifier, - ) { - items(searchItems.size) { index -> - val searchItem = searchItems[index] - if (index > 0) { - HorizontalDivider() - } - SettingsSearchResultItem( - item = searchItem, - query = state.searchQuery, - onClick = { - store.dispatch( - SettingsSearchAction.ResultItemClicked( - searchItem, - ), - ) - }, - ) - } - } -} - -@Composable -private fun RecentSearchesContent( - store: SettingsSearchStore, - recentSearches: List<SettingsSearchItem>, - modifier: Modifier = Modifier, -) { - Column( - modifier = modifier, - ) { - Row( - modifier = Modifier - .fillMaxWidth() - .height(34.dp) - .padding(start = 16.dp, end = 16.dp), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically, - ) { - Text( - text = stringResource(R.string.settings_search_recent_searches_section_header), - style = FirefoxTheme.typography.headline8, - ) - TextButton( - text = stringResource(R.string.settings_search_clear_recent_searches_message), - onClick = { - store.dispatch(SettingsSearchAction.ClearRecentSearchesClicked) - }, - ) - } - SearchResults(store, recentSearches, Modifier) - } -} - /** * Preview for the settings search screen initial state. */ @@ -198,100 +126,3 @@ private fun SettingsSearchScreenInitialStatePreview() { ) } } - -/** - * Preview for the settings search screen displaying a list of recent searches. - */ -@PreviewLightDark -@Composable -private fun SettingsSearchScreenWithRecentsPreview() { - val storeWithRecents = SettingsSearchStore( - initialState = SettingsSearchState.Default( - recentSearches = listOf( - SettingsSearchItem( - "Search engine", - "Choose your default", - "search_engine", - listOf("General"), - PreferenceFileInformation.SearchSettingsPreferences, - ), - SettingsSearchItem( - "Delete browsing data", - "Clear history, cookies, and more", - "delete_browsing_data", - listOf("Privacy"), - PreferenceFileInformation.PrivateBrowsingPreferences, - ), - ), - ), - ) - FirefoxTheme { - SettingsSearchScreen( - store = storeWithRecents, - onBackClick = {}, - ) - } -} - -/** - * Preview for the settings search screen displaying search results. - */ -@PreviewLightDark -@Composable -private fun SettingsSearchScreenWithResultsPreview() { - val storeWithResults = SettingsSearchStore( - initialState = SettingsSearchState.SearchInProgress( - searchQuery = "privacy", - searchResults = listOf( - SettingsSearchItem( - "Tracking Protection", - "Strict, Standard, or Custom", - "tracking_protection", - listOf("Privacy"), - PreferenceFileInformation.GeneralPreferences, - ), - SettingsSearchItem( - "Delete browsing data", - "Clear history, cookies, and more", - "delete_browsing_data", - listOf("Privacy"), - PreferenceFileInformation.GeneralPreferences, - ), - SettingsSearchItem( - "HTTPS-Only Mode", - "Enable in all tabs", - "https_only_mode", - listOf("Privacy", "Advanced"), - PreferenceFileInformation.GeneralPreferences, - ), - ), - recentSearches = emptyList(), - ), - ) - FirefoxTheme { - SettingsSearchScreen( - store = storeWithResults, - onBackClick = {}, - ) - } -} - -/** - * Preview for the settings search screen when no results are found. - */ -@PreviewLightDark -@Composable -private fun SettingsSearchScreenNoResultsPreview() { - val storeWithNoResults = SettingsSearchStore( - initialState = SettingsSearchState.NoSearchResults( - searchQuery = "nonexistent query", - recentSearches = emptyList(), - ), - ) - FirefoxTheme { - SettingsSearchScreen( - store = storeWithNoResults, - onBackClick = {}, - ) - } -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchStore.kt @@ -15,17 +15,13 @@ import mozilla.components.lib.state.Store * @param middleware List of [Middleware] to apply to the store. */ class SettingsSearchStore( - initialState: SettingsSearchState = SettingsSearchState.Default(emptyList()), + initialState: SettingsSearchState = SettingsSearchState.Default, middleware: List<Middleware<SettingsSearchState, SettingsSearchAction>> = emptyList(), ) : Store<SettingsSearchState, SettingsSearchAction>( initialState = initialState, reducer = ::reduce, middleware = middleware, -) { - init { - dispatch(SettingsSearchAction.Init) - } -} +) /** * Reducer for the settings search screen. @@ -34,26 +30,20 @@ private fun reduce(state: SettingsSearchState, action: SettingsSearchAction): Se return when (action) { is SettingsSearchAction.SearchQueryUpdated -> { if (action.query.isBlank()) { - SettingsSearchState.Default( - recentSearches = state.recentSearches, - ) + SettingsSearchState.Default } else { SettingsSearchState.SearchInProgress( searchQuery = action.query, searchResults = state.searchResults, - recentSearches = state.recentSearches, ) } } is SettingsSearchAction.NoResultsFound -> { if (action.query.isBlank()) { - SettingsSearchState.Default( - recentSearches = state.recentSearches, - ) + SettingsSearchState.Default } else { SettingsSearchState.NoSearchResults( searchQuery = action.query, - recentSearches = state.recentSearches, ) } } @@ -61,18 +51,9 @@ private fun reduce(state: SettingsSearchState, action: SettingsSearchAction): Se SettingsSearchState.SearchInProgress( searchQuery = action.query, searchResults = action.results, - recentSearches = state.recentSearches, ) } - is SettingsSearchAction.RecentSearchesUpdated -> { - state.copyWith(recentSearches = action.recentSearches) - } - is SettingsSearchAction.ClearRecentSearchesClicked, - is SettingsSearchAction.Init, - is SettingsSearchAction.ResultItemClicked, - is SettingsSearchAction.EnvironmentCleared, - is SettingsSearchAction.EnvironmentRehydrated, - -> state + is SettingsSearchAction.ResultItemClicked -> state } } @@ -81,45 +62,16 @@ private fun reduce(state: SettingsSearchState, action: SettingsSearchAction): Se * * @property searchQuery Current search query [String]. * @property searchResults List of [SettingsSearchItem]s that match the current search query, if any. - * @property recentSearches List of recently searched [SettingsSearchItem]s. */ sealed class SettingsSearchState( open val searchQuery: String = "", open val searchResults: List<SettingsSearchItem> = emptyList(), - open val recentSearches: List<SettingsSearchItem> = emptyList(), ) : State { - - /** - * Creates a new state of the same type with updated properties. - * This allows for clean state updates in the reducer. - */ - abstract fun copyWith( - searchQuery: String = this.searchQuery, - searchResults: List<SettingsSearchItem> = this.searchResults, - recentSearches: List<SettingsSearchItem> = this.recentSearches, - ): SettingsSearchState - /** * Default state. * No query, no results - * - * @property recentSearches List of recently searched [SettingsSearchItem]s. */ - data class Default( - override val recentSearches: List<SettingsSearchItem>, - ) : SettingsSearchState( - recentSearches = recentSearches, - ) { - override fun copyWith( - searchQuery: String, - searchResults: List<SettingsSearchItem>, - recentSearches: List<SettingsSearchItem>, - ): SettingsSearchState { - // A Default state can't have a query or search results, so we ignore those parameters - // and return a new Default state, only considering the recentSearches. - return Default(recentSearches = recentSearches) - } - } + data object Default : SettingsSearchState() /** * State when there is a query. @@ -127,53 +79,17 @@ sealed class SettingsSearchState( * * @property searchQuery Current search query [String]. * @property searchResults List of [SettingsSearchItem]s that match the current search query. - * @property recentSearches List of recently searched [SettingsSearchItem]s. */ data class SearchInProgress( override val searchQuery: String, override val searchResults: List<SettingsSearchItem>, - override val recentSearches: List<SettingsSearchItem>, - ) : SettingsSearchState( - searchQuery, - searchResults, - recentSearches, - ) { - override fun copyWith( - searchQuery: String, - searchResults: List<SettingsSearchItem>, - recentSearches: List<SettingsSearchItem>, - ): SettingsSearchState { - return this.copy( - searchQuery = searchQuery, - searchResults = searchResults, - recentSearches = recentSearches, - ) - } - } + ) : SettingsSearchState(searchQuery, searchResults) /** - * State when there is a query but it yields zero search results. + * State when there is a query but it yields zero search reuslts. * Query, no results. * * @property searchQuery Current search query [String]. - * @property recentSearches List of recently searched [SettingsSearchItem]s. */ - data class NoSearchResults( - override val searchQuery: String, - override val recentSearches: List<SettingsSearchItem>, - ) : SettingsSearchState( - searchQuery, - recentSearches = recentSearches, - ) { - override fun copyWith( - searchQuery: String, - searchResults: List<SettingsSearchItem>, - recentSearches: List<SettingsSearchItem>, - ): SettingsSearchState { - return this.copy( - searchQuery = searchQuery, - recentSearches = recentSearches, - ) - } - } + data class NoSearchResults(override val searchQuery: String) : SettingsSearchState(searchQuery) } diff --git a/mobile/android/fenix/app/src/main/proto/recent_settings_searches.proto b/mobile/android/fenix/app/src/main/proto/recent_settings_searches.proto @@ -1,24 +0,0 @@ -/* 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/. */ - -syntax = "proto3"; - -package proto; - -option java_package = "org.mozilla.fenix.settings.datastore"; -option java_multiple_files = true; - -// Represents the entire collection of recent searches -message RecentSettingsSearches { - repeated RecentSettingsSearchItem items = 1; -} - -// Represents a single recent settings search item -message RecentSettingsSearchItem { - string preference_key = 1; - string title = 2; - string summary = 3; - repeated string breadcrumbs = 4; - int32 xml_resource_id = 5; // We only need the ID to reconstruct the full object -} diff --git a/mobile/android/fenix/app/src/main/res/values/static_strings.xml b/mobile/android/fenix/app/src/main/res/values/static_strings.xml @@ -251,4 +251,19 @@ <!-- Label for enabling Relay email masks --> <string name="preferences_enable_relay_email_masks">Enable Relay email masks</string> + + <!-- Settings Search Strings --> + <!-- Title of Settings Search --> + <string name="settings_search_title">Settings Search</string> + <!-- Content description (not visible, for screen readers etc.): "Clear search field button for settings search" --> + <string name="content_description_settings_search_clear_search">Clear Search</string> + <!-- Content description (not visible, for screen readers etc.): "Navigate back button for settings search" --> + <string name="content_description_settings_search_navigate_back">Navigate Back</string> + <!-- Message displayed when the search field in the Settings Search screen is empty --> + <string name="settings_search_empty_query_placeholder">Search for settings</string> + <!-- Message when error happens with Settings Search --> + <string name="settings_search_error_message">Error occurred.</string> + <!-- Message displayed when the search field is not empty but search results is empty. + %s will be replaced by user query in search field. --> + <string name="setttings_search_no_results_found_message">No results found for %s</string> </resources> diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -3542,24 +3542,6 @@ <!-- Label for a button opening a feedback forum --> <string name="review_prompt_feedback_button">Leave feedback</string> - <!-- Settings Search Strings --> - <!-- Title of Settings Search. "Search" is a verb here--> - <string name="settings_search_title">Search Settings</string> - <!-- Content description (not visible, for screen readers etc.) for the button to clear the search field for settings search --> - <string name="content_description_settings_search_clear_search">Clear Search</string> - <!-- Content description (not visible, for screen readers etc.) for the button to navigate back to the Settings page from the Settings Search screen. --> - <string name="content_description_settings_search_navigate_back">Navigate Back</string> - <!-- Message displayed when the search field in the Settings Search screen is empty --> - <string name="settings_search_empty_query_placeholder">No recent searches</string> - <!-- Message when error happens with Settings Search --> - <string name="settings_search_error_message">An error has occurred</string> - <!-- Message title displayed when the search field is not empty but search results is empty. --> - <string name="settings_search_no_results_found_message">Nothing turned up</string> - <!-- Message in TextButton to clear recent search history for settings search --> - <string name="settings_search_clear_recent_searches_message">Clear all</string> - <!-- Section header for recent search results --> - <string name="settings_search_recent_searches_section_header">Recent search results</string> - <!-- Terms of use pop up prompt --> <!-- Title for the pop up prompt --> <string name="terms_of_use_prompt_title">We’ve got an update</string> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchMiddlewareTest.kt @@ -4,26 +4,21 @@ package org.mozilla.fenix.settings.settingssearch -import androidx.fragment.app.Fragment -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner -import androidx.lifecycle.LifecycleRegistry +import android.content.Context import androidx.navigation.NavController import androidx.test.ext.junit.runners.AndroidJUnit4 -import io.mockk.coVerify -import io.mockk.every import io.mockk.mockk -import io.mockk.spyk -import io.mockk.verify -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.support.test.libstate.ext.waitUntilIdle -import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.middleware.CaptureActionsMiddleware +import mozilla.components.support.test.mock import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +@OptIn(ExperimentalCoroutinesApi::class) @RunWith(AndroidJUnit4::class) class SettingsSearchMiddlewareTest { @@ -31,137 +26,58 @@ class SettingsSearchMiddlewareTest { val coroutineRule = MainCoroutineRule() private val navController: NavController = mockk(relaxed = true) - private val lifecycleOwner = FakeLifecycleOwner(Lifecycle.State.RESUMED) - private lateinit var fragment: Fragment - private val recentSearchesRepository: FenixRecentSettingsSearchesRepository = mockk(relaxed = true) - private val recentSearchesFlow = MutableStateFlow<List<SettingsSearchItem>>(emptyList()) - - @Before - fun setUp() { - every { recentSearchesRepository.recentSearches } returns recentSearchesFlow - fragment = spyk(Fragment()).apply { - every { context } returns testContext - } - every { fragment.viewLifecycleOwner } returns lifecycleOwner - } private fun buildMiddleware(): SettingsSearchMiddleware { return SettingsSearchMiddleware( + initialDependencies = SettingsSearchMiddleware.Companion.Dependencies( + navController = navController, + ), fenixSettingsIndexer = TestSettingsIndexer(), ) } @Test fun `WHEN the settings search query is updated and results are not found THEN the state is updated`() { - val query = "test" val middleware = SettingsSearchMiddleware( + initialDependencies = SettingsSearchMiddleware.Companion.Dependencies( + navController = navController, + ), fenixSettingsIndexer = EmptyTestSettingsIndexer(), ) - val store = SettingsSearchStore(middleware = listOf(middleware)) - store.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = fragment, - navController = navController, - context = testContext, - recentSettingsSearchesRepository = recentSearchesRepository, - ), + val capture = CaptureActionsMiddleware<SettingsSearchState, SettingsSearchAction>() + val query = "test" + val store = SettingsSearchStore( + middleware = listOf( + middleware, + capture, ), ) store.dispatch(SettingsSearchAction.SearchQueryUpdated(query)) store.waitUntilIdle() + capture.assertLastAction(SettingsSearchAction.NoResultsFound::class) + assert(store.state is SettingsSearchState.NoSearchResults) assert(store.state.searchQuery == query) } @Test fun `WHEN the settings search query is updated and results are found THEN the state is updated`() { - val query = "a" val middleware = buildMiddleware() - val store = SettingsSearchStore(middleware = listOf(middleware)) - store.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = fragment, - navController = navController, - context = testContext, - recentSettingsSearchesRepository = recentSearchesRepository, - ), + val capture = CaptureActionsMiddleware<SettingsSearchState, SettingsSearchAction>() + val query = "test" + val store = SettingsSearchStore( + middleware = listOf( + middleware, + capture, ), ) store.dispatch(SettingsSearchAction.SearchQueryUpdated(query)) store.waitUntilIdle() + capture.assertLastAction(SettingsSearchAction.SearchResultsLoaded::class) assert(store.state is SettingsSearchState.SearchInProgress) assert(store.state.searchQuery == query) - } - - @Test - fun `WHEN a result item is clicked THEN it should be added to the recent searches repository`() { - val middleware = buildMiddleware() - val store = SettingsSearchStore(middleware = listOf(middleware)) - val testItem = testList.first() - - store.dispatch(SettingsSearchAction.Init) - store.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = fragment, - navController = navController, - context = testContext, - recentSettingsSearchesRepository = recentSearchesRepository, - ), - ), - ) - - store.dispatch(SettingsSearchAction.ResultItemClicked(testItem)) - store.waitUntilIdle() - - coVerify { recentSearchesRepository.addRecentSearchItem(testItem) } - verify { navController.navigate(testItem.preferenceFileInformation.fragmentId, any()) } - } - - @Test - fun `WHEN RecentSearchesUpdated is dispatched THEN store state is updated correctly`() { - val middleware = buildMiddleware() - val store = SettingsSearchStore(middleware = listOf(middleware)) - val updatedRecents = listOf(testList.first()) - store.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = fragment, - navController = navController, - context = testContext, - recentSettingsSearchesRepository = recentSearchesRepository, - ), - ), - ) - - store.dispatch(SettingsSearchAction.RecentSearchesUpdated(updatedRecents)) - store.waitUntilIdle() - - assert(store.state.recentSearches == updatedRecents) - } - - @Test - fun `WHEN ClearRecentSearchesClicked is dispatched THEN store state is updated correctly`() { - val middleware = buildMiddleware() - val store = SettingsSearchStore(middleware = listOf(middleware)) - store.dispatch( - SettingsSearchAction.EnvironmentRehydrated( - environment = SettingsSearchEnvironment( - fragment = fragment, - navController = navController, - context = testContext, - recentSettingsSearchesRepository = recentSearchesRepository, - ), - ), - ) - - store.dispatch(SettingsSearchAction.ClearRecentSearchesClicked) - store.waitUntilIdle() - - assert(store.state.recentSearches.isEmpty()) + assert(store.state.searchResults == testList) } } @@ -209,9 +125,3 @@ class EmptyTestSettingsIndexer : SettingsIndexer { return emptyList() } } - -private class FakeLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner { - override val lifecycle: Lifecycle = LifecycleRegistry(this).apply { - currentState = initialState - } -} diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchStoreTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/settingssearch/SettingsSearchStoreTest.kt @@ -4,11 +4,18 @@ package org.mozilla.fenix.settings.settingssearch +import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.mock +import mozilla.components.support.test.rule.MainCoroutineRule +import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +@OptIn(ExperimentalCoroutinesApi::class) @RunWith(AndroidJUnit4::class) class SettingsSearchStoreTest { @@ -17,7 +24,7 @@ class SettingsSearchStoreTest { val query = "theme" val store = SettingsSearchStore() - val initialState = SettingsSearchState.Default(recentSearches = emptyList()) + val initialState = SettingsSearchState.Default assert(store.state == initialState) store.dispatch(SettingsSearchAction.SearchQueryUpdated(query)) @@ -30,17 +37,13 @@ class SettingsSearchStoreTest { @Test fun `GIVEN search in progress state WHEN SearchQueryUpdated action is dispatched with empty query THEN default state is dispatched`() { val store = SettingsSearchStore( - initialState = SettingsSearchState.SearchInProgress( - "theme", - emptyList(), - emptyList(), - ), + initialState = SettingsSearchState.SearchInProgress("theme", emptyList()), ) assert(store.state is SettingsSearchState.SearchInProgress) store.dispatch(SettingsSearchAction.SearchQueryUpdated("")) store.waitUntilIdle() - assert(store.state == SettingsSearchState.Default(emptyList())) + assert(store.state == SettingsSearchState.Default) } }