-
Notifications
You must be signed in to change notification settings - Fork 553
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: Disable Google Credentials Sign In leading to crash #1534
fix: Disable Google Credentials Sign In leading to crash #1534
Conversation
private lateinit var thisActivity: Activity | ||
private var credentialsClient: CredentialsClient? = null | ||
private var signInClient: GoogleSignInClient? = null | ||
private var thisActivity: Activity? = null |
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.
This is a very very poor implementation of ViewModel. I don't know what was I smoking when I let this get merged.
There are Android classes used in ViewModel. No doubt there was a crash 🤦♂️
Remove this feature completely and create an issue to reimplement this
Don't delete this ViewModel
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.
removed
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.
Revert this
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.
Done
b33896a
to
967a812
Compare
Fixes: fossasia#1531 Changes: - Removes the Smart Auth features to fix app crash. These will be reimplemented later SmartAuthViewModels files have not been deleted
967a812
to
e3a1602
Compare
@iamareebjamal Is anything else required in this? |
Fixes: #1531
Changes:
Screenshots for the change: