Bug 1890838 - Global Settings Entry Point For Translation Global Settings r=android-reviewers,ohall

Differential Revision: https://phabricator.services.mozilla.com/D208154
This commit is contained in:
iorgamgabriel 2024-04-30 06:22:25 +00:00
parent 255d84b782
commit b9a3133cea
17 changed files with 97 additions and 90 deletions

View file

@ -868,11 +868,12 @@ translations:
A string containing the name of the item the user tapped. These items
include:
main_flow_toolbar, main_flow_browser, page_settings, global_settings,
global_lang_settings, global_site_settings, downloads
global_lang_settings, global_site_settings, downloads, global_settings_from_preferences
type: string
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1883968
- https://bugzilla.mozilla.org/show_bug.cgi?id=1886851
- https://bugzilla.mozilla.org/show_bug.cgi?id=1890838
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1883968#c6
- https://bugzilla.mozilla.org/show_bug.cgi?id=1886851#c8

View file

@ -233,9 +233,7 @@ class DefaultBrowserToolbarController(
override fun handleTranslationsButtonClick() {
Translations.action.record(Translations.ActionExtra("main_flow_toolbar"))
val directions =
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(
sessionId = currentSession?.id,
)
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment()
navController.navigateSafe(R.id.browserFragment, directions)
}

View file

@ -424,9 +424,7 @@ class DefaultBrowserToolbarMenuController(
ToolbarMenu.Item.Translate -> {
Translations.action.record(Translations.ActionExtra("main_flow_browser"))
val directions =
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(
sessionId = currentSession?.id,
)
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment()
navController.navigateSafe(R.id.browserFragment, directions)
}
}

View file

@ -47,6 +47,7 @@ import org.mozilla.fenix.GleanMetrics.Addons
import org.mozilla.fenix.GleanMetrics.CookieBanners
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.TrackingProtection
import org.mozilla.fenix.GleanMetrics.Translations
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.components.accounts.FenixFxAEntryPoint
@ -315,6 +316,11 @@ class SettingsFragment : PreferenceFragmentCompat() {
SettingsFragmentDirections.actionSettingsFragmentToLocaleSettingsFragment()
}
resources.getString(R.string.pref_key_translation) -> {
Translations.action.record(Translations.ActionExtra("global_settings_from_preferences"))
SettingsFragmentDirections.actionSettingsFragmentToTranslationsSettingsFragment()
}
/* Privacy and security preferences */
resources.getString(R.string.pref_key_private_browsing) -> {
SettingsFragmentDirections.actionSettingsFragmentToPrivateBrowsingFragment()
@ -509,6 +515,9 @@ class SettingsFragment : PreferenceFragmentCompat() {
findPreference<Preference>(
getPreferenceKey(R.string.pref_key_sync_debug),
)?.isVisible = showSecretDebugMenuThisSession
findPreference<Preference>(
getPreferenceKey(R.string.pref_key_translation),
)?.isVisible = FxNimbus.features.translations.value().globalSettingsEnabled
preferenceStartProfiler?.isVisible = showSecretDebugMenuThisSession &&
(requireContext().components.core.engine.profiler?.isProfilerActive() != null)
}

View file

@ -17,11 +17,9 @@ import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.res.stringResource
import androidx.fragment.app.Fragment
import androidx.navigation.fragment.findNavController
import androidx.navigation.fragment.navArgs
import mozilla.components.browser.state.action.TranslationsAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.TranslationsBrowserState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.translate.TranslationPageSettingOperation
import mozilla.components.lib.state.ext.observeAsComposableState
import mozilla.components.support.base.feature.UserInteractionHandler
import org.mozilla.fenix.GleanMetrics.Translations
@ -36,7 +34,6 @@ import org.mozilla.fenix.theme.FirefoxTheme
* A fragment displaying the Firefox Translation settings screen.
*/
class TranslationSettingsFragment : Fragment(), UserInteractionHandler {
private val args by navArgs<TranslationSettingsFragmentArgs>()
private val browserStore: BrowserStore by lazy { requireComponents.core.store }
override fun onResume() {
@ -84,19 +81,19 @@ class TranslationSettingsFragment : Fragment(), UserInteractionHandler {
/**
* Set the switch item values.
* The first one is based on [TranslationPageSettings.alwaysOfferPopup].
* The first one is based on [TranslationsBrowserState.offerTranslation].
* The second one is [DownloadLanguageFileDialog] visibility.
* This pop-up will appear if the switch item is unchecked, the phone is in saving mode, and
* doesn't have a WiFi connection.
*/
@Composable
private fun getTranslationSwitchItemList(): MutableList<TranslationSwitchItem> {
val pageSettingsState = browserStore.observeAsComposableState { state ->
state.findTab(args.sessionId)?.translationsState?.pageSettings
val offerToTranslate = browserStore.observeAsComposableState { state ->
state.translationEngine.offerTranslation
}.value
val translationSwitchItems = mutableListOf<TranslationSwitchItem>()
pageSettingsState?.alwaysOfferPopup?.let {
offerToTranslate?.let {
translationSwitchItems.add(
TranslationSwitchItem(
type = TranslationSettingsScreenOption.OfferToTranslate(
@ -107,10 +104,8 @@ class TranslationSettingsFragment : Fragment(), UserInteractionHandler {
isEnabled = true,
onStateChange = { _, checked ->
browserStore.dispatch(
TranslationsAction.UpdatePageSettingAction(
tabId = args.sessionId,
operation = TranslationPageSettingOperation.UPDATE_ALWAYS_OFFER_POPUP,
setting = checked,
TranslationsAction.SetGlobalOfferTranslateSettingAction(
offerTranslation = checked,
),
)
// Ensures persistence of value
@ -141,12 +136,15 @@ class TranslationSettingsFragment : Fragment(), UserInteractionHandler {
}
override fun onBackPressed(): Boolean {
findNavController().navigate(
TranslationSettingsFragmentDirections.actionTranslationSettingsFragmentToTranslationsDialogFragment(
sessionId = args.sessionId,
translationsDialogAccessPoint = TranslationsDialogAccessPoint.TranslationsOptions,
),
)
return true
return if (findNavController().previousBackStackEntry?.destination?.id == R.id.browserFragment) {
findNavController().navigate(
TranslationSettingsFragmentDirections.actionTranslationSettingsFragmentToTranslationsDialogFragment(
translationsDialogAccessPoint = TranslationsDialogAccessPoint.TranslationsOptions,
),
)
true
} else {
false
}
}
}

View file

@ -171,6 +171,7 @@ internal fun TranslationsOptionsDialog(
context: Context,
showGlobalSettings: Boolean,
translationPageSettings: TranslationPageSettings? = null,
offerTranslation: Boolean? = null,
initialFrom: Language? = null,
onStateChange: (TranslationSettingsOption, Boolean) -> Unit,
onBackClicked: () -> Unit,
@ -181,6 +182,7 @@ internal fun TranslationsOptionsDialog(
showGlobalSettings = showGlobalSettings,
translationOptionsList = getTranslationSwitchItemList(
translationPageSettings = translationPageSettings,
offerTranslation = offerTranslation,
initialFrom = initialFrom,
context = context,
onStateChange = onStateChange,
@ -194,6 +196,7 @@ internal fun TranslationsOptionsDialog(
@Composable
private fun getTranslationSwitchItemList(
translationPageSettings: TranslationPageSettings? = null,
offerTranslation: Boolean? = null,
initialFrom: Language? = null,
context: Context,
onStateChange: (TranslationSettingsOption, Boolean) -> Unit,
@ -201,12 +204,11 @@ private fun getTranslationSwitchItemList(
val translationSwitchItemList = mutableListOf<TranslationSwitchItem>()
translationPageSettings?.let {
val alwaysOfferPopup = translationPageSettings.alwaysOfferPopup
val alwaysTranslateLanguage = translationPageSettings.alwaysTranslateLanguage
val neverTranslateLanguage = translationPageSettings.neverTranslateLanguage
val neverTranslateSite = translationPageSettings.neverTranslateSite
alwaysOfferPopup?.let {
offerTranslation?.let {
translationSwitchItemList.add(
TranslationSwitchItem(
type = TranslationPageSettingsOption.AlwaysOfferPopup(),

View file

@ -8,7 +8,7 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChangedBy
import kotlinx.coroutines.flow.mapNotNull
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.TranslationsBrowserState
@ -29,7 +29,6 @@ import java.util.Locale
class TranslationsDialogBinding(
browserStore: BrowserStore,
private val translationsDialogStore: TranslationsDialogStore,
private val sessionId: String,
private val getTranslatedPageTitle: (localizedFrom: String?, localizedTo: String?) -> String,
) : AbstractBinding<BrowserState>(browserStore) {
@ -42,7 +41,7 @@ class TranslationsDialogBinding(
}
// Session level flows
val sessionFlow = flow.mapNotNull { state -> state.findTab(sessionId) }
val sessionFlow = flow.mapNotNull { state -> state.selectedTab }
.distinctUntilChangedBy {
it.translationsState
}

View file

@ -28,7 +28,7 @@ import androidx.navigation.fragment.findNavController
import androidx.navigation.fragment.navArgs
import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.translate.Language
import mozilla.components.concept.engine.translate.TranslationError
@ -92,7 +92,6 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
listOf(
TranslationsDialogMiddleware(
browserStore = browserStore,
sessionId = args.sessionId,
settings = requireContext().settings(),
),
),
@ -245,7 +244,6 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
feature = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = args.sessionId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
requireContext().getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -384,12 +382,17 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
) {
val pageSettingsState =
browserStore.observeAsComposableState { state ->
state.findTab(args.sessionId)?.translationsState?.pageSettings
state.selectedTab?.translationsState?.pageSettings
}.value
val offerTranslation = browserStore.observeAsComposableState { state ->
state.translationEngine.offerTranslation
}.value
TranslationsOptionsDialog(
context = requireContext(),
translationPageSettings = pageSettingsState,
offerTranslation = offerTranslation,
showGlobalSettings = showGlobalSettings,
initialFrom = initialFrom,
onStateChange = { type, checked ->
@ -408,9 +411,7 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
Translations.action.record(Translations.ActionExtra("global_settings"))
findNavController().navigate(
TranslationsDialogFragmentDirections
.actionTranslationsDialogFragmentToTranslationSettingsFragment(
sessionId = args.sessionId,
),
.actionTranslationsDialogFragmentToTranslationSettingsFragment(),
)
},
aboutTranslationClicked = {
@ -425,7 +426,7 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
setFragmentResult(
TRANSLATION_IN_PROGRESS,
bundleOf(
SESSION_ID to args.sessionId,
SESSION_ID to browserStore.state.selectedTab?.id,
),
)
}

View file

@ -5,12 +5,12 @@
package org.mozilla.fenix.translations
import mozilla.components.browser.state.action.TranslationsAction
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.translate.TranslationOperation
import mozilla.components.concept.engine.translate.TranslationPageSettingOperation
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.MiddlewareContext
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.utils.Settings
/**
@ -18,16 +18,17 @@ import org.mozilla.fenix.utils.Settings
*/
class TranslationsDialogMiddleware(
private val browserStore: BrowserStore,
private val sessionId: String,
private val settings: Settings,
) : Middleware<TranslationsDialogState, TranslationsDialogAction> {
@Suppress("LongMethod")
@Suppress("LongMethod", "CyclomaticComplexMethod")
override fun invoke(
context: MiddlewareContext<TranslationsDialogState, TranslationsDialogAction>,
next: (TranslationsDialogAction) -> Unit,
action: TranslationsDialogAction,
) {
val sessionId = browserStore.state.selectedTab?.id ?: return
when (action) {
is TranslationsDialogAction.InitTranslationsDialog -> {
// If the languages are missing, we should attempt to fetch the supported languages.
@ -98,10 +99,8 @@ class TranslationsDialogMiddleware(
is TranslationPageSettingsOption.AlwaysOfferPopup -> {
// Ensures the translations engine has the correct value
browserStore.dispatch(
TranslationsAction.UpdatePageSettingAction(
tabId = sessionId,
operation = TranslationPageSettingOperation.UPDATE_ALWAYS_OFFER_POPUP,
setting = action.checkValue,
TranslationsAction.SetGlobalOfferTranslateSettingAction(
offerTranslation = action.checkValue,
),
)

View file

@ -319,6 +319,7 @@
app:destination="@id/reviewQualityCheckFragment" />
<action
android:id="@+id/action_browserFragment_to_translationsDialogFragment"
app:destination="@id/translationsDialogFragment" />
app:destination="@id/translations_graph" />
<action
android:id="@+id/action_loginsListFragment"
@ -637,6 +638,13 @@
app:exitAnim="@anim/slide_out_left"
app:popEnterAnim="@anim/slide_in_left"
app:popExitAnim="@anim/slide_out_right" />
<action
android:id="@+id/action_settingsFragment_to_translationsSettingsFragment"
app:destination="@id/translations_settings_graph"
app:enterAnim="@anim/slide_in_right"
app:exitAnim="@anim/slide_out_left"
app:popEnterAnim="@anim/slide_in_left"
app:popExitAnim="@anim/slide_out_right" />
<action
android:id="@+id/action_settingsFragment_to_addonsFragment"
app:destination="@id/addons_management_graph"
@ -1444,33 +1452,24 @@
</fragment>
</navigation>
<navigation
android:id="@+id/translations_graph"
app:startDestination="@id/translationsDialogFragment">
<dialog
android:id="@+id/translationsDialogFragment"
android:name="org.mozilla.fenix.translations.TranslationsDialogFragment">
<argument
android:name="sessionId"
app:argType="string"
app:nullable="true" />
<dialog
android:id="@+id/translationsDialogFragment"
android:name="org.mozilla.fenix.translations.TranslationsDialogFragment">
<argument
android:name="sessionId"
app:argType="string" />
<argument
android:name="translationsDialogAccessPoint"
android:defaultValue="Translations"
app:argType="org.mozilla.fenix.translations.TranslationsDialogAccessPoint" />
<action
android:id="@+id/action_translationsDialogFragment_to_translationSettingsFragment"
app:destination="@id/translationSettingsFragment" />
</dialog>
android:name="translationsDialogAccessPoint"
android:defaultValue="Translations"
app:argType="org.mozilla.fenix.translations.TranslationsDialogAccessPoint" />
<action
android:id="@+id/action_translationsDialogFragment_to_translationSettingsFragment"
app:destination="@id/translations_settings_graph" />
</dialog>
<navigation
android:id="@+id/translations_settings_graph"
app:startDestination="@id/translationSettingsFragment">
<fragment
android:id="@+id/translationSettingsFragment"
android:name="org.mozilla.fenix.translations.TranslationSettingsFragment">
<argument
android:name="sessionId"
app:argType="string" />
<action
android:id="@+id/action_translationSettingsFragment_to_translationsDialogFragment"
app:destination="@id/translationsDialogFragment"

View file

@ -17,6 +17,7 @@
<string name="pref_key_accessibility_force_enable_zoom" translatable="false">pref_key_accessibility_force_enable_zoom</string>
<string name="pref_key_advanced" translatable="false">pref_key_advanced</string>
<string name="pref_key_language" translatable="false">pref_key_language</string>
<string name="pref_key_translation" translatable="false">pref_key_translation</string>
<string name="pref_key_data_choices" translatable="false">pref_key_data_choices</string>
<string name="pref_key_delete_browsing_data" translatable="false">pref_key_delete_browsing_data</string>
<string name="pref_key_delete_browsing_data_on_quit_preference" translatable="false">pref_key_delete_browsing_data_on_quit_preference</string>

View file

@ -553,6 +553,8 @@
<string name="preferences_account_sync_error">Reconnect to resume syncing</string>
<!-- Preference for language -->
<string name="preferences_language">Language</string>
<!-- Preference for translation -->
<string name="preferences_translation">Translation</string>
<!-- Preference for data choices -->
<string name="preferences_data_choices">Data choices</string>
<!-- Preference for data collection -->

View file

@ -82,6 +82,11 @@
app:iconSpaceReserved="false"
android:title="@string/preferences_language" />
<androidx.preference.Preference
android:key="@string/pref_key_translation"
app:iconSpaceReserved="false"
android:title="@string/preferences_translation" />
<org.mozilla.fenix.settings.DefaultBrowserPreference
android:key="@string/pref_key_make_default_browser"
app:iconSpaceReserved="false"

View file

@ -454,9 +454,7 @@ class DefaultBrowserToolbarControllerTest {
verify {
navController.navigate(
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(
sessionId = "1",
),
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(),
)
}

View file

@ -837,9 +837,7 @@ class DefaultBrowserToolbarMenuControllerTest {
verify {
navController.navigate(
directionsEq(
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(
sessionId = selectedTab.id,
),
BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(),
),
)
}

View file

@ -56,7 +56,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -138,7 +137,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -184,7 +182,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -231,7 +228,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -281,7 +277,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -321,7 +316,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -359,7 +353,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,
@ -410,7 +403,6 @@ class TranslationsDialogBindingTest {
val binding = TranslationsDialogBinding(
browserStore = browserStore,
translationsDialogStore = translationsDialogStore,
sessionId = tabId,
getTranslatedPageTitle = { localizedFrom, localizedTo ->
testContext.getString(
R.string.translations_bottom_sheet_title_translation_completed,

View file

@ -4,10 +4,12 @@
package org.mozilla.fenix.translations
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import mozilla.components.browser.state.action.TranslationsAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.translate.Language
import mozilla.components.concept.engine.translate.TranslationOperation
@ -24,10 +26,17 @@ import org.mozilla.fenix.utils.Settings
@RunWith(FenixRobolectricTestRunner::class)
class TranslationsDialogMiddlewareTest {
private val browserStore = mockk<BrowserStore>(relaxed = true)
private val browserStore = spyk(
BrowserStore(
BrowserState(
tabs = listOf(createTab("https://www.mozilla.org", id = "tab1")),
selectedTabId = "tab1",
),
),
)
private val settings = Settings(testContext)
private val translationsDialogMiddleware =
TranslationsDialogMiddleware(browserStore = browserStore, sessionId = "tab1", settings = settings)
TranslationsDialogMiddleware(browserStore = browserStore, settings = settings)
@Test
fun `GIVEN translationState WHEN FetchSupportedLanguages action is called THEN call OperationRequestedAction from BrowserStore`() =
@ -164,10 +173,8 @@ class TranslationsDialogMiddlewareTest {
verify {
browserStore.dispatch(
TranslationsAction.UpdatePageSettingAction(
tabId = "tab1",
operation = TranslationPageSettingOperation.UPDATE_ALWAYS_OFFER_POPUP,
setting = false,
TranslationsAction.SetGlobalOfferTranslateSettingAction(
offerTranslation = false,
),
)
}