commit 3166cb355e99d04203aa8009403b2766d6333459
parent d4a1cd08e72f182710ea1249f38b28aefadf757a
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Tue, 7 Oct 2025 06:44:20 +0000
Bug 1992735 - Refactor Language selection screen to improve performance and state management. r=android-reviewers,avirvara
This patch refactors the language selection screen for better performance and to follow modern Compose best practices.
The changes include:
- In `LocaleFragmentCompose.kt`, the `LanguagesList` composable is updated to be more efficient and stateless. It now lazily processes language items as they are displayed.
- State handling is improved by using `selectedTag` and an `onLanguageSelected` callback, making the component more reusable and predictable.
- In `LanguageFragment.kt`, the logic for creating language list items is removed, simplifying the fragment and delegating UI state to the `LanguagesList` composable. It now uses a local state for the selected tag to provide instant UI feedback.
- In `LanguageStorage.kt`, language list creation is optimized. A `languagesByTag` map is introduced for efficient lookups, and the list of languages is now lazily initialized once, preventing redundant processing.
Differential Revision: https://phabricator.services.mozilla.com/D267623
Diffstat:
3 files changed, 111 insertions(+), 119 deletions(-)
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LanguageFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LanguageFragment.kt
@@ -8,10 +8,11 @@ import android.os.Bundle
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.MutableState
+import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.setValue
import kotlinx.coroutines.launch
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.lib.state.ext.observeAsComposableState
@@ -45,6 +46,8 @@ class LanguageFragment : BaseComposeFragment() {
),
),
)
+ languageScreenStore.state.languageList
+
defaultLanguageScreenInteractor = DefaultLanguageScreenInteractor(
languageScreenStore = languageScreenStore,
)
@@ -55,55 +58,35 @@ class LanguageFragment : BaseComposeFragment() {
@Composable
override fun Content() {
- val languagesList = languageScreenStore
- .observeAsComposableState { state -> state.languageList }.value
- val languageSelected = languageScreenStore.observeAsComposableState { state ->
- state.selectedLanguage
- }.value
- if (languageSelected != null) {
- Languages(languageSelected = languageSelected, languages = languagesList)
- }
- }
+ val languagesList by languageScreenStore.observeAsComposableState { it.languageList }
+ val languageSelected by languageScreenStore.observeAsComposableState { it.selectedLanguage }
- private fun createLanguageListItem(
- languages: List<Language>,
- state: MutableState<String>,
- ): List<LanguageListItem> {
- val languageListItems = ArrayList<LanguageListItem>()
- languages.forEach { language ->
- if (language.tag == LanguageStorage.LOCALE_SYSTEM_DEFAULT) {
- language.displayName = context?.getString(R.string.preference_language_systemdefault)
- }
- val languageListItem = LanguageListItem(
- language = language,
- onClick = {
- state.value = language.tag
- defaultLanguageScreenInteractor.handleLanguageSelected(language)
- },
- )
- languageListItems.add(languageListItem)
- }
- return languageListItems
+ languageSelected?.let { Languages(languageSelected = it, languages = languagesList) }
}
@Composable
private fun Languages(languageSelected: Language, languages: List<Language>) {
val listState = rememberLazyListState()
val coroutineScope = rememberCoroutineScope()
+
+ // This local state for the selected tag allows for instant UI feedback upon click.
+ var selectedTag by remember(languageSelected) { mutableStateOf(languageSelected.tag) }
+
LaunchedEffect(languageSelected.index) {
coroutineScope.launch {
- languageSelected.let { listState.scrollToItem(it.index) }
+ if (languageSelected.index in languages.indices) {
+ listState.scrollToItem(languageSelected.index)
+ }
}
}
- val state = remember {
- mutableStateOf(languageSelected.tag)
- }
+
LanguagesList(
- languageListItems = createLanguageListItem(
- languages = languages,
- state = state,
- ),
- state = state,
+ languages = languages,
+ selectedTag = selectedTag,
+ onLanguageSelected = {
+ selectedTag = it.tag
+ defaultLanguageScreenInteractor.handleLanguageSelected(it)
+ },
listState = listState,
)
}
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LanguageStorage.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LanguageStorage.kt
@@ -19,8 +19,35 @@ class LanguageStorage(private val context: Context) {
context.resources.getString(R.string.pref_key_locale)
}
+ /**
+ * A map of available languages, keyed by their language tag.
+ *
+ * This map is lazily initialized and includes:
+ * 1. The "System default" language.
+ * 2. All bundled locales from [LocalesList.BUNDLED_LOCALES], sorted alphabetically
+ * by their native display names.
+ *
+ * This provides efficient look-up of a [Language] object by its tag.
+ */
+ private val languagesByTag: Map<String, Language> by lazy {
+ buildMap {
+ put(systemDefaultLanguage.tag, systemDefaultLanguage)
+ LocalesList.BUNDLED_LOCALES
+ .map { LocaleDescriptor(it) }
+ .sorted()
+ .forEach { descriptor ->
+ val language = Language(
+ displayName = descriptor.getNativeName(),
+ tag = descriptor.getTag(),
+ index = size,
+ )
+ put(language.tag, language)
+ }
+ }
+ }
+
internal val languages: List<Language> by lazy {
- getLanguageList()
+ languagesByTag.values.toList()
}
private val systemDefaultLanguage: Language by lazy {
@@ -32,37 +59,22 @@ class LanguageStorage(private val context: Context) {
}
/**
- * The current selected Language or System default Language if nothing is selected
+ * The language currently selected by the user. If no language has been explicitly
+ * selected, this will default to the "System default" language.
+ *
+ * The value is retrieved from SharedPreferences using the `pref_key_locale`.
+ * If the stored language tag doesn't match any available language, it also
+ * falls back to the system default.
*/
internal val selectedLanguage: Language
get() {
val savedLanguageTag =
sharedPref.getString(localePrefKey, LOCALE_SYSTEM_DEFAULT) ?: LOCALE_SYSTEM_DEFAULT
- val matchingLanguage = languages.firstOrNull { it.tag == savedLanguageTag }
-
- return matchingLanguage ?: systemDefaultLanguage
+ return languagesByTag[savedLanguageTag] ?: systemDefaultLanguage
}
/**
- * The full list of available languages.
- * System default Language will be the first item in the list.
- */
- private fun getLanguageList(): List<Language> {
- return listOf(
- systemDefaultLanguage,
- ) + getUsableLocales().mapIndexedNotNull { i, descriptor ->
- descriptor?.let {
- Language(
- displayName = descriptor.getNativeName(),
- tag = it.getTag(),
- index = i + 1,
- )
- }
- }
- }
-
- /**
* Saves the current selected language tag
*
* @param languageTag the tag of the language
@@ -73,15 +85,6 @@ class LanguageStorage(private val context: Context) {
}
}
- /**
- * This method generates the descriptor array.
- */
- private fun getUsableLocales(): Array<LocaleDescriptor?> {
- return LocalesList.BUNDLED_LOCALES.map {
- LocaleDescriptor(it)
- }.sorted().toTypedArray()
- }
-
companion object {
const val LOCALE_SYSTEM_DEFAULT = "LOCALE_SYSTEM_DEFAULT"
}
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LocaleFragmentCompose.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/locale/screen/LocaleFragmentCompose.kt
@@ -22,65 +22,67 @@ import androidx.compose.material3.RadioButton
import androidx.compose.material3.RadioButtonDefaults
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.MutableState
+import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
+import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
-import kotlinx.coroutines.launch
+import org.mozilla.focus.R
import org.mozilla.focus.ui.theme.FocusTheme
import org.mozilla.focus.ui.theme.focusColors
-private fun getFakeLanguages(): List<LanguageListItem> {
- return mutableListOf<LanguageListItem>().apply {
- var index = 0
- add(LanguageListItem(Language("Română", "ro", index++), onClick = {}))
- add(LanguageListItem(Language("Slovenčina", "sk", index++), onClick = {}))
- add(LanguageListItem(Language("Português (Brasil)", "pt-BR", index++), onClick = {}))
- add(LanguageListItem(Language("Nederlands", "nl", index++), onClick = {}))
- add(LanguageListItem(Language("Magyar", "hu", index++), onClick = {}))
- add(LanguageListItem(Language("Lietuvių", "lt", ++index), onClick = {}))
- }
-}
-
@Composable
@Preview
private fun LanguagesListComposablePreview() {
FocusTheme {
val listState = rememberLazyListState()
- val coroutineScope = rememberCoroutineScope()
- LaunchedEffect(0) {
- coroutineScope.launch {
- listState.scrollToItem(0)
- }
- }
- val state = remember {
- mutableStateOf("ro")
+
+ val fakeLanguages = remember {
+ listOf(
+ Language("Română", "ro", 0),
+ Language("Slovenčina", "sk", 1),
+ Language("Português (Brasil)", "pt-BR", 2),
+ Language("Nederlands", "nl", 3),
+ Language("Magyar", "hu", 4),
+ Language("Lietuvių", "lt", 5),
+ )
}
+
+ var selectedTag by remember { mutableStateOf("ro") }
+
LanguagesList(
- languageListItems = getFakeLanguages(),
- state = state,
+ languages = fakeLanguages,
+ selectedTag = selectedTag,
+ onLanguageSelected = { language ->
+ selectedTag = language.tag
+ },
listState = listState,
)
}
}
/**
- * Displays a list of Languages in a listView
+ * Displays a lazily-loaded list of languages.
*
- * @param languageListItems The list of Languages items to be displayed.
- * @param state the current Selected Language
- * @param listState scrolls to the current selected Language
+ * This composable is optimized for performance by processing each language item only when it's
+ * about to be displayed. It also handles the special case for the "System Default" language
+ * by resolving its display name from string resources.
+ *
+ * @param languages The list of [Language] data to display.
+ * @param selectedTag The tag of the currently selected language, used to highlight the correct item.
+ * @param onLanguageSelected A callback invoked with the selected [Language] object when an item is clicked.
+ * @param listState The [LazyListState] for controlling and observing the scroll state of the list.
*/
@Composable
fun LanguagesList(
- languageListItems: List<LanguageListItem>,
- state: MutableState<String>,
+ languages: List<Language>,
+ selectedTag: String,
+ onLanguageSelected: (Language) -> Unit,
listState: LazyListState,
) {
FocusTheme {
@@ -88,11 +90,19 @@ fun LanguagesList(
state = listState,
contentPadding = PaddingValues(horizontal = 12.dp),
) {
- items(languageListItems) { item ->
+ items(languages, key = { it.tag }) { language ->
+ // By performing this logic here, inside the `items` block, we ensure
+ // that the work is only done for visible items, making the list fast.
+ val languageForDisplay = if (language.tag == LanguageStorage.LOCALE_SYSTEM_DEFAULT) {
+ language.copy(displayName = stringResource(R.string.preference_language_systemdefault))
+ } else {
+ language
+ }
+
LanguageNameAndTagItem(
- language = item.language,
- isSelected = state.value == item.language.tag,
- onClick = item.onClick,
+ language = languageForDisplay,
+ isSelected = selectedTag == language.tag,
+ onClick = { onLanguageSelected(language) },
)
}
}
@@ -100,22 +110,22 @@ fun LanguagesList(
}
@Composable
-fun LanguageNameAndTagItem(
+private fun LanguageNameAndTagItem(
language: Language,
isSelected: Boolean,
- onClick: (String) -> Unit,
+ onClick: (Language) -> Unit,
) {
Row(
Modifier
.fillMaxWidth()
- .wrapContentHeight(),
+ .wrapContentHeight()
+ .clickable { onClick(language) },
horizontalArrangement = Arrangement.Start,
verticalAlignment = Alignment.CenterVertically,
) {
LanguageRadioButton(
- language = language,
isSelected = isSelected,
- onClick = onClick,
+ onClick = { onClick(language) },
)
Spacer(modifier = Modifier.width(18.dp))
language.displayName?.let {
@@ -130,22 +140,18 @@ fun LanguageNameAndTagItem(
/**
* Displays a single language radiobutton
*
- * @param language of the item
* @param isSelected check or uncheck the radioButton if the language is selected or not
* @param onClick Callback when the user taps on Language
*/
@Composable
private fun LanguageRadioButton(
- language: Language,
isSelected: Boolean,
- onClick: (String) -> Unit,
+ onClick: () -> Unit,
) {
RadioButton(
selected = isSelected,
colors = RadioButtonDefaults.colors(selectedColor = focusColors.radioButtonSelected),
- onClick = {
- onClick(language.tag)
- },
+ onClick = onClick,
)
}
@@ -156,12 +162,12 @@ private fun LanguageRadioButton(
* @param onClick Callback when the user taps on Language text , the same like on the radioButton
*/
@Composable
-private fun LanguageDisplayName(language: Language, onClick: (String) -> Unit) {
+private fun LanguageDisplayName(language: Language, onClick: (Language) -> Unit) {
Text(
text = AnnotatedString(language.displayName!!),
style = MaterialTheme.typography.bodyLarge,
modifier = Modifier
.padding(10.dp)
- .clickable { onClick(language.tag) },
+ .clickable { onClick(language) },
)
}
