feat: Add On boarding Empty Screens #99
Conversation
📝 WalkthroughWalkthroughA new onboarding feature module is introduced with DI integration, navigation routing, multi-page UI screens, state management via ViewModel, and persistence layer. Default user data is updated to mark first-time users. Build configuration and module inclusions are updated accordingly. Changes
Sequence DiagramsequenceDiagram
participant User
participant NavController
participant RootNav as Root Navigation
participant Screen as OnboardingScreen
participant ViewModel as OnboardingViewmodel
participant Repository as UserPreferencesRepository
User->>NavController: navigateToOnBoarding()
NavController->>RootNav: Navigate to OnBoardingRoute
RootNav->>Screen: Render Composable
Screen->>ViewModel: Observe currentPage (via koinViewModel)
ViewModel->>Screen: Emit currentPage = 1
Screen->>Screen: Render Page 1 (of 2)
User->>Screen: Click Next
Screen->>ViewModel: onNextPage()
ViewModel->>ViewModel: currentPage = 2
ViewModel->>Screen: Emit currentPage = 2
Screen->>Screen: Render Page 2 (of 2)
User->>Screen: Click Next (final page)
Screen->>ViewModel: onNextPage()
ViewModel->>ViewModel: currentPage exceeds TOTAL_PAGES
ViewModel->>ViewModel: updateOnboardingStatus()
ViewModel->>Repository: setFirstTimeState(false)
Repository->>Repository: Persist preference
Repository-->>ViewModel: Complete
ViewModel->>Screen: Emit currentPage = 3
Screen->>NavController: Navigation ends (onboarding complete)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmp-navigation/src/commonMain/kotlin/cmp/navigation/rootnav/RootNavScreen.kt (1)
75-84: Implement navigation handlers for Auth and UserLocked states.
RootNavState.Auth(line 54 in RootNavViewModel) andRootNavState.UserLocked(lines 56, 62 in RootNavViewModel) are reachable states in the navigation flow, but in RootNavScreen they map to empty strings (lines 79, 82) and have empty navigation handlers (lines 121, 125). These states won't navigate anywhere, leaving users stuck.Define and implement routes for authentication and user unlock flows to complete the navigation logic.
🧹 Nitpick comments (4)
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingModule.kt (1)
15-17: Consider aligning naming convention: filename vs. variable name.The filename uses
OnBoardingModule.kt(capital B) while the module variable is namedOnboardingModule(lowercase b in "boarding"). For consistency and easier code navigation, consider aligning them:
- Option 1: Rename file to
OnboardingModule.kt- Option 2: Rename variable to
OnBoardingModuleThe codebase appears to use "onboarding" as a single word elsewhere (e.g.,
:feature:onboardingin settings), suggesting Option 1 may be more consistent.feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingScreen.kt (1)
35-47: Consider extracting hard-coded strings to string resources for internationalization.The titles and descriptions are hard-coded in English. For better i18n support, consider moving these to string resources.
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/components/OnBoardingScreenPage.kt (2)
118-128: Consider extracting "Next" button text to string resources.The button text is hard-coded in English. For i18n support, move this to string resources.
68-75: Remove redundant color specification.The
colorparameter on line 74 overrides the color set in the style's.copy()on line 72, making the.copy()operation unnecessary for color purposes.🔎 Suggested simplification
Text( modifier = Modifier.fillMaxWidth(), text = description, - style = MaterialTheme.typography.bodyLarge - .copy(color = MaterialTheme.colorScheme.onSurfaceVariant), + style = MaterialTheme.typography.bodyLarge, textAlign = TextAlign.Center, color = MaterialTheme.colorScheme.onSurfaceVariant, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
cmp-navigation/build.gradle.ktscmp-navigation/src/commonMain/kotlin/cmp/navigation/di/KoinModules.ktcmp-navigation/src/commonMain/kotlin/cmp/navigation/rootnav/RootNavScreen.ktcore/model/src/commonMain/kotlin/org/mifos/core/model/UserData.ktfeature/onboarding/.gitignorefeature/onboarding/build.gradle.ktsfeature/onboarding/consumer-rules.profeature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingModule.ktfeature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingRoute.ktfeature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingScreen.ktfeature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnboardingViewmodel.ktfeature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/components/OnBoardingScreenPage.ktgradle/libs.versions.tomlsettings.gradle.kts
🧰 Additional context used
🧬 Code graph analysis (2)
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingRoute.kt (2)
core-base/ui/src/commonMain/kotlin/template/core/base/ui/NavGraphBuilderExtensions.kt (1)
composableWithStayTransitions(45-60)feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingScreen.kt (1)
OnboardingScreen(24-51)
cmp-navigation/src/commonMain/kotlin/cmp/navigation/rootnav/RootNavScreen.kt (2)
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingRoute.kt (1)
onboardingDestination(23-27)cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.kt (1)
authenticatedGraph(32-50)
🔇 Additional comments (9)
core/model/src/commonMain/kotlin/org/mifos/core/model/UserData.kt (1)
43-43: LGTM! Logical default for onboarding flow.Setting
firstTimeUser = truein the default UserData instance correctly initializes new users to trigger the onboarding experience.settings.gradle.kts (1)
76-76: LGTM! Module inclusion follows project conventions.The onboarding module is correctly included in the project structure alongside other feature modules.
cmp-navigation/build.gradle.kts (1)
33-33: LGTM! Dependency correctly wired.The onboarding feature module is properly integrated into the navigation layer's dependency graph.
cmp-navigation/src/commonMain/kotlin/cmp/navigation/di/KoinModules.kt (1)
20-20: LGTM! DI wiring correctly implemented.The
OnboardingModuleis properly imported and included in the feature module composition, following the established pattern.Also applies to: 47-47
feature/onboarding/build.gradle.kts (1)
1-39: LGTM! Build configuration follows project conventions.The onboarding module's Gradle configuration correctly:
- Uses the CMP feature convention plugin
- Declares appropriate dependencies for a feature module with Compose UI
- Configures compose resources with proper namespacing
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingRoute.kt (1)
18-27: LGTM! Clean navigation setup.The onboarding route definition and navigation extensions follow established patterns in the codebase and are correctly integrated with the navigation system.
cmp-navigation/src/commonMain/kotlin/cmp/navigation/rootnav/RootNavScreen.kt (1)
33-35: LGTM! Onboarding properly integrated into navigation.The onboarding destination is correctly registered in the NavHost and follows the established navigation patterns.
Also applies to: 69-69
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnboardingViewmodel.kt (1)
24-27: LGTM! Clean state management.The state flow setup is correct and follows best practices for ViewModel state management in Compose.
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/components/OnBoardingScreenPage.kt (1)
37-86: LGTM! Well-structured onboarding page layout.The composable uses proper Compose layout patterns with clear hierarchy and appropriate spacing.
| when (currentPage) { | ||
| 1 -> OnBoardingScreenPage( | ||
| onNext = viewmodel::onNextPage, | ||
| currentPage = currentPage, | ||
| title = "Welcome", | ||
| description = "Thank you for using our template for creating your project", | ||
| modifier = modifier.padding(it), | ||
| ) | ||
| 2 -> OnBoardingScreenPage( | ||
| onNext = viewmodel::onNextPage, | ||
| currentPage = currentPage, | ||
| title = "Get Started", | ||
| description = "You can now start using your project", | ||
| modifier = modifier.padding(it), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Add an else branch to handle unexpected page values.
The when expression only handles pages 1 and 2 without an else branch. If currentPage somehow becomes invalid (e.g., 0 or > TOTAL_PAGES), nothing will render, leaving a blank screen.
🔎 Suggested fix with else branch
Scaffold(
containerColor = MaterialTheme.colorScheme.background,
) {
when (currentPage) {
1 -> OnBoardingScreenPage(
onNext = viewmodel::onNextPage,
currentPage = currentPage,
title = "Welcome",
description = "Thank you for using our template for creating your project",
modifier = modifier.padding(it),
)
2 -> OnBoardingScreenPage(
onNext = viewmodel::onNextPage,
currentPage = currentPage,
title = "Get Started",
description = "You can now start using your project",
modifier = modifier.padding(it),
)
+ else -> {
+ // Fallback for unexpected page values
+ OnBoardingScreenPage(
+ onNext = viewmodel::onNextPage,
+ currentPage = 1,
+ title = "Welcome",
+ description = "Thank you for using our template for creating your project",
+ modifier = modifier.padding(it),
+ )
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| when (currentPage) { | |
| 1 -> OnBoardingScreenPage( | |
| onNext = viewmodel::onNextPage, | |
| currentPage = currentPage, | |
| title = "Welcome", | |
| description = "Thank you for using our template for creating your project", | |
| modifier = modifier.padding(it), | |
| ) | |
| 2 -> OnBoardingScreenPage( | |
| onNext = viewmodel::onNextPage, | |
| currentPage = currentPage, | |
| title = "Get Started", | |
| description = "You can now start using your project", | |
| modifier = modifier.padding(it), | |
| ) | |
| } | |
| when (currentPage) { | |
| 1 -> OnBoardingScreenPage( | |
| onNext = viewmodel::onNextPage, | |
| currentPage = currentPage, | |
| title = "Welcome", | |
| description = "Thank you for using our template for creating your project", | |
| modifier = modifier.padding(it), | |
| ) | |
| 2 -> OnBoardingScreenPage( | |
| onNext = viewmodel::onNextPage, | |
| currentPage = currentPage, | |
| title = "Get Started", | |
| description = "You can now start using your project", | |
| modifier = modifier.padding(it), | |
| ) | |
| else -> { | |
| // Fallback for unexpected page values | |
| OnBoardingScreenPage( | |
| onNext = viewmodel::onNextPage, | |
| currentPage = 1, | |
| title = "Welcome", | |
| description = "Thank you for using our template for creating your project", | |
| modifier = modifier.padding(it), | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnBoardingScreen.kt
around lines 34–49, the when on currentPage only handles 1 and 2 so an
unexpected value leaves the UI blank; add an else branch that handles invalid
pages (e.g., log the unexpected value and render a safe default
OnBoardingScreenPage such as page 1 or a generic fallback message, or reset
currentPage via the viewmodel) so the UI never renders empty and unexpected
values are recoverable.
| fun onNextPage() { | ||
| if (_currentPage.value < totalPages) { | ||
| _currentPage.value += 1 | ||
| } else { | ||
| updateOnboardingStatus() | ||
| } | ||
| } | ||
|
|
||
| private fun updateOnboardingStatus() { | ||
| viewModelScope.launch { | ||
| preferenceRepository.setFirstTimeState(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add error handling and navigation callback after onboarding completion.
The updateOnboardingStatus() function uses a fire-and-forget pattern with no error handling. If setFirstTimeState() fails, the user won't be notified, and the onboarding state won't be persisted. Additionally, there's no mechanism to navigate the user away from the onboarding screen after completion.
Consider:
- Adding error handling for the repository call
- Providing a callback or event to trigger navigation after successful persistence
- Showing a loading state during the save operation
🔎 Suggested approach with completion callback
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.SharedFlow
+import kotlinx.coroutines.flow.asSharedFlow
+
class OnboardingViewmodel(
private val preferenceRepository: UserPreferencesRepository,
) : ViewModel() {
private val _currentPage = MutableStateFlow(1)
val currentPage: StateFlow<Int> = _currentPage.asStateFlow()
+
+ private val _onboardingComplete = MutableSharedFlow<Boolean>()
+ val onboardingComplete: SharedFlow<Boolean> = _onboardingComplete.asSharedFlow()
private val totalPages = TOTAL_PAGES
fun onNextPage() {
if (_currentPage.value < totalPages) {
_currentPage.value += 1
} else {
updateOnboardingStatus()
}
}
private fun updateOnboardingStatus() {
viewModelScope.launch {
- preferenceRepository.setFirstTimeState(false)
+ try {
+ preferenceRepository.setFirstTimeState(false)
+ _onboardingComplete.emit(true)
+ } catch (e: Exception) {
+ // Handle error - maybe emit false or show error state
+ _onboardingComplete.emit(false)
+ }
}
}
}Then in the screen, collect onboardingComplete to trigger navigation.
🤖 Prompt for AI Agents
In
feature/onboarding/src/commonMain/kotlin/org/mifos/feature/onboarding/OnboardingViewmodel.kt
around lines 29-41, update updateOnboardingStatus() to handle errors, loading
state, and emit a completion event: add a loading StateFlow/LiveData (e.g.,
_isSaving -> isSaving), an error StateFlow/LiveData (e.g., _saveError ->
saveError) and a one-time completion event (e.g., _onboardingComplete as
SharedFlow or SingleLiveEvent), then in viewModelScope.launch set isSaving true,
call preferenceRepository.setFirstTimeState(false) inside try/catch, on success
emit onboardingComplete event and clear error, on failure capture and emit the
error message/exception to saveError, and finally set isSaving false; update
onNextPage to trigger updateOnboardingStatus() when finishing and the UI should
collect isSaving/saveError/onboardingComplete to show a loading indicator,
display errors, and navigate on completion.
| espressoCore = "3.7.0" | ||
| material = "1.13.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of the newly added libraries
# Search for espresso-core usage
echo "=== Searching for espresso-core usage ==="
rg -n --type=kotlin -C2 'espresso' || echo "No matches found"
# Search for material library usage in build files
echo -e "\n=== Searching for 'material' in build files ==="
fd -e kts -e gradle --exec rg -n 'material' {} + || echo "No matches found"
# Search for references to libs.espressoCore
echo -e "\n=== Searching for libs.espressoCore ==="
rg -n 'libs\.espressoCore' || echo "No matches found"
# Search for references to libs.material
echo -e "\n=== Searching for libs\.material[^3] ==="
rg -n 'libs\.material(?!3)' || echo "No matches found"
# Check what's in gradle/libs.versions.toml around lines 115-116 and 335-336
echo -e "\n=== Content of gradle/libs.versions.toml around the added lines ==="
sed -n '110,120p' gradle/libs.versions.toml
echo "..."
sed -n '330,340p' gradle/libs.versions.tomlRepository: openMF/kmp-project-template
Length of output: 4988
🏁 Script executed:
#!/bin/bash
# Search more specifically for usage of these libraries
# Search for androidx-espresso-core references in all build files
echo "=== Searching for androidx-espresso-core in dependencies ==="
rg -n 'androidx-espresso-core|libs\.espressoCore'
# Search for Android Material library references (non-Compose)
echo -e "\n=== Searching for com.google.android.material references ==="
rg -n 'com\.google\.android\.material|libs\.material[^3]'
# Check if compose.material (not material3) is used
echo -e "\n=== Checking compose.material usage ==="
rg -n 'compose\.material(?!3|Icons)'
# List all libraries in gradle/libs.versions.toml that reference espressoCore or material
echo -e "\n=== All library definitions using espressoCore or material versions ==="
rg -n 'version\.ref.*=.*"(espressoCore|material)"' gradle/libs.versions.tomlRepository: openMF/kmp-project-template
Length of output: 1069
Remove unused library declarations from catalog.
The newly added espressoCore and material libraries are defined in gradle/libs.versions.toml (lines 115-116 and 335-336) but are not referenced or used anywhere in the codebase. Remove these entries, or add them only when you actually need them to avoid catalog bloat.
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around lines 115-116 and 335-336, the catalog
defines espressoCore = "3.7.0" and material = "1.13.0" but these libraries are
unused; remove these two entries (both occurrences) from the TOML file to avoid
catalog bloat, then run a Gradle/IDE sync (or ./gradlew build) to ensure no
references remain and the version catalog is consistent.
|
@revanthkumarJ When starting a new project, it's standard procedure to eliminate current project feature modules. This is usually because significant refactoring is needed to match them with user needs or the application's design guidelines. The present pull request includes UI-related changes that don't impact this project and would require a substantial amount of work to remove from the code during the creation of a new project. |
https://mifosforge.jira.com/browse/KMPPT-88
WhatsApp.Video.2025-12-25.at.9.10.18.PM.mp4