Authentication edit#10
Conversation
|
will review it shortly! |
2tanayk
left a comment
2tanayk
left a comment
There was a problem hiding this comment.
Hey first and foremost a big thanks to your changes it looks fine and really loved your approach!,
Can you make the changes as requested in the review? Hope to get this merged soon!
| ): View? { | ||
| // Inflate the layout for this fragment | ||
| val view = inflater.inflate(R.layout.fragment_login, container, false) | ||
| return inflater.inflate(R.layout.fragment_login, container, false) |
There was a problem hiding this comment.
although this gets rid of the boilerplate and redundancy, I prefer val view = inflater.inflate(R.layout.fragment_login, container, false) , just incase one needs to instantiate some view from the layout. So can you change it back to the way it was before?
| if (TextUtils.isEmpty(email) || TextUtils.isEmpty(password)) { | ||
| Toast.makeText(activity, "Some Field is Empty!", Toast.LENGTH_SHORT).show() | ||
| return | ||
| return false |
There was a problem hiding this comment.
since you have disabled the sign in button on an empty email/password, this if statement is effectively redundant so can you remove it?
And really sorry for the delay was caught up with a lot of work :/
| } | ||
|
|
||
| //textwatcher | ||
| private val textWatcher = object : TextWatcher { |
There was a problem hiding this comment.
can you use intuitive and specific names like signInTextWatcher for the sake of code maintenance?
| ): View? { | ||
| // Inflate the layout for this fragment | ||
| val view = inflater.inflate(R.layout.fragment_register, container, false) | ||
| return inflater.inflate(R.layout.fragment_register, container, false) |
There was a problem hiding this comment.
same as before here, so can you make the requested changes?
| } | ||
|
|
||
| //textwatcher | ||
| val textWatcher : TextWatcher = object : TextWatcher{ |
There was a problem hiding this comment.
same as before here too,use a specific name
|
|
||
| // make function independent | ||
| private fun validate(name: String, email: String, password: String, confirmPassword: String) : Boolean { | ||
| if (TextUtils.isEmpty(name) || TextUtils.isEmpty(email) || TextUtils.isEmpty(password) || TextUtils.isEmpty( |
There was a problem hiding this comment.
same as before, this is redundant now and must be removed
2 participants
managed to control button state and do some minor refactoring in login/register fragments. Criteria used to determine state is that the text fields should be filled.