-
Notifications
You must be signed in to change notification settings - Fork 0
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: Bugs with navigation and UI improvements on EditProfile #87
Conversation
- Handle onBackPressed - Invalidate `Done` button on data change on EditProfile - Add Loader while hitting API fix: LEARNER-10388
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.
One question
@@ -163,6 +165,17 @@ class EditProfileFragment : Fragment() { | |||
} | |||
} | |||
|
|||
private val onBackPressedCallback = object : OnBackPressedCallback(true) { | |||
override fun handleOnBackPressed() { | |||
if (viewModel.profileDataChanged) { |
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 code is very close to lambda in EditProfileScreen
onBackClick = {
if (it) {
viewModel.setShowLeaveDialog(true)
} else {
viewModel.setShowLeaveDialog(false)
requireActivity().supportFragmentManager.popBackStackImmediate()
}
},
maybe we can create a separate method from 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.
I shared the feedback and the patch with fixes last Friday on Slack. 👍🏻
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! Great improvement from the last review
fix: Bugs with navigation and UI improvements on EditProfile - Handle onBackPressed - Invalidate `Done` button on data change on EditProfile - Add Loader while hitting API fix: LEARNER-10388
Description:
Done
button on data change on EditProfilefix: LEARNER-10388