-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[Fragment-Migration] Sync Login Screen to use Fragments instead of activities #17908
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
Conversation
7306b07 to
25e9a69
Compare
|
Please test locally before pushing |
25e9a69 to
dde04fc
Compare
BrayanDSO
left a comment
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.
How Has This Been Tested?
Test On API 35
Quoting the PR template:
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration (SDK version(s), emulator or physical, etc)
So please do that. Your description isn't enough for anyone to reproduce or trust your testing process. A video would be even better.
c36fd3f to
033deeb
Compare
|
I have uploaded all the SSs to drive along with few test videos |
BrayanDSO
left a comment
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.
Haven't reviewed all of it, but it looks good overall
AnkiDroid/src/main/java/com/ichi2/anki/account/LoginFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/AccountActivity.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/AccountActivity.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/LoginViewModel.kt
Outdated
Show resolved
Hide resolved
f66bf81 to
3747726
Compare
a14930a to
4e1bfec
Compare
BrayanDSO
left a comment
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.
In the future, we ideally should rely less on AccountActivity and be able to launch the fragment anywhere.
AnkiDroid/src/main/java/com/ichi2/anki/account/LoginViewModel.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/LoggedInViewModel.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/LoggedInViewModel.kt
Outdated
Show resolved
Hide resolved
|
|
||
| override fun onConfigurationChanged(newConfig: Configuration) { | ||
| super.onConfigurationChanged(newConfig) | ||
| loggedInLogo.isVisible = !(isScreenSmall && newConfig.orientation == Configuration.ORIENTATION_LANDSCAPE) |
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.
If the parent activity doesn't have android:configChanges="orientation", this doesn't work.
I get that having a separate layout file just for that is a lot of boilerplate, so I'm fine with the current approach.
But do note at the class docs that the parent activity needs to listen to "orientation" configuration changes to work properly, and note why a separate layout file isn't being used.
AnkiDroid/src/main/java/com/ichi2/anki/account/LoginFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/account/AccountActivity.kt
Outdated
Show resolved
Hide resolved
4e1bfec to
a9daf73
Compare
- LoggedIn viewmodel to handle logged in state - enum class LoginError to handle the errors related to Login and LoginState class for various login states
update: MyAccount Unit Tests add activity to the activity list
…e of MyAccount activity
- update usage to new LoginFragment fix: multimedia uri security exception
a9daf73 to
c31dc7c
Compare
Purpose / Description
Approach
!LoggedInsituation as we are starting the fragments from preference screenHow Has This Been Tested?
Test On API 35 Google emulator:
I am uploading all the screenshots and videos to drive as it would be too much for the template
https://drive.google.com/drive/folders/1lYg4rz-SdFhwb-3rlQIs5aUwnjwJgDiK?usp=sharing
Checklist
Please, go through these checks before submitting the PR.