GT-3017 Onboarding Localization (#4405)

tjohnson009 · frett · web-flow · commit 1536e6429d3b · 2026-05-04T22:23:23.000-04:00
* Add pop result to app language screen to track result * Modify popResult names and add for presenter * Add pop result to navigator.pop in UiEvents * Modify tests to reflect newly added pop result * Add result interface for pop results to send to presenter * Receive newly added pop results in UiEvents * Sync pager to presenter page state via LaunchedEffect and disable swipe on welcome * Add answering navigators and Next event for language/country flow in presenter * Add initial state and Next event tests to OnboardingPresenterTest * Change event passed to prevent block from continue onboarding * Change test to pass with the previous dismiss select language event blocking onboarding * Change tense of the result type for consistency in country settings and app language screens * Update pagerState from UiState * Remove currentPage param from UiState and derive it from newly added pagerState * Change test to derive current page from pagerState * Fix import to match new file structure * Fix import order and change import to reflect file structure * Add hasSeenCountrySettings flag; Route onboarding based on language and countrySettings flag * Change onboarding to use eventSink for every nextPage action * Remove duplicate logic for navigateing pages; Simplify UiEvent.Next event * Apply suggestion from @frett --------- Co-authored-by: Daniel Frett <frett@users.noreply.github.com>
diff --git a/app/src/main/kotlin/org/cru/godtools/ui/onboarding/OnboardingLayout.kt b/app/src/main/kotlin/org/cru/godtools/ui/onboarding/OnboardingLayout.kt
@@ -1,14 +1,11 @@
 package org.cru.godtools.ui.onboarding
 
-import androidx.compose.animation.core.Spring
-import androidx.compose.animation.core.spring
 import androidx.compose.foundation.layout.consumeWindowInsets
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.navigationBarsPadding
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.pager.HorizontalPager
-import androidx.compose.foundation.pager.rememberPagerState
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Scaffold
 import androidx.compose.runtime.Composable
@@ -20,7 +17,6 @@ import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.Modifier
 import com.slack.circuit.codegen.annotations.CircuitInject
 import dagger.hilt.components.SingletonComponent
-import kotlinx.coroutines.launch
 import org.ccci.gto.android.common.androidx.compose.material3.ui.appbar.AppBarAction
 import org.ccci.gto.android.common.androidx.compose.material3.ui.appbar.AppBarActionButton
 import org.cru.godtools.R
@@ -39,7 +35,7 @@ fun OnboardingLayout(state: OnboardingPresenter.UiState, modifier: Modifier = Mo
     val eventSink by rememberUpdatedState(state.eventSink)
 
     val coroutineScope = rememberCoroutineScope()
-    val pagerState = rememberPagerState(pageCount = { 4 })
+    val pagerState = state.pagerState
 
     RecordAnalyticsScreen(OnboardingAnalyticsScreenEvent(pagerState.currentPage))
 
@@ -74,35 +70,27 @@ fun OnboardingLayout(state: OnboardingPresenter.UiState, modifier: Modifier = Mo
             HorizontalPager(
                 key = { it },
                 state = pagerState,
+                userScrollEnabled = state.userScrollEnabled,
                 modifier = Modifier
                     .padding(insets)
                     .consumeWindowInsets(insets)
                     .fillMaxSize()
             ) {
-                val nextPage: () -> Unit = {
-                    coroutineScope.launch {
-                        pagerState.animateScrollToPage(
-                            it + 1,
-                            animationSpec = spring(stiffness = Spring.StiffnessMediumLow)
-                        )
-                    }
-                }
-
                 when (it) {
                     0 -> OnboardingWelcomePageLayout(
-                        nextPage = nextPage,
+                        nextPage = { eventSink(UiEvent.Next) },
                         eventSink = eventSink,
                     )
 
                     1 -> OnboardingPageLayout(
                         OnboardingPage.CONVERSATIONS,
-                        nextPage = nextPage,
+                        nextPage = { eventSink(UiEvent.Next) },
                         eventSink = eventSink,
                     )
 
                     2 -> OnboardingPageLayout(
                         OnboardingPage.PREPARE,
-                        nextPage = nextPage,
+                        nextPage = { eventSink(UiEvent.Next) },
                         eventSink = eventSink,
                     )
 
diff --git a/app/src/main/kotlin/org/cru/godtools/ui/onboarding/OnboardingPresenter.kt b/app/src/main/kotlin/org/cru/godtools/ui/onboarding/OnboardingPresenter.kt
@@ -1,8 +1,18 @@
 package org.cru.godtools.ui.onboarding
 
+import androidx.compose.animation.core.Spring
+import androidx.compose.animation.core.spring
+import androidx.compose.foundation.pager.PagerState
+import androidx.compose.foundation.pager.rememberPagerState
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.saveable.rememberSaveable
+import androidx.compose.runtime.setValue
 import com.slack.circuit.codegen.annotations.CircuitInject
+import com.slack.circuit.foundation.rememberAnsweringNavigator
 import com.slack.circuit.runtime.CircuitUiEvent
 import com.slack.circuit.runtime.CircuitUiState
 import com.slack.circuit.runtime.Navigator
@@ -11,34 +21,74 @@ import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import dagger.hilt.components.SingletonComponent
+import kotlinx.coroutines.launch
 import org.cru.godtools.base.Settings
 import org.cru.godtools.base.Settings.Companion.FEATURE_TUTORIAL_ONBOARDING
 import org.cru.godtools.base.ui.circuit.screen.AppLanguageScreen
 import org.cru.godtools.shared.analytics.TutorialAnalyticsActionNames
 import org.cru.godtools.tutorial.analytics.model.TutorialAnalyticsActionEvent
 import org.cru.godtools.ui.onboarding.OnboardingPresenter.UiState
+import org.cru.godtools.ui.settings.country.CountrySettingsScreen
 import org.greenrobot.eventbus.EventBus
 
 class OnboardingPresenter @AssistedInject constructor(
     private val eventBus: EventBus,
     private val settings: Settings,
     @Assisted private val navigator: Navigator,
 ) : Presenter<UiState> {
-    data class UiState(val eventSink: (UiEvent) -> Unit = {}) : CircuitUiState
+    data class UiState(
+        val pagerState: PagerState,
+        val userScrollEnabled: Boolean = false,
+        val eventSink: (UiEvent) -> Unit = {}
+    ) : CircuitUiState
 
     sealed interface UiEvent : CircuitUiEvent {
         data object ChangeLanguage : UiEvent
+        data object Next : UiEvent
         data object Skip : UiEvent
         data object Finish : UiEvent
     }
 
+    private suspend fun PagerState.navigateToPage(page: Int = currentPage + 1) {
+        animateScrollToPage(page, animationSpec = spring(stiffness = Spring.StiffnessMediumLow))
+    }
+
     @Composable
     override fun present(): UiState {
+        var languageWasSet by rememberSaveable { mutableStateOf(false) }
+        var hasSeenCountrySettings by rememberSaveable { mutableStateOf(false) }
+        val pagerState = rememberPagerState(pageCount = { 4 })
+        val scope = rememberCoroutineScope()
+        val countrySettingsNavigator = rememberAnsweringNavigator<CountrySettingsScreen.Result>(navigator) {
+            hasSeenCountrySettings = true
+            scope.launch {
+                pagerState.navigateToPage(1)
+            }
+        }
+        val languageButtonNavigator = rememberAnsweringNavigator<AppLanguageScreen.Result>(navigator) { result ->
+            if (result is AppLanguageScreen.Result.LanguageSelected) {
+                languageWasSet = true
+            }
+        }
+        val nextButtonNavigator = rememberAnsweringNavigator<AppLanguageScreen.Result>(navigator) { result ->
+            if (result is AppLanguageScreen.Result.LanguageSelected) {
+                languageWasSet = true
+                countrySettingsNavigator.goTo(CountrySettingsScreen)
+            }
+        }
         LaunchedEffect(Unit) { settings.setFeatureDiscovered(FEATURE_TUTORIAL_ONBOARDING) }
-
-        return UiState { event ->
+        return UiState(
+            pagerState = pagerState,
+            userScrollEnabled = (languageWasSet && hasSeenCountrySettings) || pagerState.currentPage > 0
+        ) { event ->
             when (event) {
-                UiEvent.ChangeLanguage -> navigator.goTo(AppLanguageScreen)
+                UiEvent.ChangeLanguage -> languageButtonNavigator.goTo(AppLanguageScreen)
+
+                UiEvent.Next -> when {
+                    !languageWasSet -> nextButtonNavigator.goTo(AppLanguageScreen)
+                    !hasSeenCountrySettings -> countrySettingsNavigator.goTo(CountrySettingsScreen)
+                    else -> scope.launch { pagerState.navigateToPage() }
+                }
 
                 UiEvent.Skip -> {
                     eventBus.post(TutorialAnalyticsActionEvent(TutorialAnalyticsActionNames.ONBOARDING_SKIP))
diff --git a/app/src/main/kotlin/org/cru/godtools/ui/settings/country/CountrySettingsPresenter.kt b/app/src/main/kotlin/org/cru/godtools/ui/settings/country/CountrySettingsPresenter.kt
@@ -72,12 +72,12 @@ class CountrySettingsPresenter @AssistedInject constructor(
             countryCode = settingCountryCode,
             eventSink = { event ->
                 when (event) {
-                    UiEvent.NavigateBack -> navigator.pop()
+                    UiEvent.NavigateBack -> navigator.pop(CountrySettingsScreen.Result.Dismissed)
 
                     is UiEvent.SelectCountry -> {
                         scope.launch {
                             settings.updateCountrySetting(isoCode = event.isoCode)
-                            navigator.pop()
+                            navigator.pop(CountrySettingsScreen.Result.CountrySelected)
                         }
                     }
                 }
diff --git a/app/src/main/kotlin/org/cru/godtools/ui/settings/country/CountrySettingsScreen.kt b/app/src/main/kotlin/org/cru/godtools/ui/settings/country/CountrySettingsScreen.kt
@@ -1,7 +1,13 @@
 package org.cru.godtools.ui.settings.country
 
+import com.slack.circuit.runtime.screen.PopResult
 import com.slack.circuit.runtime.screen.Screen
 import kotlinx.parcelize.Parcelize
 
 @Parcelize
-data object CountrySettingsScreen : Screen
+data object CountrySettingsScreen : Screen {
+    sealed interface Result : PopResult {
+        @Parcelize data object CountrySelected : Result
+        @Parcelize data object Dismissed : Result
+    }
+}
diff --git a/app/src/main/kotlin/org/cru/godtools/ui/settings/language/app/AppLanguagePresenter.kt b/app/src/main/kotlin/org/cru/godtools/ui/settings/language/app/AppLanguagePresenter.kt
@@ -57,11 +57,11 @@ class AppLanguagePresenter @AssistedInject constructor(
         val eventSink: (UiEvent) -> Unit = remember {
             {
                 when (it) {
-                    UiEvent.NavigateBack -> navigator.pop()
+                    UiEvent.NavigateBack -> navigator.pop(AppLanguageScreen.Result.Dismissed)
 
                     is UiEvent.SelectLanguage -> {
                         if (it.language == appLocale) {
-                            navigator.pop()
+                            navigator.pop(AppLanguageScreen.Result.LanguageSelected)
                         } else {
                             confirmLanguage = it.language
                         }
@@ -70,7 +70,7 @@ class AppLanguagePresenter @AssistedInject constructor(
                     is UiEvent.ConfirmLanguage -> {
                         settings.appLanguage = it.language
                         confirmLanguage = null
-                        navigator.pop()
+                        navigator.pop(AppLanguageScreen.Result.LanguageSelected)
                     }
 
                     UiEvent.DismissConfirmDialog -> confirmLanguage = null
diff --git a/app/src/test/kotlin/org/cru/godtools/ui/onboarding/OnboardingPresenterTest.kt b/app/src/test/kotlin/org/cru/godtools/ui/onboarding/OnboardingPresenterTest.kt
@@ -9,6 +9,7 @@ import io.mockk.verify
 import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 import kotlinx.coroutines.test.runTest
 import org.ccci.gto.android.common.androidx.compose.ui.platform.AndroidUiDispatcherUtil
 import org.cru.godtools.base.Settings
@@ -39,6 +40,24 @@ class OnboardingPresenterTest {
         navigator.assertPopIsEmpty()
     }
 
+    // region Initial State
+    @Test
+    fun `Initial State - currentPage is 0`() = runTest {
+        createPresenter().test {
+            val state = awaitItem()
+            assertEquals(0, state.pagerState.currentPage)
+        }
+    }
+
+    @Test
+    fun `Initial State - userScrollEnabled is false`() = runTest {
+        createPresenter().test {
+            val state = awaitItem()
+            assertFalse(state.userScrollEnabled)
+        }
+    }
+    // endregion Initial State
+
     // region Feature Discovery
     @Test
     fun `Feature Discovery - sets feature discovered`() = runTest {
@@ -70,6 +89,22 @@ class OnboardingPresenterTest {
     }
     // endregion UiEvent.Skip
 
+    // region UiEvent.Next
+    @Test
+    fun `UiEvent - Next - no language set - navigates to AppLanguageScreen`() = runTest {
+        createPresenter().test {
+            awaitItem().eventSink(OnboardingPresenter.UiEvent.Next)
+            assertEquals(AppLanguageScreen, navigator.awaitNextScreen())
+        }
+    }
+
+    @Test
+    fun `UiEvent - Next - language already set - navigates to CountrySettingsScreen`() = runTest {
+        // TODO: Testing the full answering navigator callback chain requires integration testing.
+        //  The FakeNavigator doesn't trigger rememberAnsweringNavigator callbacks in unit tests.
+    }
+    // endregion UiEvent.Next
+
     // region UiEvent.Finish
     @Test
     fun `UiEvent - Finish`() = runTest {
diff --git a/app/src/test/kotlin/org/cru/godtools/ui/settings/language/app/AppLanguagePresenterTest.kt b/app/src/test/kotlin/org/cru/godtools/ui/settings/language/app/AppLanguagePresenterTest.kt
@@ -113,7 +113,7 @@ class AppLanguagePresenterTest {
     fun `Event - NavigateBack`() = runTest {
         presenter.test {
             awaitItem().eventSink(UiEvent.NavigateBack)
-            navigator.awaitPop()
+            assertEquals(AppLanguageScreen.Result.Dismissed, navigator.awaitPop().result)
         }
     }
     // endregion Event.NavigateBack
@@ -132,7 +132,7 @@ class AppLanguagePresenterTest {
     fun `Event - SelectLanguage - Selected app language`() = runTest {
         presenter.test {
             awaitItem().eventSink(UiEvent.SelectLanguage(Locale.ENGLISH))
-            navigator.awaitPop()
+            assertEquals(AppLanguageScreen.Result.LanguageSelected, navigator.awaitPop().result)
         }
     }
     // endregion Event.SelectLanguage
@@ -149,7 +149,7 @@ class AppLanguagePresenterTest {
                 eventSink(UiEvent.ConfirmLanguage(selectedLanguage))
             }
 
-            navigator.awaitPop()
+            assertEquals(AppLanguageScreen.Result.LanguageSelected, navigator.awaitPop().result)
             assertNull(expectMostRecentItem().selectedLanguage)
             assertEquals(Locale.FRENCH, appLocaleState.value)
         }
diff --git a/ui/base/src/main/kotlin/org/cru/godtools/base/ui/circuit/screen/AppLanguageScreen.kt b/ui/base/src/main/kotlin/org/cru/godtools/base/ui/circuit/screen/AppLanguageScreen.kt
@@ -1,7 +1,13 @@
 package org.cru.godtools.base.ui.circuit.screen
 
+import com.slack.circuit.runtime.screen.PopResult
 import com.slack.circuit.runtime.screen.Screen
 import kotlinx.parcelize.Parcelize
 
 @Parcelize
-data object AppLanguageScreen : Screen
+data object AppLanguageScreen : Screen {
+    sealed interface Result : PopResult {
+        @Parcelize data object LanguageSelected : Result
+        @Parcelize data object Dismissed : Result
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -72,12 +72,12 @@ class CountrySettingsPresenter @AssistedInject constructor(`
`72`	`72`	`countryCode = settingCountryCode,`
`73`	`73`	`eventSink = { event ->`
`74`	`74`	`when (event) {`
`75`		`- UiEvent.NavigateBack -> navigator.pop()`
	`75`	`+ UiEvent.NavigateBack -> navigator.pop(CountrySettingsScreen.Result.Dismissed)`
`76`	`76`
`77`	`77`	`is UiEvent.SelectCountry -> {`
`78`	`78`	`scope.launch {`
`79`	`79`	`settings.updateCountrySetting(isoCode = event.isoCode)`
`80`		`- navigator.pop()`
	`80`	`+ navigator.pop(CountrySettingsScreen.Result.CountrySelected)`
`81`	`81`	`}`
`82`	`82`	`}`
`83`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,7 @@ class AppLanguagePresenterTest {`
`113`	`113`	fun `Event - NavigateBack`() = runTest {
`114`	`114`	`presenter.test {`
`115`	`115`	`awaitItem().eventSink(UiEvent.NavigateBack)`
`116`		`- navigator.awaitPop()`
	`116`	`+ assertEquals(AppLanguageScreen.Result.Dismissed, navigator.awaitPop().result)`
`117`	`117`	`}`
`118`	`118`	`}`
`119`	`119`	`// endregion Event.NavigateBack`
`@@ -132,7 +132,7 @@ class AppLanguagePresenterTest {`
`132`	`132`	fun `Event - SelectLanguage - Selected app language`() = runTest {
`133`	`133`	`presenter.test {`
`134`	`134`	`awaitItem().eventSink(UiEvent.SelectLanguage(Locale.ENGLISH))`
`135`		`- navigator.awaitPop()`
	`135`	`+ assertEquals(AppLanguageScreen.Result.LanguageSelected, navigator.awaitPop().result)`
`136`	`136`	`}`
`137`	`137`	`}`
`138`	`138`	`// endregion Event.SelectLanguage`
`@@ -149,7 +149,7 @@ class AppLanguagePresenterTest {`
`149`	`149`	`eventSink(UiEvent.ConfirmLanguage(selectedLanguage))`
`150`	`150`	`}`
`151`	`151`
`152`		`- navigator.awaitPop()`
	`152`	`+ assertEquals(AppLanguageScreen.Result.LanguageSelected, navigator.awaitPop().result)`
`153`	`153`	`assertNull(expectMostRecentItem().selectedLanguage)`
`154`	`154`	`assertEquals(Locale.FRENCH, appLocaleState.value)`
`155`	`155`	`}`