Skip to content
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

Issue #120 Clear input box after updating password #128

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

umih874
Copy link
Contributor

@umih874 umih874 commented Mar 23, 2018

NOTE

While solving this issue some bugs related to issue #27 were discovered. Since the pull request, #66 has already been merged and team Veovo worked on issue #27 we decide to add fixes for that issue in this pull request.

Changes made for the issue #27

This is also referenced in #66 which explains how to test and validate the implemented functionality.
The current password field was bounded to new password field so whenever there was an error in new password field the error for current password would be displayed. The bonding issue was solved and so that error for each field are only associated and triggered by the faulty field.
Further changes were made to limit the number of characters the user can input (i.e. min 4 and max 50) identical to the property of the other input boxes.

Changes made for this issue #120

The additional function was added to the component file to clear the input fields from the entered input once the user attempts to log in. This is done by simply writing null to the input box value. The issue of the unwanted message showing described in #120 (in image 4) was resolved by adding boolean value that will block them being displayed when the user tries to log in. The blocking will stop once the user tries to input new text.

Testing

  • Try to change the password with an invalid and/or valid current password and check if the input boxes and the messages "Your current password is required." have cleared upon error or success message shows up.
  • Test if the functionality implemented does not affect any other functionalities.

Image evidence

Prior to password change

prelogin

After changing the password

postlogin

umih874 added 4 commits March 16, 2018 15:23
Current password input box was set bunded to new password input box. So same error will display for both when new password box is invalid. Fixed it so that current password input box has its own error association.
Once the user applies changes the password input box will get cleared from the previously entered values.
@umih874
Copy link
Contributor Author

umih874 commented Mar 23, 2018

@zaclochhead @sidpartha1 please review changes

@zaclochhead
Copy link
Contributor

Functionality is working as stated in the pull request. LGTM!

Copy link
Contributor

@zaclochhead zaclochhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@sidpartha1 sidpartha1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JoeHorvath
Copy link
Contributor

Just a heads up, my issue #4, which will be merged soon, involves moving the password page to settings so will no doubt cause some conflicts with yours. Basically you will have to move your password.component html and ts changes to settings.component html and ts. It shouldn't be too hard.

@umih874 umih874 closed this Mar 27, 2018
@umih874 umih874 reopened this Mar 27, 2018
@umih874
Copy link
Contributor Author

umih874 commented Mar 27, 2018

Is there a way you can pull my changes and then move code around? That way we shouldn't get any merge conflicts?

@JoeHorvath
Copy link
Contributor

I think it will be easier if you merge your changes as you understand them better than me.

@umih874
Copy link
Contributor Author

umih874 commented Mar 28, 2018

I had a look at it. This issue is depended on the issue #27 PR #66 which has already been merged. So I think that you will be moving those to a different component anyways, right? If I move these changes to a different component than this will cause an error because these changes are depended on those is #66. I still think it is easier for you to pull my changes since you won't get any merge conflicts. What is the best way of doing this? @softeng-701?

@JoeHorvath
Copy link
Contributor

My branch is already conflict free with the merged changes as they were submitted before my request. Those dependencies should not be affected as long as you merge your branch correctly.

@softeng-701
Copy link
Collaborator

Squash & review please~

umih874 and others added 5 commits March 28, 2018 16:43
Fixed the input box bounding error from Issue#27 (PR kblincoe#66)

Current password input box was set bunded to new password input box. So same error will display for both when new password box is invalid. Fixed it so that current password input box has its own error association.
Once the user applies changes the password input box will get cleared from the previously entered values.
@umih874
Copy link
Contributor Author

umih874 commented Mar 28, 2018

@softeng-701 I have tried squashing my commits but it added all these commits when I pulled from the kelly's fork. I can't squash these now. What should I do?

Jcen902 and others added 5 commits March 28, 2018 17:39
Fixed the input box bounding error from Issue#27 (PR kblincoe#66)

Current password input box was set bunded to new password input box. So same error will display for both when new password box is invalid. Fixed it so that current password input box has its own error association.

Added clear field functionality to password page

Once the user applies changes the password input box will get cleared from the previously entered values.

Redesigned main page. Added Login/Signup button and images.

Added class to email body.

Class preserves white space and new lines.

add batch files to launch/test QualOpt

added shell scripts for mac users

updating formating of mac shell scripts

Fix Twitter sign-in

Issue kblincoe#8

Added a SendInvitation option to the main study page

Connected GitHub API to search and instantiate new Participants

Add warning that there are unsaved changes in the edit study form

delete unnecessary whitespace (line 85)

chore: add +x permission to mvnw kblincoe#57

Fixing typos

Fixed all typing and punctuation errors in the abbout page kblincoe#29

Cleaning up about page

Removed go to studies button and login status on the about page to make it look cleaner

Added shadow to welcome text. Fixed Typo
(this is a squashed version)

Delete Entities tab

Fixed the input box bounding error from Issue#27 (PR kblincoe#66)

Current password input box was set bunded to new password input box. So same error will display for both when new password box is invalid. Fixed it so that current password input box has its own error association.
Once the user applies changes the password input box will get cleared from the previously entered values.
@umih874
Copy link
Contributor Author

umih874 commented Mar 28, 2018

@softeng-701 Should be all good now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.