-
Notifications
You must be signed in to change notification settings - Fork 1
Develop #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Develop #24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a first-launch onboarding flow, updates theme color logic, and overhauls the onboarding screen UI.
- Renamed onboarding image resources and wired them into the
OnBoardingsealed class. - Made theme text colors dynamic based on
isDark, and updated the black color logic. - Rebuilt the onboarding screen layout with a full-screen image, gradient overlay, and typography updates.
- Added navigation and storage logic to show onboarding on first app launch and persist that state.
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/java/org/mahd_e_learning_platform/utils/OnBoarding.kt | Updated image resource names for onboarding screens. |
| app/src/main/java/org/mahd_e_learning_platform/ui/theme/Theme.kt | Made text and black colors conditional on isDark. |
| app/src/main/java/org/mahd_e_learning_platform/presentation/screens/onboarding/OnBoarding.kt | Reworked layout to a full‐screen image with overlay and updated text styling. |
| app/src/main/java/org/mahd_e_learning_platform/presentation/navigation/Screen.kt | Added ONBOARDING destination to routing. |
| app/src/main/java/org/mahd_e_learning_platform/presentation/navigation/AppNavigator.kt | Registered the onboarding composable in the nav graph. |
| app/src/main/java/org/mahd_e_learning_platform/presentation/MainActivity.kt | Injected SecureTokenStore, added first-launch logic, and navigation. |
| app/src/main/java/org/mahd_e_learning_platform/data/source/remote/auth/SecureTokenStore.kt | Exposed accessFirstLaunchState flow for first-launch. |
Comments suppressed due to low confidence (2)
app/src/main/java/org/mahd_e_learning_platform/presentation/MainActivity.kt:63
- The log tag "isLoggedIn" is reused for printing
isFirstLaunch. Use a distinct tag like "isFirstLaunch" to avoid confusion.
Log.d("isLoggedIn", "onCreate: $isFirstLaunch")
app/src/main/java/org/mahd_e_learning_platform/data/source/remote/auth/SecureTokenStore.kt:53
- No unit tests cover
accessFirstLaunchState. Add tests to verify the defaulttruevalue and subsequent updates after saving.
val accessFirstLaunchState = context.dataStore.data.map { prefs ->
| import androidx.compose.ui.graphics.BlendMode | ||
| import androidx.compose.ui.graphics.Brush | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.graphics.LinearGradient | ||
| import androidx.compose.ui.graphics.TileMode |
Copilot
AI
Jun 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LinearGradient, BlendMode, and TileMode are imported but not used. Consider removing these imports to clean up the code.
| import androidx.compose.ui.graphics.BlendMode | |
| import androidx.compose.ui.graphics.Brush | |
| import androidx.compose.ui.graphics.Color | |
| import androidx.compose.ui.graphics.LinearGradient | |
| import androidx.compose.ui.graphics.TileMode | |
| import androidx.compose.ui.graphics.Brush | |
| import androidx.compose.ui.graphics.Color |
|
|
||
| override fun onStart() { | ||
| super.onStart() | ||
| CoroutineScope(Dispatchers.Main).launch { |
Copilot
AI
Jun 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new CoroutineScope inside an Activity can lead to lifecycle leaks. Consider using lifecycleScope.launch instead.
| CoroutineScope(Dispatchers.Main).launch { | |
| lifecycleScope.launch { |
| CoroutineScope(Dispatchers.Main).launch { | ||
| secureTokenStore.saveFirstLaunch(false) | ||
| } | ||
|
|
Copilot
AI
Jun 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Persisting firstLaunch = false in onStart may mark the app as launched before onboarding completes. Consider saving this flag after the user finishes onboarding.
| CoroutineScope(Dispatchers.Main).launch { | |
| secureTokenStore.saveFirstLaunch(false) | |
| } | |
| // Removed logic to save firstLaunch flag here. |
No description provided.