anitab-org / mentorship-android

Mentorship System is an application that matches women in tech to mentor each other, on career development, through 1:1 relations during a certain period of time. This is the Android application of this project.
https://github.com/anitab-org/mentorship-android/raw/apk/app-debug.apk
GNU General Public License v3.0
165 stars 379 forks source link

Error icon overlaps with password toggle icon in password and confirm password editTexts in SignUpActivity #1141

Closed Acash512 closed 3 years ago

Acash512 commented 3 years ago

Is your feature request related to a problem? Please describe.

When the text in the confirm password editText doesn't match the Password, an error is displayed but the error icon overlaps the toggle password icon as evident in the screenshot attached. Moreover when the password is not strong enough and the user click the sign up button, an error message is displayed below Password text field but the error icon in this case unlike the former completely covers the password toggle icon and password toggle is not even clickable now(as here the error is set on TextInputLayout) ie I cannot see what I typed unless I enter a strong password as recommended and then click the signup button because until then the error won't go and the password toggle icon won't be clickable.

Describe the solution you'd like

Firstly instead of setting the errors on EditTexts in the TextWatchers, they should be set on TextInputLayouts. In fact in the validateDetails function the errors have been set on the TextInputLayouts only, I propose to do the same in TextWatchers too. Then if we set errorIconDrawable as null for password and confirm password text input layouts, the error message will be visible below the respective text fields but no error icon will be shown which will solve the issue.

Additional context

Error set on EditText Error set on TextInputLayout
epicadk commented 3 years ago

I remember this being reported before, can't seem to recall where though cc @vj-codes.

@Acash512 Would you like to work on it?

Acash512 commented 3 years ago

Yes, definitely!

epicadk commented 3 years ago

Yes, definitely!

Good luck

Acash512 commented 3 years ago

Thankyou @epicadk , I have a doubt though, shall I set errors on TextInputLayouts for other fields too or just the password ones, I guess the former would be better as far as the user interface is concerned

epicadk commented 3 years ago

Sure you can set relevant errors on other layouts as well. Just remove the visibility toggle from the confirm password. Please update the issue title and description accordingly.

Acash512 commented 3 years ago

There is no need to remove the visibility toggle, we can just avoid showing the error icon which causes the problem.

epicadk commented 3 years ago

Generally the confirm password doesn't have a visibility toggle cc @anitab-org/mentorship-designers

Acash512 commented 3 years ago

Even if I remove it from Confirm password, the issue will persist in the Password text field as evident in the second screenshot that I have attached

epicadk commented 3 years ago

Even if I remove it from Confirm password, the issue will persist in the Password text field as evident in the second screenshot that I have attached

Yeah, remove the error icon form the Password field and the toggle from the confirm password field

Acash512 commented 3 years ago

This is how it looks now, please take a look @epicadk

Acash512 commented 3 years ago

It would be better if we keep the toggle in confirm password too from user's perspective, I myself enter it wrong sometimes in the confirm password field, and if I could see it, it would be much better.

epicadk commented 3 years ago

Hmm, maybe we can discuss this in the design open session. cc @annabauza

Acash512 commented 3 years ago

Shall I send the pr @epicadk, for the above changes?

epicadk commented 3 years ago

Yup go ahead.

Acash512 commented 3 years ago

I have created a pr @epicadk, you may review