-
Notifications
You must be signed in to change notification settings - Fork 521
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
Fix Part of #4938: Introduce New App Language Selection Screen for onboarding #5373
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.
Thanks @adhiamboperes! I have left some comments. PTAL.
app/src/main/java/org/oppia/android/app/customview/OppiaCurveBackgroundView.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/customview/OppiaCurveBackgroundView.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/onboardingv2/OnboardingFragmentPresenter.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/onboarding/OnboardingFragmentTest.kt
Outdated
Show resolved
Hide resolved
Unassigning @theMr17 since the review is done. |
Hi @adhiamboperes, it looks like some changes were requested on this pull request by @theMr17. PTAL. Thanks! |
@theMr17, I have addressed your comments, PTAL. |
Unassigning @adhiamboperes since a re-review was requested. @adhiamboperes, please make sure you have addressed all review comments. Thanks! |
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.
Thanks @adhiamboperes! This LGTM.
Unassigning @theMr17 since they have already approved the PR. |
…creen # Conflicts: # app/src/sharedTest/java/org/oppia/android/app/databinding/BUILD.bazel
@BenHenning, I have addressed all your comments. PTAL. |
Hi @adhiamboperes, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
…creen # Conflicts: # app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 16 MiB (old), 17 MiB (new), 1151 KiB (Added) APK download size (estimated): 14 MiB (old), 16 MiB (new), 1134 KiB (Added) Method count: 214503 (old), 235368 (new), 20865 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6492 (old), 6550 (new), 58 (Added)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 15 MiB (old), 17 MiB (new), 1150 KiB (Added)
Configuration hdpiAPK file size: 59 KiB (old), 59 KiB (new), 280 bytes (Added) Configuration ldpiAPK file size: 56 KiB (old), 56 KiB (new), 352 bytes (Added) Configuration mdpiAPK file size: 53 KiB (old), 53 KiB (new), 288 bytes (Added) Configuration tvdpiAPK file size: 102 KiB (old), 103 KiB (new), 488 bytes (Added) Configuration xhdpiAPK file size: 67 KiB (old), 67 KiB (new), 344 bytes (Added) Configuration xxhdpiAPK file size: 76 KiB (old), 76 KiB (new), 288 bytes (Added) Configuration xxxhdpiAPK file size: 78 KiB (old), 79 KiB (new), 296 bytes (Added) AlphaExpand to see flavor specificsUniversal APKAPK file size: 9 MiB (old), 9 MiB (new), 33 KiB (Added) APK download size (estimated): 9016 KiB (old), 9044 KiB (new), 28 KiB (Added) Method count: 93710 (old), 93608 (new), 102 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5432 (old), 5504 (new), 72 (Added)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 9 MiB (old), 9 MiB (new), 32 KiB (Added)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 204 bytes (Added) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 336 bytes (Added) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 208 bytes (Added) Configuration tvdpiAPK file size: 90 KiB (old), 90 KiB (new), 340 bytes (Added) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 268 bytes (Added) Configuration xxhdpiAPK file size: 68 KiB (old), 69 KiB (new), 208 bytes (Added) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 212 bytes (Added) BetaExpand to see flavor specificsUniversal APKAPK file size: 9 MiB (old), 9 MiB (new), 32 KiB (Added) APK download size (estimated): 9003 KiB (old), 9032 KiB (new), 29 KiB (Added) Method count: 93710 (old), 93608 (new), 102 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5432 (old), 5504 (new), 72 (Added)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 9 MiB (old), 9 MiB (new), 32 KiB (Added)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 204 bytes (Added) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 336 bytes (Added) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 208 bytes (Added) Configuration tvdpiAPK file size: 90 KiB (old), 90 KiB (new), 340 bytes (Added) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 268 bytes (Added) Configuration xxhdpiAPK file size: 68 KiB (old), 69 KiB (new), 208 bytes (Added) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 212 bytes (Added) GaExpand to see flavor specificsUniversal APKAPK file size: 9 MiB (old), 9 MiB (new), 32 KiB (Added) APK download size (estimated): 9003 KiB (old), 9032 KiB (new), 29 KiB (Added) Method count: 93710 (old), 93608 (new), 102 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5432 (old), 5504 (new), 72 (Added)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 9 MiB (old), 9 MiB (new), 32 KiB (Added)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 204 bytes (Added) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 336 bytes (Added) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 208 bytes (Added) Configuration tvdpiAPK file size: 90 KiB (old), 90 KiB (new), 340 bytes (Added) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 268 bytes (Added) Configuration xxhdpiAPK file size: 68 KiB (old), 69 KiB (new), 208 bytes (Added) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 212 bytes (Added) |
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.
Thanks @adhiamboperes! PR LGTM. Feel free to resolve the last few comments when they seem addressed & merge at your convenience.
app/src/sharedTest/java/org/oppia/android/app/databinding/ColorBindingAdaptersTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/databinding/ColorBindingAdaptersTest.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/res/layout-sw600dp-land/onboarding_app_language_selection_fragment.xml
Outdated
Show resolved
Hide resolved
Enabling Auto-merge, everything looks good after addressing remaining comments. |
Explanation
Fixes Part of #4938: Introuduces new app language selection screen.
This PR introduces the layout files and the presenter for displaying the language functionality, along with associated test cases.
I have modified the custom view
SurveyOnboardingBackgroundView
to make it generic and reusable in the new layouts.These changes include both darkmode support and alternate screen size and orientation layouts, as per figma mocks.
Essential Checklist
For UI-specific PRs only