Feat : Updated SignUp UI with password constraints #587

shikhart98 · 2018-09-26T00:52:26Z

ScreenShot:

Fixes #585

iamareebjamal · 2018-09-26T01:56:27Z

Please follow commit guidelines

shikhart98 · 2018-09-26T05:52:04Z

I'll commit code for constraints today itself, and then if you want I can squash all commits.

nikit19 · 2018-09-26T19:45:10Z

app/src/main/res/layout/fragment_signup.xml

+        <TextView
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:hint="@string/password_constraints"/>


This textview is not required IMO

I think at the time of signup we should tell user about the constraints. It is not required in login page.

shikhart98 · 2018-09-28T15:35:22Z

Please Review my PR.

iamareebjamal · 2018-09-29T07:25:47Z

This should be handled in a way that if the password does not follow constraints, show them a warning or error. Not always

shikhart98 · 2018-09-29T11:53:40Z

@iamareebjamal Okay sir, so should I place a toast whenever user enters a password less than 6 characters?

iamareebjamal · 2018-09-29T14:05:16Z

No, display TextInputLayout error

shikhart98 · 2018-09-30T05:07:22Z

@iamareebjamal I have taken care of the error message this time. Now until and unless user writes a password greater than or equal to 6 characters this warning in red will be displayed (it will not be displayed initially when length of password is zero or when user clears the password field)

simarsingh24

Why are you fixing length to 6? I guess we ill have to ask the mentors for the limit @iamareebjamal what are your views?

shikhart98 · 2018-09-30T15:00:33Z

Explanation for latest commit ( b737bc4 ) : What if user clicks the sign up button even after seeing the warning? Later the application let user to signup, but now it will show a toast as shown below and won't let user to signup until and unless user obeys the password constraints.

shikhart98 · 2018-09-30T15:04:05Z

@simarsingh24 When I posted the issue no one commented on this thing! You can tell me the limit, no problem.

iamareebjamal · 2018-09-30T08:11:55Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt


        signUpActivityViewModel.error.observe(this, Observer {
-            Toast.makeText(context, it, Toast.LENGTH_LONG).show()
+                Toast.makeText(context, it, Toast.LENGTH_LONG).show()


Wrong indentation

iamareebjamal · 2018-09-30T08:12:30Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt

+                    else -> textInputLayoutPassword.error = "Password too short!"
+                }
+            }
+            override fun beforeTextChanged(p0: CharSequence?, p1: Int, p2: Int, p3: Int) {}


Add comments about empty body

iamareebjamal · 2018-09-30T08:12:51Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt

+                }
+            }
+            override fun beforeTextChanged(p0: CharSequence?, p1: Int, p2: Int, p3: Int) {}
+            override fun onTextChanged(p0: CharSequence?, p1: Int, p2: Int, p3: Int) {}


Add comments about empty body

iamareebjamal · 2018-09-30T15:54:49Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragmentViewModel.kt

            error.value = "Passwords do not match!"
            return true
        }
+        if(password.length <6){


Newline above this line and space before and after parens

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragmentViewModel.kt

iamareebjamal · 2018-10-02T03:48:20Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt

+                        textInputLayoutPassword.error = null
+                        textInputLayoutPassword.isErrorEnabled = false
+                    }
+                    else -> textInputLayoutPassword.error = "Password too short!"


This should be if else

Previously I wrote 'If else' statement here but then kotlin gave me this suggestion(to use when)! If you want I can roll back to 'If else' statement

Yes, please move back to if else. When is like switch case and should be used when there are multiple cases

iamareebjamal · 2018-10-02T03:48:59Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt

+        rootView.passwordSignUp.addTextChangedListener(object : TextWatcher{
+            override fun afterTextChanged(p0: Editable?) {
+                when {
+                    (passwordSignUp.text.toString().length>=6 || passwordSignUp.text.toString().isEmpty()) -> {


Second case is redundant. First case already checks that the string can't be empty

@iamareebjamal Suppose user writes a password of less than 6 characters (error message appears), now user clear the password field (now because the password field is empty there should not be any error message, this situation is same as if user didn't type any password)

@iamareebjamal My second case is if the string is empty then also remove the error message!

Yeah alright, my mistake. I didn't see the condition quite clearly 👍

shikhart98 · 2018-10-06T03:34:02Z

Please merge this PR

iamareebjamal · 2018-10-07T04:12:48Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragment.kt

            redirectToMain()
        })

+        rootView.passwordSignUp.addTextChangedListener(object : TextWatcher{


Add space after TextWatcher

iamareebjamal · 2018-10-07T16:48:21Z

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragmentViewModel.kt

        }
+
+        if (password.length < 6) {
+            error.value = "Password should be greater than 6 characters!"


simarsingh24 · 2018-10-09T04:45:48Z

app/src/main/res/values/strings.xml

    <!--settings profile dialog-->
    <string name="message">Are you sure you want to log out?</string>
    <string name="cancel">Cancel</string>
+    <string name="password_constraints">* Atleast 6 characters</string>


where is this used?

otherwise LGTM 👍

Now it is not used! I'll remove this today itself!

liveHarshit · 2018-10-09T15:12:19Z

I think something went wrong!
git showed me this
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

then I did

git pull upstream development --rebase
git push origin bb -f

Now all my previous commits are stacked here again :/ can anyone help?

@shikhart98 Simply follow these steps :

run git checkout bb
run git pull --rebase upstream development
run git log
copy the latest commit id of (upstream->development)
run git reset --soft "paste commit id here"
now commit your changes 'git commit'
now run git push origin +bb

shikhart98 · 2018-10-09T15:14:26Z

@liveHarshit its all sorted now :)
@nikit19 @simarsingh24 @iamareebjamal Please review!

liveHarshit · 2018-10-09T15:19:46Z

@liveHarshit its all sorted now :)
@nikit19 @simarsingh24 @iamareebjamal Please review!

@shikhart98 But you have nine commits, so you need to squash them. Simply follow the steps.

simarsingh24 · 2018-10-09T15:31:32Z

squash your commits

nikit19

LGTM. Please squash your commits

shikhart98 closed this Sep 26, 2018

shikhart98 reopened this Sep 26, 2018

shikhart98 force-pushed the bb branch from b5ca766 to c21c761 Compare September 26, 2018 05:49

nikit19 reviewed Sep 26, 2018

View reviewed changes

shikhart98 closed this Sep 27, 2018

shikhart98 reopened this Sep 27, 2018

simarsingh24 reviewed Sep 30, 2018

View reviewed changes

iamareebjamal requested changes Sep 30, 2018

View reviewed changes

shikhart98 added a commit to shikhart98/open-event-android that referenced this pull request Oct 1, 2018

fix : Improve indentation and add comments (fossasia#587)

b34f881

iamareebjamal reviewed Oct 1, 2018

View reviewed changes

app/src/main/java/org/fossasia/openevent/general/auth/SignUpFragmentViewModel.kt Outdated Show resolved Hide resolved

shikhart98 added a commit to shikhart98/open-event-android that referenced this pull request Oct 2, 2018

fix : Improve code style (fossasia#587)

1d83399

iamareebjamal reviewed Oct 2, 2018

View reviewed changes

simarsingh24 previously approved these changes Oct 6, 2018

View reviewed changes

iamareebjamal reviewed Oct 7, 2018

View reviewed changes

shikhart98 dismissed simarsingh24’s stale review via 98618f6 October 7, 2018 16:34

iamareebjamal approved these changes Oct 7, 2018

View reviewed changes

iamareebjamal requested changes Oct 7, 2018

View reviewed changes

iamareebjamal previously approved these changes Oct 7, 2018

View reviewed changes

simarsingh24 reviewed Oct 9, 2018

View reviewed changes

shikhart98 dismissed iamareebjamal’s stale review via c4352ad October 9, 2018 13:59

shikhart98 force-pushed the bb branch from 81c4233 to c4352ad Compare October 9, 2018 13:59

shikhart98 added a commit to shikhart98/open-event-android that referenced this pull request Oct 9, 2018

fix : Improve indentation and add comments (fossasia#587)

5e878d5

shikhart98 added a commit to shikhart98/open-event-android that referenced this pull request Oct 9, 2018

fix : Improve code style (fossasia#587)

a200736

nikit19 reviewed Oct 9, 2018

View reviewed changes

fix : Update signup with password constraints

d5e1076

shikhart98 force-pushed the bb branch from c4352ad to d5e1076 Compare October 9, 2018 17:07

iamareebjamal approved these changes Oct 9, 2018

View reviewed changes

iamareebjamal merged commit 59088b2 into fossasia:development Oct 9, 2018

Feat : Updated SignUp UI with password constraints #587

Feat : Updated SignUp UI with password constraints #587

Uh oh!

Conversation

shikhart98 commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamareebjamal commented Sep 26, 2018

Uh oh!

shikhart98 commented Sep 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shikhart98 Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shikhart98 commented Sep 28, 2018

Uh oh!

iamareebjamal commented Sep 29, 2018

Uh oh!

shikhart98 commented Sep 29, 2018

Uh oh!

iamareebjamal commented Sep 29, 2018

Uh oh!

shikhart98 commented Sep 30, 2018

Uh oh!

simarsingh24 left a comment

Choose a reason for hiding this comment

Uh oh!

shikhart98 commented Sep 30, 2018

Uh oh!

shikhart98 commented Sep 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shikhart98 commented Oct 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liveHarshit commented Oct 9, 2018

Uh oh!

shikhart98 commented Oct 9, 2018

Uh oh!

liveHarshit commented Oct 9, 2018

Uh oh!

simarsingh24 commented Oct 9, 2018

Uh oh!

nikit19 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

shikhart98 commented Sep 26, 2018 •

edited

Loading

shikhart98 Sep 27, 2018 •

edited

Loading

nikit19 left a comment •

edited

Loading