-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[FEATURE REQUEST] New UI for "Manage accounts" view #4410
Conversation
277df42
to
2d78b9e
Compare
Modified too:
The aim of the last 2 points is to centralize all the accounts management functionalities in the new dialog |
c8c4e08
to
be124fc
Compare
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.
Some questions here @JuancaG05
owncloudApp/src/main/java/com/owncloud/android/presentation/accounts/ManageAccountsAdapter.kt
Outdated
Show resolved
Hide resolved
...App/src/main/java/com/owncloud/android/presentation/accounts/ManageAccountsDialogFragment.kt
Outdated
Show resolved
Hide resolved
...App/src/main/java/com/owncloud/android/presentation/accounts/ManageAccountsDialogFragment.kt
Show resolved
Hide resolved
...App/src/main/java/com/owncloud/android/presentation/accounts/ManageAccountsDialogFragment.kt
Outdated
Show resolved
Hide resolved
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.
LGTM, ready for QA!
(1) [FIXED]
Current: app crashes. This is the stacktrace:
Expected: no crash Android 11, Pixel 2 |
@jesmrec (1) should be fixed now |
(2) [FIXED]
Current: Nothing is locally deleted Expected: Local copies are removed Pixel 2, Android 11 |
(3) [WONT FIX HERE]This one is also reproducible in
Current: app crashes, this is the stacktrace:
Expected: login view displayed with no crash Pixel 2, Android 11 |
@jesmrec (2) should be fixed now. I'm able to reproduce (3) but I don't think it has an immediate fix, so in order not to block the release, we can move it to another issue (also, it produces a crash but after the crash the app works normally, so nothing very intrusive) |
…y and with a nice UX
…d set account when changing context
c12ed3b
to
4fd814d
Compare
(2) is fixed and (3) will be addressed to another issue as no-regression Approved |
Related Issues
App: #4312
ReleaseNotesViewModel.kt
creating a newReleaseNote()
with String resources (if required)QA
Regression test plan over manage accounts: https://github.com/owncloud/QA/blob/master/Mobile/Android/Executions/Release_4.3/Account%20Manager%20Regression.md