-
Notifications
You must be signed in to change notification settings - Fork 2
Fix/misc/clean up toasts #361
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
base: main
Are you sure you want to change the base?
Conversation
…sswordInput and AuthenticationEmailInput
Kept the Toast messages in the TODO section since they will be removed along with other unimplemented features in a future update.
…/clean-up-toasts # Conflicts: # app/src/main/java/com/android/periodpals/ui/map/Map.kt
|
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.
Summary
Good implementation of the feature that streamlines the user flow.
Important
Code Quality
- Code quality is consistent with the rest of the project.
Functionality
- The apps runs as intended.
Testing
- No tests needed.
Documentation
- No documentation needed.
Suggestions
- Read my review comments
- Remove unused toast text in
strings.xml
.
Steps before PR approved
- LGTM
@@ -183,7 +168,6 @@ fun CreateAlertScreen( | |||
else -> null | |||
} | |||
if (errorMessage != null) { | |||
Toast.makeText(context, errorMessage, Toast.LENGTH_SHORT).show() |
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.
add a Log.d
for errorMessage
@@ -177,15 +173,9 @@ fun EditAlertScreen( | |||
else -> null | |||
} | |||
if (errorMessage != null) { | |||
Toast.makeText(context, errorMessage, Toast.LENGTH_SHORT).show() |
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.
add a Log.d for errorMessage
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.
Summary
Great last minute cleanup, should make users' experience smoother and lighter without the toasts.
Important (which includes)
Code Quality
LGTM
Functionality
LGTM
Suggestions
Perhaps add the Log.d like Harrishan asked but I feel as if it is a nitpick.
Clean up the toasts messages (UX oriented)
And task : Clean up the error messages (UX oriented #344
Description
This PR focuses on removing the use of
Toast
messages across the application and replacing them withLog
statements for improved user experience and debugging. It also includes code refactoring to eliminate unnecessary context parameters and handlers, enhancing code clarity and maintainability. I closes issue #343 and #344.Note: The toasts in the TODOs were not removed because they need to be addressed simultaneously in #346.
Changes
Files
Modified
AlertLists.kt
: RemovedToast
messages inAlertListsScreen
andAlertAcceptButtons
, replaced withLog
statements.CreateAlert.kt
: UpdatedToast
messages inCreateAlertScreen
toLog
statements.EditAlert.kt
: ReplacedToast
messages withLog
statements inEditAlertScreen
.SignIn.kt
: Removed context parameters and handlers forToast
messages inSignInScreen
andattemptSignIn
.SignUp.kt
: Cleaned up context parameters and handlers inSignUpScreen
andattemptSignUp
.ProfileComponents.kt
: Removed context parameters and handlers inProfileSaveButton
and related functions.AuthenticationComponents.kt
: Enhanced error state handling for email and password input fields.Map.kt
: ReplacedToast
messages inFetchAlertsAndDrawMarkers
withLog
statements.CreateProfile.kt
: Removed unnecessary context parameters inCreateProfileScreen
.Removed
Toast
messages and related context handlers.Dependencies Added
Testing
Log
statements function correctly and provide meaningful debugging information.Points to Consider
Toast
messages.