Fix #4072 and Fix Part of #4938: Revised profile chooser UI #5468
Conversation
…le-domain-config # Conflicts: # model/src/main/proto/oppia_logger.proto # testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt # utility/src/main/java/org/oppia/android/util/logging/EventBundleCreator.kt
…reen # Conflicts: # app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt
…le-domain-config # Conflicts: # app/src/main/java/org/oppia/android/app/onboarding/IntroFragment.kt
Coverage ReportResultsNumber of files assessed: 18 Exempted coverageFiles exempted from coverage
|
There was a problem hiding this comment.
Thanks @adhiamboperes! Took a full pass. Some top-level feedback:
- The 'add more profiles' and floating action button feel a bit crowded on portrait and would benefit from a bit more space (similar to the landscape version).
- How do the arrows and fling behaviors in landscape behave in RTL mode?
- As discussed during today's CLaM meeting, the landscape layout looks a bit off due to the solo learner bubble being so far left. I'm not quite sure that it should be centered, though, as landscape layouts are usually designed to have important elements easily reachable with one's thumbs. Perhaps placing it at the 1/4 or 1/3 portion of the screen from the left might be a bit cleaner? This seems even worse on tablet landscape mode--that's a lot of blank whitespace.
- As discussed during today's CLaM meeting, how does the new layout behave with Talkback?
app/src/main/java/org/oppia/android/app/onboarding/IntroFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/onboarding/IntroFragmentPresenter.kt
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/profile/ProfileListView.kt
Outdated
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt
Outdated
Show resolved
Hide resolved
| @@ -1225,7 +1226,7 @@ class ProfileManagementController @Inject constructor( | |||
| // TODO(#3616): Migrate to the proper SDK 29+ APIs. | |||
| @Suppress("DEPRECATION") // The code is correct for targeted versions of Android. | |||
| val bitmap = MediaStore.Images.Media.getBitmap(context.contentResolver, avatarImagePath) | |||
| val fileName = avatarImagePath.pathSegments.last() | |||
| val fileName = UUID.randomUUID().toString() | |||
There was a problem hiding this comment.
Since this is fixing a bug, is it possible for us to add a test to at least verify this functionality (even if it's a bit contrived)?
There was a problem hiding this comment.
Reminder on this comment since I don't think it was addressed yet.
| val offset = | ||
| if (isLeft) scrollDistance - scrollableWidth else scrollableWidth - scrollDistance |
There was a problem hiding this comment.
Does this work correctly in RTL?
There was a problem hiding this comment.
With some modification to account for layout direction, it works correctly.
https://github.com/user-attachments/assets/5371b810-e068-48e0-8129-95dff2bee516
There was a problem hiding this comment.
This looks really nice! Just to check (since I couldn't quite determine from the video): does RTL mode start with being scrolled all the way to the right? I see that the profile list is correctly reversed (with admin starting at the right), so just wanted to check for initial scroll state.
app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/profile/ProfileChooserFragmentTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/profile/ProfileChooserFragmentTest.kt
Show resolved
Hide resolved
…file-chooser-ui-views
…-android into new-profile-chooser-ui-views
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: 19 MiB (old), 19 MiB (new), 43 KiB (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 36 KiB (Added) Method count: 260197 (old), 260501 (new), 304 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6818 (old), 6868 (new), 50 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 42 KiB (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 36 bytes (Added) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 48 bytes (Added) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 48 bytes (Added) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 60 bytes (Added) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 36 bytes (Added) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 48 bytes (Added) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 60 bytes (Added) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 29 KiB (Added) Method count: 116267 (old), 116452 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 29 KiB (Added) Method count: 116273 (old), 116458 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 25 KiB (Added) Method count: 116273 (old), 116458 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
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: 19 MiB (old), 19 MiB (new), 43 KiB (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 37 KiB (Added) Method count: 260202 (old), 260501 (new), 299 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6818 (old), 6868 (new), 50 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 42 KiB (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 36 bytes (Added) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 48 bytes (Added) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 48 bytes (Added) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 60 bytes (Added) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 36 bytes (Added) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 48 bytes (Added) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 60 bytes (Added) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 28 KiB (Added) Method count: 116280 (old), 116452 (new), 172 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 33 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 28 KiB (Added) Method count: 116286 (old), 116458 (new), 172 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 24 KiB (Added) Method count: 116286 (old), 116458 (new), 172 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
|
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. |
oppiabot
bot
added
the
stale
There was a problem hiding this comment.
Apologies for the delayed review. Thanks @adhiamboperes, this LGTM!
oppiabot
bot
removed
the
stale
|
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. |
oppiabot
bot
added
the
stale
oppiabot
bot
removed
the
stale
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: 19 MiB (old), 19 MiB (new), 43 KiB (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 35 KiB (Added) Method count: 260219 (old), 260523 (new), 304 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6818 (old), 6868 (new), 50 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 42 KiB (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 36 bytes (Added) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 48 bytes (Added) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 48 bytes (Added) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 60 bytes (Added) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 36 bytes (Added) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 48 bytes (Added) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 60 bytes (Added) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 36 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 31 KiB (Added) Method count: 116287 (old), 116472 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 36 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 30 KiB (Added) Method count: 116293 (old), 116478 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 36 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 27 KiB (Added) Method count: 116293 (old), 116478 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5827 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 35 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
|
@adhiamboperes I think you mentioned in our quick sync today that you'd like another review on this PR. Is that correct, and does that mean that this PR is ready for another pass? I'm asking since it's still assigned to you, so I wasn't sure. :) |
…file-chooser-ui-views
|
@BenHenning, PTAL. |
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: 19 MiB (old), 19 MiB (new), 38 KiB (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 30 KiB (Added) Method count: 260263 (old), 260552 (new), 289 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6820 (old), 6870 (new), 50 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 37 KiB (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 36 bytes (Added) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 48 bytes (Added) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 48 bytes (Added) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 60 bytes (Added) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 36 bytes (Added) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 48 bytes (Added) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 60 bytes (Added) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 33 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 27 KiB (Added) Method count: 115793 (old), 115978 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5788 (old), 5829 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 33 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 31 KiB (Added) Method count: 115799 (old), 115984 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5788 (old), 5829 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 11 MiB (new), 33 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 34 KiB (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 31 KiB (Added) Method count: 115799 (old), 115984 (new), 185 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5788 (old), 5829 (new), 41 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 11 MiB (new), 33 KiB (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
There was a problem hiding this comment.
Thanks @adhiamboperes! Took another pass. I think most of my previous comments have been addressed, but a few haven't. Besides the open conversations, this includes:
- The 'add more profiles' and floating action button feel a bit crowded on portrait and would benefit from a bit more space (similar to the landscape version).
- As discussed during today's CLaM meeting, the landscape layout looks a bit off due to the solo learner bubble being so far left. I'm not quite sure that it should be centered, though, as landscape layouts are usually designed to have important elements easily reachable with one's thumbs. Perhaps placing it at the 1/4 or 1/3 portion of the screen from the left might be a bit cleaner? This seems even worse on tablet landscape mode--that's a lot of blank whitespace.
- As discussed during today's CLaM meeting, how does the new layout behave with Talkback?
PTAL at the open comment threads and the above.
| val scrollLeftInRtl = isRtl && isLeft | ||
| val scrollRightInRtl = isRtl && !isLeft | ||
| val scrollLeftInLtr = !isRtl && isLeft | ||
| val scrollRightInLtr = !isRtl && !isLeft |
There was a problem hiding this comment.
Are there tests for verifying this RTL/LTR and directional scrolling behaviors?
| val offset = | ||
| if (isLeft) scrollDistance - scrollableWidth else scrollableWidth - scrollDistance |
There was a problem hiding this comment.
This looks really nice! Just to check (since I couldn't quite determine from the video): does RTL mode start with being scrolled all the way to the right? I see that the profile list is correctly reversed (with admin starting at the right), so just wanted to check for initial scroll state.
| @@ -1225,7 +1226,7 @@ class ProfileManagementController @Inject constructor( | |||
| // TODO(#3616): Migrate to the proper SDK 29+ APIs. | |||
| @Suppress("DEPRECATION") // The code is correct for targeted versions of Android. | |||
| val bitmap = MediaStore.Images.Media.getBitmap(context.contentResolver, avatarImagePath) | |||
| val fileName = avatarImagePath.pathSegments.last() | |||
| val fileName = UUID.randomUUID().toString() | |||
There was a problem hiding this comment.
Reminder on this comment since I don't think it was addressed yet.
Explanation
Fixes #4072.
The selected image uri created by
com.miui.galleryapp in Xiaomi devices has a different format from the android recommended uri, and is therefore not parsed correctly. Changing the name of the stored file, to be generated using a random UUID, fixes the issue. Please see #4072 (comment) for more information.Fixes Part of #4938: Creates the new Profile chooser UI.
The portrait mode layout is a grid layout recylerview, while landscape mode utilizes a custom recyclerview to create a carousel. This design choice is kind of inconsistent, but more intuitive for users in landscape mode.
ProfileChooserFragmentPresenterhandles the data binding and UI interactions for both layouts.Care has been taken to ensure the existing profile behaviour e.g. random color selection and 10 profiles limit have been retained.
Additionally, a new
ParentScreenenum has been created for the onboarding IntroActivity to control when to show the step count. Per PRD, the step count should not show for additional learners.Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then:
Phone
Tablet