Skip to content

feat(language): add language selection to onboarding #1973

Open
mena-rizkalla wants to merge 20 commits intoopenMF:developmentfrom
mena-rizkalla:feat/install-language-selection
Open

feat(language): add language selection to onboarding #1973
mena-rizkalla wants to merge 20 commits intoopenMF:developmentfrom
mena-rizkalla:feat/install-language-selection

Conversation

@mena-rizkalla
Copy link

@mena-rizkalla mena-rizkalla commented Feb 24, 2026

This PR addresses MW-335
by allowing users to select their preferred language rather than strictly defaulting to English. It introduces language selection both during the initial installation/onboarding flow and within the app settings.

Screenshots

Onboarding Onboarding
image2 image3
document_5859593426319713362.mp4

Description

  • Apply the AndroidStyle.xml style template to your code in Android Studio.

  • Run the unit tests with ./gradlew check to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

Summary by CodeRabbit

  • New Features

    • Onboarding language selection screen added; choice is persisted and onboarding can be skipped thereafter.
    • Language option added to Settings with a dedicated Language screen and navigation.
    • App applies selected language at runtime.
  • Localization

    • Arabic locale added and supported languages expanded (EN, FR, ES, DE, PT, RU, HI, ZH, JA, TR, AR, UR, BN, ID, KM, KN, TE, MY, PL, SW, FA).
  • Design

    • New Language icon added to the design system.

- Add `LanguageConfig` model with support for multiple locales.
- Implement `OnboardingLanguageScreen` to allow users to select their preferred language during the initial setup.
- Add `LanguageScreen` within the settings feature to allow language changes after onboarding.
- Update `UserPreferencesRepository` and `UserPreferencesDataSource` to persist language settings and onboarding status.
- Integrate language selection into the main navigation flow and `MainActivity` to apply selected locales using `AppCompatDelegate`.
- Add localized string resources for English and Arabic.
…-wallet into feat/install-language-selection

# Conflicts:
#	cmp-shared/src/commonMain/kotlin/org/mifospay/shared/navigation/MifosNavHost.kt
#	core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt
#	core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesRepository.kt
#	core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesRepositoryImpl.kt
#	core/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/icon/MifosIcons.kt
#	feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/SettingsScreen.kt
#	feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/navigation/SettingsNavigation.kt
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new onboarding-language feature module, persists and exposes language and onboarding flags in datastore/repository, wires language selection into onboarding and settings navigation, applies runtime locales in MainActivity, and registers DI and resources for language UI.

Changes

Cohort / File(s) Summary
Project & Gradle
settings.gradle.kts, cmp-shared/build.gradle.kts, feature/onboarding-language/build.gradle.kts, feature/settings/build.gradle.kts
Adds :feature:onboarding-language to the build and registers onboardingLanguage dependency in shared/commonMain; updates settings and settings feature dependencies.
Runtime Classpath
cmp-android/dependencies/prodReleaseRuntimeClasspath.txt, cmp-android/dependencies/prodReleaseRuntimeClasspath.tree.txt
Registers feature:onboarding-language in runtime classpath and expands dependency graph with additional transitive edges.
Android Entrypoint
cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt
Changes Activity base to AppCompatActivity and applies selected locale via AppCompatDelegate.setApplicationLocales when main UI state reports language.
Shared app wiring
cmp-shared/src/.../MifosPayApp.kt, .../MifosPayViewModel.kt, navigation/MifosNavHost.kt, navigation/RootNavGraph.kt, di/KoinModules.kt
ViewModel now emits language and showOnboarding; onboarding gating added to app start; navigation wired to onboarding-language route; onboardingLanguageModule added to DI modules.
Datastore / Repo
core/datastore/.../UserPreferencesDataSource.kt, UserPreferencesRepository.kt, UserPreferencesRepositoryImpl.kt
Adds persistent keys for LANGUAGE and SHOW_ONBOARDING, exposes language and showOnboarding StateFlows, adds suspend setters, encoding helpers, and preserves/restores values during clear.
Model & Design
core/model/.../LanguageConfig.kt, core/designsystem/.../MifosIcons.kt
Introduces serializable LanguageConfig enum and exposes MifosIcons.Language icon.
Onboarding Language feature
feature/onboarding-language/src/.../OnboardingLanguageScreen.kt, .../OnboardingLanguageViewModel.kt, .../navigation/OnboardingLanguageNavigation.kt, .../di/OnboardingLanguageModule.kt, src/androidMain/AndroidManifest.xml, .gitignore
New feature module with UI, ViewModel, navigation route, Koin module, localized resources and placeholder manifest.
Settings language feature
feature/settings/src/.../LanguageScreen.kt, .../LanguageViewModel.kt, .../navigation/LanguageNavigation.kt, .../SettingsScreen.kt, .../SettingsViewModel.kt, .../di/SettingsModule.kt
Adds language-change UI and ViewModel, navigation wiring, settings UI item to trigger language screen, and registers LanguageViewModel in settings DI.
String resources
feature/onboarding-language/src/commonMain/composeResources/values*/strings.xml, feature/settings/src/commonMain/composeResources/values*/strings.xml
Adds English and Arabic localized strings for onboarding-language and settings language screens.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant App as MainActivity
    participant OnboardingVM as OnboardingLanguageViewModel
    participant Repo as UserPreferencesRepository
    participant Datastore as Datastore
    participant Locale as AppCompatDelegate

    User->>App: Launch app / trigger onboarding
    App->>OnboardingVM: Observe onboarding language state
    OnboardingVM->>Repo: subscribe to language flow
    Repo->>Datastore: read stored language
    Datastore-->>Repo: return language
    Repo-->>OnboardingVM: emit language
    User->>OnboardingVM: select language + submit
    OnboardingVM->>Repo: setLanguage(selected) & setShowOnboarding(false)
    Repo->>Datastore: persist language and flag
    Datastore-->>Repo: ack
    OnboardingVM-->>App: emit NavigateToNext
    App->>Locale: setApplicationLocales(selected)
Loading
sequenceDiagram
    participant User as User
    participant Settings as SettingsScreen
    participant SettingsVM as SettingsViewModel
    participant LanguageVM as LanguageViewModel
    participant Repo as UserPreferencesRepository
    participant Datastore as Datastore

    User->>Settings: Tap "Change Language"
    Settings->>SettingsVM: dispatch ChangeLanguage
    SettingsVM-->>Settings: emit OnNavigateToLanguageScreen
    Settings->>LanguageVM: navigate to LanguageScreen
    LanguageVM->>Repo: subscribe language flow
    Repo->>Datastore: fetch language
    Datastore-->>Repo: return language
    Repo-->>LanguageVM: emit current language
    User->>LanguageVM: pick language + submit
    LanguageVM->>Repo: setLanguage(selected)
    Repo->>Datastore: persist language
    Datastore-->>Repo: ack
    LanguageVM-->>Settings: emit NavigateBack
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • sahilshivekar
  • HekmatullahAmin

Poem

🐰 Hop, hop, a language patch I bring,

Little choices flutter, locales sing,
Select and save, the app greets you new,
From onboarding to settings, strings align,
I nibble defaults and hop off—voilà, adieu!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat(language): add language selection to onboarding' accurately reflects the primary change: introducing language selection capability during the onboarding flow, which is the central objective of this changeset.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt (1)

81-90: ⚠️ Potential issue | 🟠 Major

startDestination is computed reactively but NavHost ignores post-composition changes — users will not reach the onboarding language screen.

NavHost only reads startDestination during initial composition. The combine() in MifosPayViewModel uses SharingStarted.WhileSubscribed(5_000), so the transition from LoadingSuccess(showOnboarding=true) occurs after the NavHost is already initialized:

  1. Initial composition: uiState = LoadingnavDestination = LOGIN_GRAPH → NavHost initializes here
  2. Flows emit: uiState transitions to Success(showOnboarding=true)navDestination = ONBOARDING_LANGUAGE_ROUTE
  3. NavHost silently ignores the change to startDestination (it only applies on first composition)

Result: Fresh-install users see the login screen and never reach onboarding language selection.

Replace the reactive startDestination with a LaunchedEffect that navigates imperatively once the state resolves:

🛠️ Proposed fix
-    val navDestination = when (uiState) {
-        is MainUiState.Loading -> LOGIN_GRAPH
-        is Success -> if ((uiState as Success).showOnboarding) {
-            ONBOARDING_LANGUAGE_ROUTE
-        } else if ((uiState as Success).userData.authenticated) {
-            PASSCODE_GRAPH
-        } else {
-            LOGIN_GRAPH
-        }
-    }
+    LaunchedEffect(uiState) {
+        if (uiState is Success) {
+            val state = uiState as Success
+            val target = when {
+                state.showOnboarding -> ONBOARDING_LANGUAGE_ROUTE
+                state.userData.authenticated -> PASSCODE_GRAPH
+                else -> LOGIN_GRAPH
+            }
+            navController.navigate(target) {
+                popUpTo(navController.graph.id) { inclusive = true }
+            }
+        }
+    }

Set startDestination to a stable initial value:

         RootNavGraph(
             ...
-            startDestination = navDestination,
+            startDestination = LOGIN_GRAPH,
             ...
         )

Also fix the unnecessary double cast of uiState at lines 83 and 85.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt` around
lines 81 - 90, The computed reactive startDestination (navDestination based on
uiState) is ignored by NavHost after initial composition; change
startDestination to a stable default (e.g., LOGIN_GRAPH) and add a
LaunchedEffect keyed on uiState that imperatively navigates to
ONBOARDING_LANGUAGE_ROUTE or PASSCODE_GRAPH when uiState transitions to Success;
locate the navDestination computation and replace it with a stable start value,
implement a LaunchedEffect that checks uiState is Success and then calls
navController.navigate(...) to the appropriate route (ONBOARDING_LANGUAGE_ROUTE
or PASSCODE_GRAPH), and remove the unnecessary double casts like (uiState as
Success) by using a single smart-cast or local val success = uiState as Success
in the LaunchedEffect.
🧹 Nitpick comments (8)
settings.gradle.kts (1)

89-89: Move the new module include into the :feature: group.

:feature:onboarding-language is appended after the :libs: block (line 88), breaking the existing grouping where all feature modules are declared together (lines 63–86).

♻️ Proposed fix: relocate within the feature group
 include(":feature:fast-mpay")
+include(":feature:onboarding-language")
 
 include(":libs:mifos-passcode")
-include(":feature:onboarding-language")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@settings.gradle.kts` at line 89, The new module include
include(":feature:onboarding-language") is placed after the :libs: block,
breaking the feature modules grouping; move the
include(":feature:onboarding-language") entry up into the existing :feature:
group where other feature includes are declared (the block that contains
includes like include(":feature:...") between the start of feature declarations
and before the :libs: block) so all feature modules remain grouped together.
feature/onboarding-language/build.gradle.kts (1)

25-25: uiToolingPreview belongs outside the production commonMain source set.

compose.components.uiToolingPreview is a tooling artifact intended for @Preview support during development. Including it in commonMain adds it to all production builds unnecessarily. Consider scoping it to a non-production source set (e.g., using debugImplementation on Android targets or a dedicated preview source set).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@feature/onboarding-language/build.gradle.kts` at line 25,
compose.components.uiToolingPreview is currently declared in the production
commonMain via implementation(compose.components.uiToolingPreview); move this
dependency out of commonMain and scope it to a non-production source set instead
(for example replace the implementation call with debugImplementation for
Android targets or add it to a dedicated preview source set used only in
development). Update the build script so compose.components.uiToolingPreview is
only referenced from the debug/preview configuration and remove it from the
commonMain dependencies to avoid shipping tooling artifacts in production.
cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt (1)

83-89: Extract uiState as Success to a local val to avoid the double cast.

Since uiState is a delegated property (by), Kotlin cannot smart-cast it. The current code casts it twice. Extracting to a local eliminates the repetition.

♻️ Proposed refactor
-        is Success -> if ((uiState as Success).showOnboarding) {
-            ONBOARDING_LANGUAGE_ROUTE
-        } else if ((uiState as Success).userData.authenticated) {
-            PASSCODE_GRAPH
-        } else {
-            LOGIN_GRAPH
-        }
+        is Success -> {
+            val state = uiState as Success
+            if (state.showOnboarding) ONBOARDING_LANGUAGE_ROUTE
+            else if (state.userData.authenticated) PASSCODE_GRAPH
+            else LOGIN_GRAPH
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt` around
lines 83 - 89, Extract the casted uiState into a local val to avoid repeating
the cast: inside the branch handling the Success sealed case, create a local val
like success = uiState as Success and then use success.showOnboarding and
success.userData.authenticated to decide between ONBOARDING_LANGUAGE_ROUTE,
PASSCODE_GRAPH, and LOGIN_GRAPH; update the conditionals in the Success branch
to reference that local val instead of casting uiState twice.
feature/settings/src/commonMain/composeResources/values/strings.xml (1)

30-32: feature_onboarding_* keys are duplicated across modules.

These three keys are defined identically in both feature/onboarding-language/src/commonMain/composeResources/values/strings.xml and this file. Since Compose Multiplatform generates a separate Res class per module, there is no runtime conflict, but any future copy change must be applied in two places. Consider either:

  • Moving these strings to a shared/core resources module, or
  • Declaring a module dependency from feature/settings on feature/onboarding-language and using its Res directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@feature/settings/src/commonMain/composeResources/values/strings.xml` around
lines 30 - 32, The three duplicate Compose resource keys
feature_onboarding_choose_your_app_language,
feature_onboarding_chosen_language_can_be_changed_later_in_the_settings, and
feature_onboarding_submit are defined in both feature/settings and
feature/onboarding-language; remove the duplication by either (A) moving these
string entries into a shared/core resources module (e.g., common resources
module) and update consumers to use that shared Res, or (B) add a module
dependency from feature/settings to feature/onboarding-language and delete the
local entries in feature/settings so code in settings references
feature/onboarding-language's Res (the generated Res class and the three
resource keys) instead; ensure only one definition remains and update module
dependencies/imports accordingly.
core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt (1)

137-139: language and showOnboarding are exposed as MutableStateFlow, allowing external mutation.

val language = _language          // type: MutableStateFlow<LanguageConfig>
val showOnboarding = _showOnboarding  // type: MutableStateFlow<Boolean>

Any consumer can call .value = ... directly, bypassing setLanguage/setShowOnboarding and skipping persistence. This is a pre-existing pattern in this file (same issue with selectedInstance, accountExternalIds, etc.), but since you're adding new fields, consider narrowing the type to StateFlow:

♻️ Proposed change: expose as read-only StateFlow
-val language = _language
+val language: StateFlow<LanguageConfig> = _language

-val showOnboarding = _showOnboarding
+val showOnboarding: StateFlow<Boolean> = _showOnboarding
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt`
around lines 137 - 139, language and showOnboarding are currently exposed as
MutableStateFlow which allows external mutation and bypasses persistence; change
their declarations to expose them as read-only StateFlow by typing them as
StateFlow (e.g., val language: StateFlow<LanguageConfig> = _language and val
showOnboarding: StateFlow<Boolean> = _showOnboarding) so consumers must call
setLanguage / setShowOnboarding to update values and trigger persistence; update
any references expecting MutableStateFlow to use StateFlow where appropriate.
feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt (2)

139-157: LanguageSelectionItem differs from LanguageScreen.LanguageItem in callback signature and double-fires on RadioButton click.

LanguageSelectionItem passes onSelect: (LanguageConfig) -> Unit to the RadioButton.onClick, while the outer Row.clickable also fires onSelect. In LanguageScreen.kt, LanguageItem uses a no-arg onLanguageSelected: () -> Unit on both, which is the safer idiom (callers capture the specific language in the lambda). Align these two to reduce confusion and potential future bugs:

♻️ Align signature with LanguageItem
 `@Composable`
 fun LanguageSelectionItem(
     language: LanguageConfig,
     isSelected: Boolean,
-    onSelect: (LanguageConfig) -> Unit,
+    onSelect: () -> Unit,
 ) {
     Row(
         modifier = Modifier
             .fillMaxWidth()
-            .clickable { onSelect(language) }
+            .clickable(onClick = onSelect)
             ...
     ) {
         RadioButton(
             selected = isSelected,
-            onClick = { onSelect(language) },
+            onClick = onSelect,
         )

Update call site at line 125:

-    onSelect = { selectedLanguage = it },
+    onSelect = { selectedLanguage = language },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`
around lines 139 - 157, LanguageSelectionItem currently exposes onSelect:
(LanguageConfig) -> Unit and both Row.clickable and RadioButton.onClick call it,
causing a double-fire; change its callback to a no-arg () -> Unit (matching
LanguageScreen.LanguageItem's onLanguageSelected) and use that single no-arg
callback for both Row.clickable and RadioButton.onClick so callers capture the
specific language in their lambda; update the LanguageSelectionItem signature
and all call sites to accept a () -> Unit and remove any direct LanguageConfig
parameter passing.

133-157: LanguageSelectionItem duplicates LanguageItem from LanguageScreen.kt.

Both composables render an identical RadioButton + Text row for language selection. Consider extracting to a shared composable (e.g., in core/designsystem or a shared language-ui module) to avoid divergence.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`
around lines 133 - 157, LanguageSelectionItem duplicates the UI implemented in
LanguageItem; extract a shared composable (e.g., LanguageRow or
LanguageSelectionRow) into a common module (core/designsystem or language-ui)
and replace both LanguageSelectionItem and LanguageItem with thin wrappers that
call the new shared composable. Locate the implementations in
OnboardingLanguageScreen.kt (LanguageSelectionItem) and LanguageScreen.kt
(LanguageItem), move the common Row/RadioButton/Text layout into the new shared
function, keep the same parameters (language: LanguageConfig, isSelected:
Boolean, onSelect: (LanguageConfig) -> Unit) and update imports/usages so both
screens use the shared composable. Ensure behavior (clickable Row and
RadioButton onClick) and styling (KptTheme spacing/typography/colorScheme) are
preserved.
feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt (1)

56-62: viewModelScope.launch is unnecessary for a synchronous StateFlow.update.

mutableStateFlow.update {} is atomic and thread-safe — no coroutine scope is needed. This is also inconsistent with handleLoadLanguage (lines 71–78) which calls update directly without launching.

♻️ Proposed refactor
 private fun handleLanguageSelection(language: LanguageConfig) {
-    viewModelScope.launch {
-        mutableStateFlow.update {
-            it.copy(selectedLanguage = language)
-        }
-    }
+    mutableStateFlow.update {
+        it.copy(selectedLanguage = language)
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`
around lines 56 - 62, handleLanguageSelection currently wraps a synchronous,
atomic mutableStateFlow.update call in viewModelScope.launch; remove the
coroutine launch and call mutableStateFlow.update directly inside
handleLanguageSelection so it matches handleLoadLanguage and avoids unnecessary
coroutine overhead. Locate the handleLanguageSelection function and replace the
viewModelScope.launch { mutableStateFlow.update { ... } } block with a direct
mutableStateFlow.update { it.copy(selectedLanguage = language) } call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt`:
- Around line 62-74: The code currently calls
AppCompatDelegate.setApplicationLocales(...) on every MainUiState.Success
emission; change the onEach block to only call setApplicationLocales when the
language actually changes by comparing the new languageTag
(state.language.localName) with the previously applied language (store
lastLanguageTag or compare against uiState?.language.localName before assigning
uiState). Perform the comparison inside the onEach lambda and only
compute/create the LocaleListCompat and call
AppCompatDelegate.setApplicationLocales(...) when the tags differ (use
null/blank-safe equality), leaving other fields (e.g., showOnboarding) to update
without retriggering locale application.

In
`@core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt`:
- Around line 42-43: clearInfo() in UserPreferencesDataSource currently calls
settings.clear(), which removes install-level keys LANGUAGE_KEY and
SHOW_ONBOARDING_KEY that should persist across logouts; update clearInfo() to
preserve those keys by either (1) reading and re-writing LANGUAGE_KEY and
SHOW_ONBOARDING_KEY around the clear() call (save values, clear, restore), or
(2) move those constants and their read/write logic to a separate Settings
instance that is never cleared on logout and update all accessors (the
properties backing _language and _showOnboarding) to use that persistent
Settings instead of the session-cleared settings.

In `@core/model/src/commonMain/kotlin/org/mifospay/core/model/LanguageConfig.kt`:
- Around line 14-78: LanguageConfig's INDONESIAN enum uses the deprecated locale
code "in" which is passed to LocaleListCompat.forLanguageTags() (see
MainActivity.kt) and conflicts with SampleLocale.kt's "id_ID"; update the
INDONESIAN entry in the LanguageConfig enum to use localName = "id" (in the
LanguageConfig enum declaration) so BCP 47 tags are correct and Android 15+
locale normalization works consistently.

In
`@feature/onboarding-language/src/commonMain/composeResources/values-ar/strings.xml`:
- Line 13: The Arabic translation for the resource key
feature_onboarding_chosen_language_can_be_changed_later_in_the_settings is
inconsistent across modules; pick the canonical phrasing (e.g., "يمكن تغيير
اللغة المختارة لاحقاً في الإعدادات") and update this occurrence to match the
other module so both modules use the exact same Arabic string for
feature_onboarding_chosen_language_can_be_changed_later_in_the_settings.

In
`@feature/onboarding-language/src/commonMain/composeResources/values/strings.xml`:
- Line 3: Update the inconsistent copyright year so both language resource files
match: locate the strings.xml for the onboarding-language module in
composeResources/values and the companion values-ar/strings.xml and make their
copyright year identical (choose the correct year used across the module, e.g.,
change 2024 to 2026 or vice versa), then commit the change so both resource
files use the same year.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/navigation/OnboardingLanguageNavigation.kt`:
- Around line 27-31: The onboarding language route is using composable(...)
which causes inconsistent animation; replace that call with
composableWithSlideTransitions(...) so the OnboardingLanguageScreenRoute uses
the same slide transitions as the settings language screen. Locate the block
that registers ONBOARDING_LANGUAGE_ROUTE (the composable(route =
ONBOARDING_LANGUAGE_ROUTE) that constructs OnboardingLanguageScreenRoute) and
swap it to use composableWithSlideTransitions with the same arguments (including
onNavigateToNext) so the screen animates consistently with LanguageNavigation's
implementation.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`:
- Line 78: The composable's local var selectedLanguage (defined via
rememberSaveable { mutableStateOf(uiState.currentLanguage) }) only captures
uiState.currentLanguage at first composition and won't update when the ViewModel
loads the real preference; update the design by moving selection into the
ViewModel like in LanguageScreen: add a selectedLanguage field to
OnboardingLanguageState, set it during the LoadLanguage handler and on
LanguageSelected actions, then remove the local selectedLanguage from
OnboardingLanguageScreen so the UI reads selection directly from
uiState.selectedLanguage (or alternatively, if you must keep a local draft, sync
it from uiState.currentLanguage using a LaunchedEffect keyed to
uiState.currentLanguage).

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageViewModel.kt`:
- Around line 41-47: In handleSetLanguage in OnboardingLanguageViewModel,
reverse the operations so
userPreferencesRepository.setLanguage(action.languageConfig) is called before
userPreferencesRepository.setShowOnboarding(false), and wrap the calls in a
try/catch: only call setShowOnboarding(false) and
sendEvent(OnboardingLanguageEvent.NavigateToNext) if setLanguage succeeds; on
failure catch the exception, log/report it and surface an error event instead of
swallowing it to avoid leaving onboarding marked complete or triggering a
navigation race. Ensure you reference handleSetLanguage, setLanguage,
setShowOnboarding, sendEvent, and OnboardingLanguageEvent.NavigateToNext when
making the change.

In `@feature/settings/src/commonMain/composeResources/values-ar/strings.xml`:
- Around line 23-26: Add the missing Arabic translation entry for the key
feature_settings_profile by inserting a <string> element named
"feature_settings_profile" with the Arabic translation (e.g., "الملف الشخصي")
into the Arabic strings file near the other settings keys (for example right
after the existing feature_settings_log_out_title entry) so the Profile label is
localized properly.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageScreen.kt`:
- Line 98: Replace the hardcoded Spacer height (Spacer(modifier =
Modifier.height(32.dp))) with the theming token KptTheme.spacing.* used
elsewhere (e.g., KptTheme.spacing.large or the same token used by other spacers
in this file/OnboardingLanguageScreen) so the component follows the app spacing
tokens; update the Spacer call to use
Modifier.height(KptTheme.spacing.<appropriateToken>) and remove the
androidx.compose.ui.unit.dp import if it becomes unused.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`:
- Around line 1-20: Remove the duplicated copyright header: keep the first
(2026) header and delete the second (2024) header so only one copyright block
remains at the top of LanguageViewModel.kt; locate the duplicate immediately
above the package declaration (package org.mifospay.feature.language) and delete
the lower/commented block with the 2024 year.
- Around line 71-78: handleLoadLanguage currently overwrites selectedLanguage
with the repository emission, discarding any in-progress user choice; change
handleLoadLanguage (handling LanguageAction.Internal.LoadLanguage) to only
update currentLanguage and leave selectedLanguage untouched so selection is only
changed by user actions (LanguageSelected) or confirmation (SetLanguage); seed
selectedLanguage from LanguageConfig.DEFAULT in constructor if needed and ensure
init subscription no longer resets user selection.

---

Outside diff comments:
In `@cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt`:
- Around line 81-90: The computed reactive startDestination (navDestination
based on uiState) is ignored by NavHost after initial composition; change
startDestination to a stable default (e.g., LOGIN_GRAPH) and add a
LaunchedEffect keyed on uiState that imperatively navigates to
ONBOARDING_LANGUAGE_ROUTE or PASSCODE_GRAPH when uiState transitions to Success;
locate the navDestination computation and replace it with a stable start value,
implement a LaunchedEffect that checks uiState is Success and then calls
navController.navigate(...) to the appropriate route (ONBOARDING_LANGUAGE_ROUTE
or PASSCODE_GRAPH), and remove the unnecessary double casts like (uiState as
Success) by using a single smart-cast or local val success = uiState as Success
in the LaunchedEffect.

---

Nitpick comments:
In `@cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt`:
- Around line 83-89: Extract the casted uiState into a local val to avoid
repeating the cast: inside the branch handling the Success sealed case, create a
local val like success = uiState as Success and then use success.showOnboarding
and success.userData.authenticated to decide between ONBOARDING_LANGUAGE_ROUTE,
PASSCODE_GRAPH, and LOGIN_GRAPH; update the conditionals in the Success branch
to reference that local val instead of casting uiState twice.

In
`@core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt`:
- Around line 137-139: language and showOnboarding are currently exposed as
MutableStateFlow which allows external mutation and bypasses persistence; change
their declarations to expose them as read-only StateFlow by typing them as
StateFlow (e.g., val language: StateFlow<LanguageConfig> = _language and val
showOnboarding: StateFlow<Boolean> = _showOnboarding) so consumers must call
setLanguage / setShowOnboarding to update values and trigger persistence; update
any references expecting MutableStateFlow to use StateFlow where appropriate.

In `@feature/onboarding-language/build.gradle.kts`:
- Line 25: compose.components.uiToolingPreview is currently declared in the
production commonMain via implementation(compose.components.uiToolingPreview);
move this dependency out of commonMain and scope it to a non-production source
set instead (for example replace the implementation call with
debugImplementation for Android targets or add it to a dedicated preview source
set used only in development). Update the build script so
compose.components.uiToolingPreview is only referenced from the debug/preview
configuration and remove it from the commonMain dependencies to avoid shipping
tooling artifacts in production.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`:
- Around line 139-157: LanguageSelectionItem currently exposes onSelect:
(LanguageConfig) -> Unit and both Row.clickable and RadioButton.onClick call it,
causing a double-fire; change its callback to a no-arg () -> Unit (matching
LanguageScreen.LanguageItem's onLanguageSelected) and use that single no-arg
callback for both Row.clickable and RadioButton.onClick so callers capture the
specific language in their lambda; update the LanguageSelectionItem signature
and all call sites to accept a () -> Unit and remove any direct LanguageConfig
parameter passing.
- Around line 133-157: LanguageSelectionItem duplicates the UI implemented in
LanguageItem; extract a shared composable (e.g., LanguageRow or
LanguageSelectionRow) into a common module (core/designsystem or language-ui)
and replace both LanguageSelectionItem and LanguageItem with thin wrappers that
call the new shared composable. Locate the implementations in
OnboardingLanguageScreen.kt (LanguageSelectionItem) and LanguageScreen.kt
(LanguageItem), move the common Row/RadioButton/Text layout into the new shared
function, keep the same parameters (language: LanguageConfig, isSelected:
Boolean, onSelect: (LanguageConfig) -> Unit) and update imports/usages so both
screens use the shared composable. Ensure behavior (clickable Row and
RadioButton onClick) and styling (KptTheme spacing/typography/colorScheme) are
preserved.

In `@feature/settings/src/commonMain/composeResources/values/strings.xml`:
- Around line 30-32: The three duplicate Compose resource keys
feature_onboarding_choose_your_app_language,
feature_onboarding_chosen_language_can_be_changed_later_in_the_settings, and
feature_onboarding_submit are defined in both feature/settings and
feature/onboarding-language; remove the duplication by either (A) moving these
string entries into a shared/core resources module (e.g., common resources
module) and update consumers to use that shared Res, or (B) add a module
dependency from feature/settings to feature/onboarding-language and delete the
local entries in feature/settings so code in settings references
feature/onboarding-language's Res (the generated Res class and the three
resource keys) instead; ensure only one definition remains and update module
dependencies/imports accordingly.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`:
- Around line 56-62: handleLanguageSelection currently wraps a synchronous,
atomic mutableStateFlow.update call in viewModelScope.launch; remove the
coroutine launch and call mutableStateFlow.update directly inside
handleLanguageSelection so it matches handleLoadLanguage and avoids unnecessary
coroutine overhead. Locate the handleLanguageSelection function and replace the
viewModelScope.launch { mutableStateFlow.update { ... } } block with a direct
mutableStateFlow.update { it.copy(selectedLanguage = language) } call.

In `@settings.gradle.kts`:
- Line 89: The new module include include(":feature:onboarding-language") is
placed after the :libs: block, breaking the feature modules grouping; move the
include(":feature:onboarding-language") entry up into the existing :feature:
group where other feature includes are declared (the block that contains
includes like include(":feature:...") between the start of feature declarations
and before the :libs: block) so all feature modules remain grouped together.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db6b7b7 and e4a9cf4.

📒 Files selected for processing (34)
  • cmp-android/dependencies/prodReleaseRuntimeClasspath.tree.txt
  • cmp-android/dependencies/prodReleaseRuntimeClasspath.txt
  • cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt
  • cmp-shared/build.gradle.kts
  • cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayApp.kt
  • cmp-shared/src/commonMain/kotlin/org/mifospay/shared/MifosPayViewModel.kt
  • cmp-shared/src/commonMain/kotlin/org/mifospay/shared/di/KoinModules.kt
  • cmp-shared/src/commonMain/kotlin/org/mifospay/shared/navigation/MifosNavHost.kt
  • cmp-shared/src/commonMain/kotlin/org/mifospay/shared/navigation/RootNavGraph.kt
  • core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt
  • core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesRepository.kt
  • core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesRepositoryImpl.kt
  • core/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/icon/MifosIcons.kt
  • core/model/src/commonMain/kotlin/org/mifospay/core/model/LanguageConfig.kt
  • feature/onboarding-language/.gitignore
  • feature/onboarding-language/build.gradle.kts
  • feature/onboarding-language/src/androidMain/AndroidManifest.xml
  • feature/onboarding-language/src/commonMain/composeResources/values-ar/strings.xml
  • feature/onboarding-language/src/commonMain/composeResources/values/strings.xml
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageViewModel.kt
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/di/OnboardingLanguageModule.kt
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/navigation/OnboardingLanguageNavigation.kt
  • feature/settings/build.gradle.kts
  • feature/settings/src/commonMain/composeResources/values-ar/strings.xml
  • feature/settings/src/commonMain/composeResources/values/strings.xml
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageScreen.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/navigation/LanguageNavigation.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/SettingsScreen.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/SettingsViewModel.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/di/SettingsModule.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/settings/navigation/SettingsNavigation.kt
  • settings.gradle.kts

mena-rizkalla and others added 3 commits February 25, 2026 09:01
- Improve `OnboardingLanguageScreen` by adding a snackbar for error handling and migrating selection state to the ViewModel.
- Update `OnboardingLanguageViewModel` to handle language selection, error states, and atomic preference updates.
- Refactor `UserPreferencesDataSource.clearInfo()` to preserve language, onboarding status, and instance settings while clearing user-specific data.
- Optimize `MainActivity` to only update application locales when the requested language differs from the current configuration.
- Update `LanguageConfig` to use the correct ISO code (`id`) for Indonesian.
- Integrate `composableWithSlideTransitions` for onboarding language navigation.
- Add localized error strings and update copyright headers to 2026.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt (1)

31-31: Unused import.

The setValue import is no longer needed since local mutable state was removed from this screen.

🧹 Remove unused import
-import androidx.compose.runtime.setValue
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`
at line 31, Remove the now-unused import statement
"androidx.compose.runtime.setValue" from OnboardingLanguageScreen (the import
for setValue is no longer needed because local mutable state was removed);
locate the import line that references setValue and delete it so the file no
longer contains that unused import.
cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt (1)

74-76: Consider extracting the primary locale for comparison (optional).

The comparison languageTag != currentAppLocales.toLanguageTags() works correctly when the app has set a single locale. However, if currentAppLocales ever contains multiple locales (e.g., from system configuration), toLanguageTags() returns a comma-separated list that won't match a single tag, causing unnecessary updates.

For added robustness, consider comparing against only the first locale:

♻️ Optional improvement
 val shouldUpdate = if (isRequestedDefault) {
     !isCurrentDefault
 } else {
-    languageTag != currentAppLocales.toLanguageTags()
+    currentAppLocales.isEmpty || languageTag != currentAppLocales[0]?.toLanguageTag()
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt` around lines 74 -
76, The current comparison uses languageTag !=
currentAppLocales.toLanguageTags(), which can mismatch when currentAppLocales
contains multiple locales; update the check in MainActivity (where languageTag
and currentAppLocales.toLanguageTags() are used) to compare only the
primary/first locale from currentAppLocales (e.g., currentAppLocales[0] or
equivalent API to get the first Locale) against languageTag so a single-tag
languageTag won't be falsely considered different when the system provides
multiple locales.
feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt (1)

46-52: Unnecessary coroutine wrapper for synchronous state update.

MutableStateFlow.update is synchronous and thread-safe. The viewModelScope.launch wrapper adds overhead without benefit here.

♻️ Proposed simplification
 private fun handleLanguageSelection(language: LanguageConfig) {
-    viewModelScope.launch {
-        mutableStateFlow.update {
-            it.copy(selectedLanguage = language)
-        }
+    mutableStateFlow.update {
+        it.copy(selectedLanguage = language)
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`
around lines 46 - 52, The handleLanguageSelection function wraps a synchronous,
thread-safe mutableStateFlow.update call in viewModelScope.launch unnecessarily;
remove the coroutine wrapper and call mutableStateFlow.update {
it.copy(selectedLanguage = language) } directly inside handleLanguageSelection
(keep the function signature and the LanguageConfig parameter) so the state
update happens synchronously without launching a coroutine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`:
- Around line 54-59: The handleSetLanguage in LanguageViewModel currently calls
userPreferencesRepository.setLanguage(...) without error handling and
immediately sends LanguageEvent.NavigateBack; wrap the persistence call in a
try/catch (as done in OnboardingLanguageViewModel.handleSetLanguage) so failures
are caught, and on success send LanguageEvent.NavigateBack while on failure send
an error event (e.g., LanguageEvent.ShowError or
LanguageEvent.ShowPersistenceError) and optionally log the exception; update
LanguageViewModel.handleSetLanguage to use viewModelScope.launch { try {
userPreferencesRepository.setLanguage(action.languageConfig);
sendEvent(LanguageEvent.NavigateBack) } catch (e: Exception) {
sendEvent(LanguageEvent.ShowError(...)); /* log e */ } }.

---

Nitpick comments:
In `@cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt`:
- Around line 74-76: The current comparison uses languageTag !=
currentAppLocales.toLanguageTags(), which can mismatch when currentAppLocales
contains multiple locales; update the check in MainActivity (where languageTag
and currentAppLocales.toLanguageTags() are used) to compare only the
primary/first locale from currentAppLocales (e.g., currentAppLocales[0] or
equivalent API to get the first Locale) against languageTag so a single-tag
languageTag won't be falsely considered different when the system provides
multiple locales.

In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt`:
- Line 31: Remove the now-unused import statement
"androidx.compose.runtime.setValue" from OnboardingLanguageScreen (the import
for setValue is no longer needed because local mutable state was removed);
locate the import line that references setValue and delete it so the file no
longer contains that unused import.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`:
- Around line 46-52: The handleLanguageSelection function wraps a synchronous,
thread-safe mutableStateFlow.update call in viewModelScope.launch unnecessarily;
remove the coroutine wrapper and call mutableStateFlow.update {
it.copy(selectedLanguage = language) } directly inside handleLanguageSelection
(keep the function signature and the LanguageConfig parameter) so the state
update happens synchronously without launching a coroutine.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 252625c and be8ec22.

📒 Files selected for processing (12)
  • cmp-android/src/main/kotlin/org/mifospay/MainActivity.kt
  • core/datastore/src/commonMain/kotlin/org/mifospay/core/datastore/UserPreferencesDataSource.kt
  • core/model/src/commonMain/kotlin/org/mifospay/core/model/LanguageConfig.kt
  • feature/onboarding-language/src/commonMain/composeResources/values-ar/strings.xml
  • feature/onboarding-language/src/commonMain/composeResources/values/strings.xml
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageScreen.kt
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageViewModel.kt
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/navigation/OnboardingLanguageNavigation.kt
  • feature/settings/src/commonMain/composeResources/values-ar/strings.xml
  • feature/settings/src/commonMain/composeResources/values/strings.xml
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageScreen.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • feature/onboarding-language/src/commonMain/composeResources/values/strings.xml
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageScreen.kt

- Remove unused `setValue` import in `OnboardingLanguageScreen.kt`.
- Remove redundant comment regarding locale comparison in `MainActivity.kt`.
In `handleSetLanguage`, trigger the back navigation event before updating the language preference to ensure a smoother transition.
- Update `OnboardingLanguageViewModel` and `LanguageViewModel` to use `NonCancellable` context when saving language preferences, ensuring data persistence even if the scope is cancelled during navigation.
- Trigger navigation events before starting the preference save operation to improve UI responsiveness.
- Refine language state logic to prevent overwriting user selection with default values during initial load.
- Remove unnecessary coroutine launch for simple state updates in `LanguageViewModel`.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageViewModel.kt`:
- Around line 58-69: In handleSetLanguage, move the call to
sendEvent(OnboardingLanguageEvent.NavigateToNext) so it runs after the
persistence succeeds: perform the withContext(NonCancellable) block that calls
userPreferencesRepository.setLanguage(...) and
userPreferencesRepository.setShowOnboarding(false) first, let any exception
bubble to the catch which updates mutableStateFlow with the error, and only call
sendEvent(...) after that block completes successfully; ensure sendEvent is not
executed before persistence to avoid navigating on failed saves.

In `@feature/settings/src/commonMain/composeResources/values/strings.xml`:
- Around line 30-31: Remove the duplicate, unused string resources from the
settings module: delete the <string> entries named
feature_onboarding_choose_your_app_language and feature_onboarding_submit in the
settings module's strings.xml since those keys are defined and used in the
feature/onboarding-language module only; ensure no references remain in Settings
code (search for feature_onboarding_choose_your_app_language and
feature_onboarding_submit) before committing.

In
`@feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt`:
- Around line 60-70: The handler handleSetLanguage currently emits
LanguageEvent.NavigateBack before persisting via
userPreferencesRepository.setLanguage, so failures are not surfaced; move the
persistence call to occur before sendEvent and keep the save inside the
NonCancellable with viewModelScope.launch, then only call
sendEvent(LanguageEvent.NavigateBack) after
userPreferencesRepository.setLanguage succeeds, and in the catch block update
mutableStateFlow with Res.string.feature_settings_error_saving_settings as
before.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb369ac and 754bdde.

📒 Files selected for processing (5)
  • feature/onboarding-language/src/commonMain/kotlin/org/mifospay/feature/onboarding/language/OnboardingLanguageViewModel.kt
  • feature/settings/src/commonMain/composeResources/values-ar/strings.xml
  • feature/settings/src/commonMain/composeResources/values/strings.xml
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageScreen.kt
  • feature/settings/src/commonMain/kotlin/org/mifospay/feature/language/LanguageViewModel.kt

- Remove `feature_onboarding_choose_your_app_language` and `feature_onboarding_submit` from English and Arabic string resources in the settings feature.
try {
sendEvent(OnboardingLanguageEvent.NavigateToNext)

withContext(NonCancellable) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove with context no need here..ViewModelScope handle this functionality properly and remove the tey-catch instead move into repository and return DataState<*> and check accordingly

@niyajali
Copy link
Collaborator

@mena-rizkalla This PR should only address one ticket and one fix — can you explain why changes are being made across multiple feature modules? Remove the changes from other features module. And send separate PR for that

@mena-rizkalla
Copy link
Author

@mena-rizkalla This PR should only address one ticket and one fix — can you explain why changes are being made across multiple feature modules? Remove the changes from other features module. And send separate PR for that

Thanks for the review! Regarding why changes made across multiple feature because the logic for both the Onboarding and Settings language features was very similar, I went ahead and implemented both on same pr. Should i remove the changes related to the Settings module and update this PR to focus strictly on the Onboarding ticket ?

@niyajali
Copy link
Collaborator

Yes, remove the changes on other modules on this PR and send a separate PR for that and in future consider this before sending a PR.

… handling

- Update `UserPreferencesRepository` and its implementation to return `DataState<Unit>` for `setLanguage` and `setShowOnboarding` operations.
- Refactor `OnboardingLanguageViewModel` to handle results from preference updates, ensuring navigation only occurs on success and errors are correctly captured in the state.
- Refactor `LanguageViewModel` to use the new `DataState` return type, moving navigation logic to follow a successful preference update.
- Remove redundant `NonCancellable` context and manual try-catch blocks in ViewModels in favor of the repository's state-based response.
…ection

- Update `MainActivity` in `AndroidManifest.xml` to include `locale` and `layoutDirection` in `android:configChanges` to prevent activity recreation when language or layout direction changes.
- Delete `LanguageScreen`, `LanguageViewModel`, and associated navigation logic in the settings feature.
- Remove language-related strings (`feature_settings_change_language`, `feature_settings_language`, `feature_settings_error_saving_settings`) from English and Arabic resources.
- Update `SettingsViewModel` and `SettingsScreen` to remove actions, events, and UI components related to changing the language.
- Remove `LanguageViewModel` from `SettingsModule` dependency injection.
- Cleanup unused `core.datastore` and `coreBase.designsystem` project dependencies from `feature:settings` build configuration.
- Update `MifosNavHost` and `SettingsNavigation` to remove language destination and navigation routing.
@mena-rizkalla mena-rizkalla changed the title feat(language): add language selection to onboarding and settings feat(language): add language selection to onboarding Feb 26, 2026
…eRuntimeClasspath

- Remove `project :core:datastore` and `project :core-base:designsystem` from the `prodReleaseRuntimeClasspath` dependency tree.
- Update copyright headers to 2026 in `OnboardingLanguageModule.kt` and `build.gradle.kts`.
- Remove unused project dependencies (`core.model`, `core.designsystem`, and `core.ui`) from the `onboarding-language` module.
- Refactor `UserPreferencesDataSource.clearInfo()` to stop preserving instance and interbank instance settings when clearing user data.
…path

- Remove `:core:model` dependency from the `prodReleaseRuntimeClasspath` in `cmp-android`.
@niyajali
Copy link
Collaborator

niyajali commented Mar 3, 2026

@mena-rizkalla Reviewing the template project will demonstrate our method for configuring the application's language across all operating systems, not just Android. It's crucial to verify that the languages are configured accurately and maintained consistently across all platforms. Upload videos for all platforms by selecting a language other than English. Then, open the app, close it, and reopen it to verify that the language has been set correctly.

@mena-rizkalla mena-rizkalla requested a review from a team March 4, 2026 15:19
- Add `MifosRadioButton` and `MifosRadioButtonGroup` components to the design system, featuring animated state transitions, accessibility support, and customizable styles.
- Integrate `MifosRadioButton` into `OnboardingLanguageScreen`, replacing the standard `RadioButton` and manual `Row` layout for a more polished selection experience.
- Expand the base design system theme to include `KptStrokes` for consistent border width management across components.
- Update `KptTheme` and associated builders/providers to support the new strokes attribute.
@mena-rizkalla
Copy link
Author

mena-rizkalla commented Mar 5, 2026

@mena-rizkalla Reviewing the template project will demonstrate our method for configuring the application's language across all operating systems, not just Android. It's crucial to verify that the languages are configured accurately and maintained consistently across all platforms. Upload videos for all platforms by selecting a language other than English. Then, open the app, close it, and reopen it to verify that the language has been set correctly.

np, just tell me if everything looks good for now before uploading the other os as this will take time for build, so if there are any other changes required tell me to do it first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants